linux-kernel - Re: [REPOST] RFC: sched: Prevent wakeup to enter critical section needlessly

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: <1349792104.7880.41.camel@twins>
Date:	Tue, 09 Oct 2012 16:15:04 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	Ivo Sieben <meltedpianoman@...il.com>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	linux-serial@...r.kernel.org, Alan Cox <alan@...ux.intel.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [REPOST] RFC: sched: Prevent wakeup to enter critical section
 needlessly

On Tue, 2012-10-09 at 06:37 -0700, Andi Kleen wrote:
> Ivo Sieben <meltedpianoman@...il.com> writes:
> 
> > Check the waitqueue task list to be non empty before entering the critical
> > section. This prevents locking the spin lock needlessly in case the queue
> > was empty, and therefor also prevent scheduling overhead on a PREEMPT_RT
> > system.
> >
> > Signed-off-by: Ivo Sieben <meltedpianoman@...il.com>
> > ---
> >
> >  REPOST:
> >  Request for comments:
> >  - Does this make any sense?
> 
> Looks good to me. Avoiding any spinlock is good.  I don't even think you
> need "careful", if you miss an update it was just as it happened a
> little later.

Yeah, so I've started looking at this several times, but never had the
time to actually think it through. I think I'll agree with you that
using the careful list empty check isn't needed.

In general there's already the race of doing a wakeup before the wait
and if the code using the waitqueue doesn't deal with that its already
broken, so in that respect you should be good, since you're simply
shifting the timing a little.

One thing you might need to consider is the memory ordering, will the
list_empty -- either careful or not -- observe the right list pointer,
or could it -- when racing with wait_event()/prepare_to_wait() --
observe a stale value. Or.. is that all already covered in on the use
site.

I started auditing a few users to see what they all assume, if they're
already broken and or if they would now be broken.. but like said, I
didn't get anywhere.

I'd like to see this patch/Changelog amended to at least cover these
points so that I feel all warm and fuzzy when I read it and not have to
think too hard ;-)
--
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/