Messages in this thread | | | Date | Wed, 8 Jan 2014 12:30:47 +0800 | Subject | Re: [PATCH] lib/percpu_counter.c: disable local irq when updating percpu couter | From | Ming Lei <> |
| |
On Wed, Jan 8, 2014 at 11:29 AM, Ming Lei <tom.leiming@gmail.com> wrote: > Hi Andrew, > > On Wed, Jan 8, 2014 at 9:36 AM, Andrew Morton <akpm@linux-foundation.org> wrote: >>> I am wondering if the above patch is more efficient, because: >>> >>> - raw_local_irq_save()/raw_local_irq_restore() should be cheaper >>> than preempt_enable() in theory >> >> Don't think so - local_irq_disable() requires quite some internal >> synchronization in the CPU and is expensive. preempt_disable() is just > > Yes, it might be a little expensive on some CPUs, but should be > arch-dependent(CPU inside things are involved) > >> an add+barrier, minus the add if the kernel is non-preemptable. > > IMO, generally, from software view, local_irq_save() only save the > CPU's interrupt mask to the local variable 'flag', and sets irq mask > to register, considered local variable can be thought to be in cache, > so I think it might be cheaper than preempt_enable() because > preempt counter may not be in cache. > > Also this_cpu_add() won't work in batch path(slow path), we still > need to avoid interrupt coming between reading the percpu counter > and resetting it, otherwise counts might be lost too, :-)
Sorry, I miss the __this_cpu_sub() in slow path, so it is correct, and even preempt_enable() and preempt_disable() can be removed.
Thanks, -- Ming Lei
| |