linux-kernel - Re: [GIT PULL] RCU changes for v3.4

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]

Date:	Thu, 22 Mar 2012 18:08:01 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	paulmck@...ux.vnet.ibm.com
Cc:	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] RCU changes for v3.4

On Mon, Mar 19, 2012 at 8:33 PM, Paul E. McKenney
<paulmck@...ux.vnet.ibm.com> wrote:
>
> If it would help, I would be happy to put together an itemized list.
> I will of course do so for the next merge window.

Ok. I would like to get an itemized list next time, now I ended up
re-doing the merges with the one Ingo sent me.

However, looking at the current state of RCU, the thing I would
*really* like to *finally* be fixed is that total disaster called
__rcu_read_lock() (and to a lesser degree __rcu_read_unlock).

Why do I call it a total disaster?

Simple: there are two versions of that function (not counting the
inlined trivial non-preempt version that just disables preemption),
AND THEY ARE BOTH IDENTICAL.

More importantly, they are both IDENTICALLY BAD.

They are crap because:

 - they shouldn't be out-of-line to begin with.

   Doing a function call for these things is stupid. It's  not like
it's even "oh, there are two versions of it, so the linker picks one
over the other". Sure, there are two versions of it, but they are the
same stupid code just duplicated.

 - the rcu counter should be a per-cpu counter, not a per-thread one

Right now that function ends up being two instructions:

        mov    %gs:0xb700,%rax
        incl   0x100(%rax)

and dammit, using a function call to do that is pretty much
borderline. But it should be *one* instruction that just increments
the percpu variable:

        incl %gs:rcu_read_lock_nesting

and it shouldn't be out-of-line.

Because wouldn't it be nice to just make the scheduler save/restore
the rcu read lock depth for the rcu preemption case.

Yeah, we should do the same thing with the preempt count. It shouldn't
be in the thread structure, it should be per-cpu.

Please? Every time I look at some profiles, that silly rcu_read_lock
is there in the profile. It's annoying. I'd rather see it in the
function that invokes it.

                      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/