linux-kernel - Re: [GIT PULL] slab fixes for 3.2-rc4

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: <CA+55aFzCY7TyEgkWFn6wxUMjBBaKGcx0hdV6gTdta5u8eK=Ymg@mail.gmail.com>
Date:	Fri, 23 Dec 2011 12:54:12 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Christoph Lameter <cl@...ux.com>
Cc:	Tejun Heo <tj@...nel.org>, Pekka Enberg <penberg@...nel.org>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [GIT PULL] slab fixes for 3.2-rc4

On Fri, Dec 23, 2011 at 8:55 AM, Christoph Lameter <cl@...ux.com> wrote:
>
> There is an #ifndef CONFIG_M386 around the implementation of these for
> x86. Use on i386 will not generate an xadd instructions but fallback to a
> generic implementation.

Ok, good. It's not the really right thing to do, but it will at least work.

> The add_return stuff was already available with the earlier per cpu apis
> (local_t and percpu) that this_cpu replaces and is still available through
> the atomic operations.

Sure. The "atomic_op_return()" form actually *makes*sense*. It has
atomicity guarantees, and the return value is meaningful.

The same IS SIMPLY NOT TRUE of the "this_cpu" versions.

> If one wants to exploit the per cpu atomicity then one may want to know
> what the result was.

No. You're blathering and doing "magical thinking" instead of actually
thinking things through.

Think about the semantics. What does it mean to get a result of the op
on a percpu variable?

You have two cases:

Case 1: you actually know what CPU you are on, and the number may have
some meaning. But in that case you are also *pinned* to that CPU, and
there is no difference between

   x = percpu_value;
   x += y;
   percpu_value = x;
   .. look at 'x' .. to see what you did

and

    percpu_value += x;
   ..look at percpu_value to see what you did ..

and

    x = xadd percpu_value,y

so there are no "atomicity advantages" - xadd is the same as
open-coding it. You are pinned to the CPU, all three ways are 100%
equivalent.

The other case is:

Case 2: you're not pinned to the CPU, and you just do statistics and
add some value to memory. BUT NOW THE RESULT HAS ABSOLUTELY NO
MEANING!

Now "xadd" would possibly give different results from "add and read
the result", since you migth be moving around between cpu's, but there
is absolutely no way you could ever care, sine the value you read back
is meaningless regardless of whether it's the return value of xadd, or
the value from some other CPU. There are other cpu's with the same
percpu variable, and they are all equivalent as far as you are
concerned, because you're not pinned to one of them.

See? xadd buys you *nothing* from a semantic standpoint. And anybody
who *thinks* it buys you something is just confused and just
introduced a bug.

So the "thiscpu_op_return" functions are broken-by-design! They have
no meaning. They are completely crazy.

That's why they should be removed. Anybody who thinks they add value
has not thought through what they actually do or mean.

                            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/