Messages in this thread | | | Date | Sat, 12 Apr 2008 08:12:51 -0600 | From | Matthew Wilcox <> | Subject | Re: [DOC PATCH] semaphore documentation |
| |
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."
| |