linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Pekka Enberg <penberg@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [GIT PULL] slab fixes for 3.2-rc4
Date: Tue, 20 Dec 2011 08:23:15 -0800	[thread overview]
Message-ID: <20111220162315.GC10752@google.com> (raw)
In-Reply-To: <CAOJsxLEHT4D4rfMvWe5iPFpunOaLWZD9fJxbOJ6_qMwt_rd7eg@mail.gmail.com>

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

  reply	other threads:[~2011-12-20 16:23 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <alpine.LFD.2.02.1111292001420.4019@tux.localdomain>
2011-11-29 19:29 ` [GIT PULL] slab fixes for 3.2-rc4 Linus Torvalds
2011-11-29 19:38   ` Linus Torvalds
2011-11-29 19:38     ` Linus Torvalds
2011-12-20  9:47   ` Pekka Enberg
2011-12-20 16:23     ` Tejun Heo [this message]
2011-12-20 16:31       ` Christoph Lameter
2011-12-20 16:31         ` Christoph Lameter
2011-12-20 19:28       ` Linus Torvalds
2011-12-20 20:28         ` Tejun Heo
2011-12-21  8:08           ` Pekka Enberg
2011-12-21 17:09             ` Tejun Heo
2011-12-21 15:16           ` Christoph Lameter
2011-12-21 17:05             ` Tejun Heo
2011-12-22  2:19               ` Linus Torvalds
2011-12-22 16:05                 ` Tejun Heo
2011-12-28 10:25                 ` Benjamin Herrenschmidt
2011-12-22 14:58               ` Christoph Lameter
2011-12-22 14:58                 ` Christoph Lameter
2011-12-22 16:08                 ` Tejun Heo
2011-12-22 17:58                   ` Christoph Lameter
2011-12-22 18:03                     ` Ingo Molnar
2011-12-22 18:31                     ` Linus Torvalds
2011-12-23 16:55                       ` Christoph Lameter
2011-12-23 20:54                         ` Linus Torvalds
2012-01-04 15:30                           ` Christoph Lameter
2012-01-04 16:07                             ` Linus Torvalds
2012-01-04 17:00                               ` Christoph Lameter
2012-01-04 23:10                                 ` Linus Torvalds
2012-01-04 23:10                                   ` Linus Torvalds
2012-01-05 19:15                                   ` Christoph Lameter
2012-01-05 19:27                                     ` Linus Torvalds
2011-12-22 18:47                     ` Tejun Heo
2011-12-20 16:26     ` Christoph Lameter
2011-12-20 16:26       ` Christoph Lameter
2011-12-21  8:06       ` Pekka Enberg
2011-12-21 15:20         ` Christoph Lameter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111220162315.GC10752@google.com \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=penberg@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).