Messages in this thread | | | Date | Thu, 14 Apr 2011 04:04:11 +0900 | From | Tejun Heo <> | Subject | Re: [patch v2 3/4] percpu_counter: fix code for 32bit systems |
| |
On Wed, Apr 13, 2011 at 03:57:18PM +0800, shaohua.li@intel.com wrote: > static inline s64 percpu_counter_read(struct percpu_counter *fbc) > { > +#if BITS_PER_LONG == 32 > + s64 count; > + unsigned long flags; > + spin_lock_irqsave(&fbc->lock, flags); > + count = fbc->count; > + spin_unlock_irqrestore(&fbc->lock, flags); > + return count; > +#else > return fbc->count; > +#endif
I don't think this is safe. The possible deadlock scenario was percpu_counter_read() being called from irq context and adding irq locking to percpu_counter_read() doesn't change that in any way. You should be changing locking in other places. Given that the next patch would make this dancing with locks all pointless, my suggestion is to drop this patch and proceed with atomic64_t conversion directly and note that the conversion also removes possible 32bit deviation on 32bit archs.
Thanks.
-- tejun
| |