Re: [DOC PATCH] semaphore documentation

From: Andrew Morton
Date: Sat Apr 12 2008 - 15:28:33 EST


On Sat, 12 Apr 2008 08:12:51 -0600 Matthew Wilcox <matthew@xxxxxx> wrote:

> 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@xxxxxxxxxx> 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.

urgh, yes, I'd forgotten about that mess.

I suppose that if might_sleep() checking in down() is useful (and surely it
is) we could provide a separate down_im_stupid() (and
lock_kernel_im_stupid()) which omits the check, and call that from the
problematic sites.

> 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 think it'd be worth playing with some time, but it's off-topic for this
current work.

> I thought that schedule() would warn itself in that case,
> but I can't see the code that would do that now I check.

schedule() will warn ("scheduling while atomic"), but only if we happened
to hit contention.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/