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: <20080412122724.0897d881.akpm@linux-foundation.org>
Date:	Sat, 12 Apr 2008 12:27:24 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Matthew Wilcox <matthew@....cx>
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 Sat, 12 Apr 2008 08:12:51 -0600 Matthew Wilcox <matthew@....cx> 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@...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.

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/