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: <20111220162315.GC10752@google.com>
Date:	Tue, 20 Dec 2011 08:23:15 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Pekka Enberg <penberg@...nel.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux.com>, 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

Hello, Pekka.

On Tue, Dec 20, 2011 at 11:47:26AM +0200, Pekka Enberg wrote:
> So, I actually looked into doing something like this and wasn't
> actually able to understand the purpose of the various percpu
> variants. It seems rather obvious that we can just drop the
> non-irqsafe cmpxchg() variant but what about the rest of the percpu
> ops? Why do we have preempt safe, irqsafe, and unsafe variants? How
> are they supposed to be used?
> 
> To illustrate the issue, for "per cpu add" we have:
> 
> __this_cpu_add()
> this_cpu_add()
> irqsafe_cpu_add()

Description for 7340a0b152 "this_cpu: Introduce this_cpu_ptr() and
generic this_cpu_* operations" should explain the above three.  In
short,

  __this_cpu_add()	: synchronization is caller's responsibility
  this_cpu_add()	: protected against preemption
  irqsafe_cpu_add()	: protected against irq

> percpu_add()

This is __this_cpu_add() + preemption disabled check.  Should be
removed.  Christoph, is there a use case where __this_cpu_XXX() is
used without preemption disabled?  Why doesn't it have preemption
check?

> Why do we need all of them?

It would great if we can drop the preempt safe one.  For x86, it
doesn't make any difference.  For archs which can't do it in single
instruction && irq on/off is expensive, it can be bad.  I don't know
how bad tho.

percpu API needs to be cleaned up.  There are quite a few duplicates -
some are from the days when static and dynamic percpu memories were
different, some got added during the this_cpu_*() stuff.  It has been
on the todo list for a while now but I never got around to do it.

If I'm not missing something, all we need are,

* per_cpu_ptr()

* get_cpu_var(), put_cpu_var() - it would be more consistent if
  they're get_cpu_ptr() and put_cpu_ptr().

* [__]this_cpu_ptr()

* Hopefully, smaller subset of this_cpu_XXX() ops.

Thanks.

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