Re: block_cache

Ingo Weinhold <[email protected]> Tue, 19 Aug 2008 22:33:24 +0200
Newsgroups gmane.os.openbeos.kernel.devel
Message-ID <[email protected]>
On 2008-08-19 at 21:16:20 [+0200], Axel D=F6rfler <[email protected]> =

wrote:
> "Salvatore Benedetto" <[email protected]> wrote:
> > while trying to hunt bug #2634 I found that:
> > 1 - We don't check for the correct registration of low memory handler
> > in the block_cache constructor, which can fail (the registration) due
> > to luck
> > of memory. I don't know how problematic is that, just wanted to
> > pointed it out ;-)
> =

> That's more or less neglectable, at least it's not crucial for the
> functioning of the block cache - maybe more for the rest of the system
> :-)

I guess when registering the low memory handler fails due to lack of memory=
, =

it wouldn't be particularly wise to continue with creating the block cache.

> > 2 - There seem to be an useless check in the
> > get_next_locked_block_cache,
> > -               if (cache->deleting) {
> > -                       mutex_unlock(&cache->lock);
> > -                       continue;
> > -               }
> > =

> > that's after gainig the lock of the cache. The deleting flag on the
> > other hand,
> > it's only set when calling the block_cache destructor, which happens
> > in case the constructor fails, in which case the block_cache is now
> > not even in the
> > sCacheList (and before the mutex itself wouldn't even been
> > initiliazed), and in
> > block_cache_delete, but in this case we first gain the lock of the
> > cache before
> > calling the destructor. So unless I'm missing something else (which
> > might be the case)
> > the check can be safely removed. ;-)
> =

> That depends on the fact if mutex_lock() will really fail upon a
> destroyed mutex.
> Looking at its code, it doesn't really look likely; we might want to
> change that. Until that, this code is needed, though.

Just to throw in a word of warning: mutex_lock() really shouldn't be called =

on a destroyed mutex; that's just begging for hard-to-find multi-threading =

problems. The only acceptable behavior would be to panic(), just as is =

already done when called on an uninitialized mutex.

That reminds me: When we discussed whether the more appropriate behavior fo=
r =

mutex_destroy() is to unblock waiters or to panic() when there still are =

waiters, I spotted a related problem in the network stack (timer?) code. Ha=
s =

this been fixed in the meantime or shall I look up what it was exactly and =

file a ticket?

CU, Ingo

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great priz=
es
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=3D100&url=3D/