Re: [DOC PATCH] semaphore documentation

[Matthew Wilcox <matthew@wil.cx>]

On Fri, Apr 11, 2008 at 10:09:11PM -0700, Andrew Morton wrote:
> On Fri, 11 Apr 2008 13:27:54 -0700 Randy Dunlap <randy.dunlap@oracle.com> wrote:
> > Looks good to me.  Thanks.
> 
> Yup, most excellent.

Thanks for the review.

> btw, down() and friends should have might_sleep() checks in them, shouldn't
> they?  They don't seem to be in there, nor in mainline
> lib/semaphore-sleepers.c.  Confused.

Mmm.  Ingo gets annoyed when I add additional checks to semaphores -- he
wants them to maintain their current semantics and to get better checking
by migrating more users to mutexes.  I've already exposed at least one
problem (in aacraid) by adding the __must_check to down_interruptible().

As I wrote in one of the comments, we have places in the kernel which
know that even though they're in a non-sleeping context, there is at
least one more token left in the semaphore.  One place this bit me was
in start_kernel().  We disable interrupts and then call lock_kernel()
which calls down().  Since we're in start_kernel(), we know there's
nothing else running and this is perfectly safe.  But a might_sleep()
would warn bogusly.

I'd be open to putting a might_sleep() in __down().  We definitely are
going to sleep at that point, so getting a warning out of it would
be good.  I thought that schedule() would warn itself in that case,
but I can't see the code that would do that now I check.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."