public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Martin Peschke <mp3@de.ibm.com>
To: Andi Kleen <ak@suse.de>
Cc: heiko.carstens@de.ibm.com, clg@fr.ibm.com, linux-kernel@vger.kernel.org
Subject: Re: [Patch] statistics infrastructure - update 9
Date: Mon, 10 Jul 2006 16:41:55 +0200	[thread overview]
Message-ID: <44B26733.6090904@de.ibm.com> (raw)
In-Reply-To: <200607061900.39406.ak@suse.de>

Andi Kleen wrote:
>> Good question. Btw. - faster by what order of magnitude?
> 
> pushf + popf is on K8 at least ~18 cycles, on P4 it is much more
> because they synchronize the pipeline there (hundreds of cycles)
> 
> cpu local add would be a few cycles at best and doesn't have
> any impact on the pipeline
> 
> 
>> local_irq_save/restore seems to be fine for kernel/profile.c
>>
>>
>> Reason 1:
>> cpu_local_* uses __get_cpu_var, which conflicts with struct statistic
>> being embedded into struct xyz that is allocated whenever the client
>> needs it.
>>
>> I could try to use local_t in conjunction with local_add etc.
>> (as seen in include/linux/dmaengine.h in 2.6.17-mm6).
>> Does this also yield a performance gain worth consideration?
> 
> Yes, but you would need preempt_disable() then. For non preemptible
> kernels (far majority) that would be already a big win.
> 
> 
>> So, removing local_irq_save/restore would require statistics to be
>> switched on and their buffers being available all the time. That is,
>> buffers holding counters etc. can't be allocated at run time - what
>> if allocation fails? (Should I leave this issue to clients?).
> 
> Can't you use RCU for this?
> 
> 
>> Reason 4:
>> The alleged overhead of local_irq_save/restore (as compared
>> to atomic operations) 
> 
> local_* doesn't need to be atomic. IT isn't on x86 at least.
> On some other architectures it can be, but i think it's just a SMOP
> of fixing them.
> 
> -Andi

Thanks. I am seriously considering these techniques.

If I manage to use RCU for most of the read-mostly struct statistic
and to push any other locking issues down into individual statistic
disciplines (utilisation indicator, histogram and so on), I should be
able to use local_t for some disciplines (particularyl counter and
histogram) without needing other locking primitives, like
local_irq_save/restore currently currently found in the code.

Not a change that can be done this afternoon, though.
And it will require careful review.

Martin


  reply	other threads:[~2006-07-10 14:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-03 16:24 [Patch] statistics infrastructure - update 9 Martin Peschke
2006-07-03 16:41 ` Cedric Le Goater
2006-07-04  0:17 ` Andi Kleen
2006-07-06 16:55   ` Martin Peschke
2006-07-06 17:00     ` Andi Kleen
2006-07-10 14:41       ` Martin Peschke [this message]
2006-07-04  6:17 ` Heiko Carstens
2006-07-04  7:19   ` Andrew Morton

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=44B26733.6090904@de.ibm.com \
    --to=mp3@de.ibm.com \
    --cc=ak@suse.de \
    --cc=clg@fr.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.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