linux-kernel - Re: [PATCH 5/6] net: __this_cpu

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: <00000141c1cee440-28c489c7-4335-4f03-933f-138440dbf427-000000@email.amazonses.com>
Date:	Wed, 16 Oct 2013 15:07:43 +0000
From:	Christoph Lameter <cl@...ux.com>
To:	Peter Zijlstra <peterz@...radead.org>
cc:	Eric Dumazet <eric.dumazet@...il.com>, Tejun Heo <tj@...nel.org>,
	akpm@...uxfoundation.org, rostedt@...dmis.org,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH 5/6] net: __this_cpu_inc in route.c

On Wed, 16 Oct 2013, Peter Zijlstra wrote:

> For x86 its fairly easy to see its correct this way; but for load-store
> archs this isn't immediately obvious to me.
>
> Suppose; r1 contains our per-cpu pointer:
>
> 	LOAD  r2, per-cpu-base
> 	ADD   r1, r2
> 	LOAD  r2, $(r1) # load value
> 	INC   r2
> 	STORE $(r1), r2 # store value
>
> If such a thing is done without preempt disable; we could be
> preempted/migrated at any place along that chain. In that case the STORE
> could be to another CPUs memory (we get migrated near the INC) and could
> conflict with a per-cpu operation on that CPU corrupting state.
>
> If I look at percpu.h; the generic __this_cpu versions look like they
> generate the above for such archs.

Yes they do.

> In that case; I don't see how even for statistics (where we really don't
> care what cpu the op happens on, as long as it happens to a cpu,
> coherently) it is correct to use the raw_this_cpu stuff without
> preemption protection.
>
> In fact; I think the comment near __this_cpu_read actually alludes to
> this.

Indeed. This is well known. For some statistics this has been
judged to be acceptable in the past when the alternative was disabling
interrupts since the overhead was so much higher and the statistics
operations had to be used in very performance critical sections.

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