Re: [patch] Avoid use of spinlock for percpu_counter

[Eric Dumazet <dada1@cosmosbay.com>]

Ravikiran G Thirumalai a écrit :
> The spinlock in struct percpu_counter protects just one counter.  It's
> not obvious why it was done this way (I am guessing it was because earlier,
> atomic_t was guaranteed 24 bits only on some arches).  Since we have
> atomic_long_t now, I don't see why this cannot be replaced with an atomic_t.
> 
> Comments?

Yes this makes sense.

Furthermore, we could try to fix 'struct percpu_counter' management (if SMP) 
if alloc_percpu(long) call done in percpu_counter_init() fails. This is 
currently ignored and can crash.

Something like (hybrid patch, to get the idea) :

--- a/mm/swap.c 2006-01-26 15:58:42.000000000 +0100
+++ b/mm/swap.c 2006-01-26 16:00:54.000000000 +0100
@@ -472,9 +472,12 @@
  {
         long count;
         long *pcount;
-       int cpu = get_cpu();

-       pcount = per_cpu_ptr(fbc->counters, cpu);
+       if (unlikely(fbc->counters == NULL)) {
+               atomic_long_add(amount, &fbc->count);
+               return;
+       }
+       pcount = per_cpu_ptr(fbc->counters, get_cpu());
         count = *pcount + amount;
         if (count >= FBC_BATCH || count <= -FBC_BATCH) {
                 atomic_long_add(count, &fbc->count);
--- a/include/linux/percpu_counter.h    2006-01-26 16:02:31.000000000 +0100
+++ b/include/linux/percpu_counter.h    2006-01-26 16:02:53.000000000 +0100
@@ -35,7 +35,8 @@

  static inline void percpu_counter_destroy(struct percpu_counter *fbc)
  {
-       free_percpu(fbc->counters);
+       if (fbc->counters)
+               free_percpu(fbc->counters);
  }

  void percpu_counter_mod(struct percpu_counter *fbc, long amount);


Eric