linux-kernel - Re: [RFC] per-cpu preempt

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] [day] [month] [year] [list]

Message-ID: <20130818175702.GK29406@linux.vnet.ibm.com>
Date:	Sun, 18 Aug 2013 10:57:02 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Mike Galbraith <bitbucket@...ine.de>,
	Peter Anvin <hpa@...or.com>, Andi Kleen <ak@...ux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] per-cpu preempt_count

On Mon, Aug 12, 2013 at 10:35:48AM -0700, Linus Torvalds wrote:
> On Mon, Aug 12, 2013 at 4:51 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> >
> > The below boots to wanting to mount a root filesystem with
> > CONFIG_PREEMPT=y using kvm -smp 4.
> 
> But doesn't work in general? Or you just never tested?
> 
> I think that "thread_info->preempt_count" variable would need to be
> renamed to "saved_preempt_count" or similar to make sure we catch any
> users. But the patch certainly looks simple otherwise.
> 
> I'm pretty sure I had a discussion about this with Paul McKenney some
> time ago (because the RCU readlock is the most noticeable user of the
> preempt count - the others tend to be hidden inside the out-of-line
> spinlock functions etc), and I thought he had tried this and had some
> problems. Maybe we've fixed things since, or maybe he missed some
> case..

I was doing something a bit different -- trying to put preemptible RCU's
nesting counter into a per-CPU variable.  I considered putting this
counter into thread_info, but got flummoxed by the save/restore code.
If Peter's approach works out, I will look into a similar approach for
RCU's nesting counter.

For whatever it is worth, with the current Kconfigs, RCU only invokes
preempt_enable() and preempt_disable() when CONFIG_PREEMPT=n, in which
case these two functions are nops.  So RCU never exercises the
conditional function call in preempt_enable().

However, preemptible RCU has a situation similar to preempt_disable()
and preempt_enable(): simple increment and (not so simple) decrement in
the common case, and rare conditional function call from rcu_read_unlock()
that is invoked only if the read-side critical section was preempted or
ran for a long time.

							Thanx, Paul

> But if the patch really is this simple, then we should just do it. Of
> course, we should double-check that the percpu preempt count is in a
> cacheline that is already accessed (preferably already dirtied) by the
> context switching code.  And I think this should be an
> architecture-specific thing, because using a percpu variable might be
> good on some architectures but not others. So I get the feeling that
> it should be in the x86 __switch_to(), rather than in the generic
> code. I think it would fit very well with the per-cpu "old_rsp" and
> "current_task" updates that we already do.
> 
> > Adding TIF_NEED_RESCHED into the preempt count would allow a single test
> > in preempt_check_resched() instead of still needing the TI. Removing
> > PREEMPT_ACTIVE from preempt count should allow us to get rid of
> > ti::preempt_count altogether.
> >
> > The only problem with TIF_NEED_RESCHED is that its cross-cpu which would
> > make the entire thing atomic which would suck donkey balls so maybe we
> > need two separate per-cpu variables?
> 
> Agreed. Making it atomic would suck, and cancel all advantages of the
> better code generation to access it. Good point.
> 
> And yeah, it could be two variables in the same cacheline or something.
> 
>                  Linus
> --
> 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/
> 

--
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/