linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Eric Dumazet <dada1@cosmosbay.com>
Cc: Theodore Tso <tytso@mit.edu>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux kernel <linux-kernel@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Mingming Cao <cmm@us.ibm.com>,
	linux-ext4@vger.kernel.org
Subject: Re: [PATCH] percpu_counter: Fix __percpu_counter_sum()
Date: Wed, 10 Dec 2008 08:12:42 +0100	[thread overview]
Message-ID: <1228893162.6978.25.camel@twins> (raw)
In-Reply-To: <493F4EF4.4080205@cosmosbay.com>

On Wed, 2008-12-10 at 06:09 +0100, Eric Dumazet wrote:
> Now percpu_counter_sum() is 'fixed', what about "percpu_counter_add()" ?
> 
> void __percpu_counter_add(struct percpu_counter *fbc, s64 amount, s32 batch)
> {
>         s64 count;
>         s32 *pcount;
>         int cpu = get_cpu();
> 
>         pcount = per_cpu_ptr(fbc->counters, cpu);
>         count = *pcount + amount;
>         if (count >= batch || count <= -batch) {
>                 spin_lock(&fbc->lock);
>                 fbc->count += count;
>                 *pcount = 0;
>                 spin_unlock(&fbc->lock);
>         } else {
>                 *pcount = count;
>         }
>         put_cpu();
> }
> 
> 
> If I read this well, this is not IRQ safe.
> 
> get_cpu() only disables preemption IMHO
> 
> For nr_files, nr_dentry, nr_inodes, it should not be a problem.
> 
> But for network counters (only in net-next-2.6) 
> and lib/proportions.c, we have a problem ?

Non of percpu_counter if irqsafe, for lib/proportions I disabled irqs by
hand when needed - I don't think we ought to bother with local_t, esp as
it basically sucks chunks on anything !x86.

  parent reply	other threads:[~2008-12-10  7:12 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <4936D287.6090206@cosmosbay.com>
     [not found] ` <4936EB04.8000609@cosmosbay.com>
2008-12-07  4:22   ` [PATCH] percpu_counter: Fix __percpu_counter_sum() Andrew Morton
2008-12-07 10:25     ` Peter Zijlstra
2008-12-07 13:28     ` Eric Dumazet
2008-12-07 17:28       ` Andrew Morton
2008-12-07 18:00         ` Eric Dumazet
2008-12-08  4:52           ` Andrew Morton
2008-12-08 22:12             ` Theodore Tso
2008-12-08 22:20               ` Peter Zijlstra
2008-12-08 23:00                 ` Theodore Tso
2008-12-08 23:05                   ` Peter Zijlstra
2008-12-08 23:08                     ` Peter Zijlstra
2008-12-09  8:12                     ` Eric Dumazet
2008-12-09  8:34                       ` Peter Zijlstra
2008-12-10  5:09                         ` Eric Dumazet
2008-12-10  5:49                           ` Andrew Morton
2008-12-10 22:56                             ` Eric Dumazet
2008-12-12  8:17                               ` Rusty Russell
2008-12-12  8:22                                 ` Eric Dumazet
2008-12-12 11:08                                 ` [PATCH] percpu_counter: use local_t and atomic_long_t if possible Eric Dumazet
2008-12-12 11:29                                   ` Peter Zijlstra
2008-12-23 11:43                                   ` Peter Zijlstra
2008-12-25 13:26                                     ` Rusty Russell
2008-12-15 12:53                             ` [PATCH] percpu_counter: Fix __percpu_counter_sum() Rusty Russell
2008-12-16 20:16                               ` Ingo Molnar
2008-12-10  7:12                           ` Peter Zijlstra [this message]
2008-12-08 23:07                   ` Andrew Morton
2008-12-08 23:49                     ` Theodore Tso
2008-12-08 22:22               ` Andrew Morton
2008-12-08 22:44               ` Mingming Cao
2008-12-07 22:24         ` [PATCH] atomic: fix a typo in atomic_long_xchg() Eric Dumazet
2008-12-07 15:28     ` [PATCH] percpu_counter: Fix __percpu_counter_sum() Theodore Tso
2008-12-08  4:42       ` Andrew Morton
2008-12-08 17:55         ` Mingming Cao
2008-12-11 16:32           ` [stable] " Greg KH
2008-12-08 17:44     ` Mingming Cao

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=1228893162.6978.25.camel@twins \
    --to=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=cmm@us.ibm.com \
    --cc=dada1@cosmosbay.com \
    --cc=davem@davemloft.net \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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).