All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Christoph Lameter <cl@gentwo.org>,
	Valdis.Kletnieks@vt.edu, linux-kernel@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC][PATCH 0/5] Introduce checks for preemptable code for this_cpu_read/write()
Date: Tue, 20 Sep 2011 14:34:56 -0400	[thread overview]
Message-ID: <20110920183455.GA12919@Krystal> (raw)
In-Reply-To: <1316543244.29966.118.camel@gandalf.stny.rr.com>

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Tue, 2011-09-20 at 14:12 -0400, Mathieu Desnoyers wrote:
> > Not quite. What I was proposing more precisely:
> > 
> > - this_cpu_*() for the case where the caller needs to disable
> >   preemption. This is the default case. This is exactly what you
> >   proposed, with WARN_ON debug checks. This could even be "percpu_*()"
> >   now that I think of it. There is no real point in the "this_cpu"
> >   prefix.
> > 
> > - preempt_protected_percpu_*() and irq_protected_percpu_*() for
> >   statistics/slub use. Those primitives disable preemption or irq
> >   internally on non-x86 architectures. The caller of these primitives
> >   is not required to disable preemption nor irqs.
> 
> This is totally confusing. It suggests to me that the percpu requires
> preemption protected. You are coupling the implementation of the
> function too much with the name. The name should describe its use. What
> does "preempt_protected" mean?  To me, it sounds like I should use this
> in preempt protected mode. Still way too confusing.
> 
> any_cpu_*() is still much more understanding. It means that we are
> manipulating a CPU variable, and we do not care which one.
> 
> Looking at the real use cases of this_cpu(), that seems to be exactly
> the use case for it. That is, we modify the cpu variable, maybe we get
> migrated, but in the end, we just read all the cpu variables and report
> the net sum. Thus the design POV is that we do not care what CPU
> variable we read/write. From an implementation point of view, it just
> happens to be an optimization that we try to read/write to the current
> cpu pointer. But in reality it doesn't matter what CPU variable we
> touch.
> 
> Do not confuse implementation and optimizations with design. The big
> picture design is that we do not care what CPU variable is touched. The
> name should reflect that.

Yep, understood. We might want to consider percpu_*() for the case where
the caller must disable preemption, and any_percpu_*() for the case
where we don't care on which cpu we actually are. These are all touching
per-cpu variables after all. But still, it does not take into account
the "irqsafe" vs "preemptsafe" cases. So maybe irqsafe_any_percpu_*()
and preemptsafe_any_percpu_*() would do it ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2011-09-20 18:35 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-19 21:20 [RFC][PATCH 0/5] Introduce checks for preemptable code for this_cpu_read/write() Steven Rostedt
2011-09-19 21:20 ` [RFC][PATCH 1/5] x86: Remove const_udelay() caring about which cpu var it uses Steven Rostedt
2011-09-19 21:51   ` Christoph Lameter
2011-09-19 23:31     ` Steven Rostedt
2011-09-19 21:20 ` [RFC][PATCH 2/5] mm: Switch mod_state() to __this_cpu_read() Steven Rostedt
2011-09-19 22:02   ` Christoph Lameter
2011-09-19 23:48     ` Steven Rostedt
2011-09-20 14:46       ` Christoph Lameter
2011-09-20 15:16         ` Steven Rostedt
2011-09-20 15:54           ` Christoph Lameter
2011-09-20 16:07             ` Steven Rostedt
2011-09-20 22:19             ` Valdis.Kletnieks
2011-09-20 13:49     ` Thomas Gleixner
2011-09-20 14:01       ` Steven Rostedt
2011-09-20 14:51       ` Christoph Lameter
2011-09-20 15:11         ` Steven Rostedt
2011-09-20 15:59           ` Christoph Lameter
2011-09-20 16:03             ` Steven Rostedt
2011-09-20 16:07               ` Christoph Lameter
2011-09-20 15:27         ` Thomas Gleixner
2011-09-20 16:02           ` Christoph Lameter
2011-09-20 16:51             ` Thomas Gleixner
2011-09-20 17:08               ` Steven Rostedt
2011-09-19 21:20 ` [RFC][PATCH 3/5] memcg: Disable preemption in memcg_check_events() Steven Rostedt
2011-09-20 14:20   ` Johannes Weiner
2011-09-20 14:24     ` Johannes Weiner
2011-09-20 14:33       ` Steven Rostedt
2011-09-24  0:46   ` Steven Rostedt
2011-09-19 21:20 ` [RFC][PATCH 4/5] printk: Have wake_up_klogd() use __this_cpu_write() Steven Rostedt
2011-09-19 21:54   ` Christoph Lameter
2011-09-19 23:33     ` Steven Rostedt
2011-09-20 14:54       ` Christoph Lameter
2011-09-20 14:55         ` Peter Zijlstra
2011-09-19 21:20 ` [RFC][PATCH 5/5] percpu: Add preempt checks back into this_cpu_read/write() Steven Rostedt
2011-09-19 21:49 ` [RFC][PATCH 0/5] Introduce checks for preemptable code for this_cpu_read/write() Christoph Lameter
2011-09-20  3:06   ` Steven Rostedt
2011-09-20 12:44     ` Valdis.Kletnieks
2011-09-20 13:51       ` Thomas Gleixner
2011-09-20 14:58         ` Christoph Lameter
2011-09-20 15:17           ` Steven Rostedt
2011-09-20 14:57       ` Christoph Lameter
2011-09-20 15:19         ` Steven Rostedt
2011-09-20 16:08           ` Christoph Lameter
2011-09-20 16:31             ` Steven Rostedt
2011-09-20 16:56               ` Steven Rostedt
2011-09-20 17:09                 ` Peter Zijlstra
2011-09-20 17:15                   ` Steven Rostedt
2011-09-20 17:25                     ` Mathieu Desnoyers
2011-09-20 18:03                       ` Steven Rostedt
2011-09-20 18:12                         ` Mathieu Desnoyers
2011-09-20 18:27                           ` Steven Rostedt
2011-09-20 18:34                             ` Mathieu Desnoyers [this message]
2011-09-20 22:32             ` Valdis.Kletnieks
2011-09-20 22:17           ` Valdis.Kletnieks
2011-09-21  1:33             ` Steven Rostedt
2011-09-20 15:46     ` Mathieu Desnoyers
2011-09-20 16:00       ` Steven Rostedt
2011-09-20 16:10         ` Christoph Lameter
2011-09-20 16:50           ` Peter Zijlstra
2011-09-20 18:54           ` Steven Rostedt
2011-09-21 15:16             ` Christoph Lameter
2011-09-21 15:31               ` Steven Rostedt
2011-09-21 15:59                 ` Christoph Lameter
2011-09-21 16:12                   ` Steven Rostedt
2011-09-21 16:32               ` Thomas Gleixner
2011-09-20  2:20 ` Andi Kleen
2011-09-20  3:12   ` Steven Rostedt
2011-09-20  3:17     ` Steven Rostedt
2011-09-20  8:32     ` Thomas Gleixner
2011-09-20 12:10       ` Steven Rostedt
2011-09-20 15:03       ` Christoph Lameter
2011-09-20 15:07         ` Peter Zijlstra
2011-09-20 16:05           ` 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=20110920183455.GA12919@Krystal \
    --to=mathieu.desnoyers@efficios.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.