linux-kernel - Re: [DOC PATCH] semaphore documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080412141250.GK11962@parisc-linux.org>
Date:	Sat, 12 Apr 2008 08:12:51 -0600
From:	Matthew Wilcox <matthew@....cx>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Randy Dunlap <randy.dunlap@...cle.com>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Harvey Harrison <harvey.harrison@...il.com>
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@...cle.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."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/