On Tue, 6 Sep 2011 11:58:52 +0200 Johannes Weiner <jweiner@xxxxxxxxxx> wrote: > On Wed, Aug 17, 2011 at 11:50:53PM -0700, Greg Thelen wrote: > > Both mem_cgroup_charge_statistics() and mem_cgroup_move_account() were > > unnecessarily disabling preemption when adjusting per-cpu counters: > > preempt_disable() > > __this_cpu_xxx() > > __this_cpu_yyy() > > preempt_enable() > > > > This change does not disable preemption and thus CPU switch is possible > > within these routines. This does not cause a problem because the total > > of all cpu counters is summed when reporting stats. Now both > > mem_cgroup_charge_statistics() and mem_cgroup_move_account() look like: > > this_cpu_xxx() > > this_cpu_yyy() > > > > Reported-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> > > Signed-off-by: Greg Thelen <gthelen@xxxxxxxxxx> > > I just noticed that both cases have preemption disabled anyway because > of the page_cgroup bit spinlock. > > So removing the preempt_disable() is fine but we can even keep the > non-atomic __this_cpu operations. > > Something like this instead? > > --- > From: Johannes Weiner <jweiner@xxxxxxxxxx> > Subject: mm: memcg: remove needless recursive preemption disabling > > Callsites of mem_cgroup_charge_statistics() hold the page_cgroup bit > spinlock, which implies disabled preemption. > > The same goes for the explicit preemption disabling to account mapped > file pages in mem_cgroup_move_account(). > > The explicit disabling of preemption in both cases is redundant. > > Signed-off-by: Johannes Weiner <jweiner@xxxxxxxxxx> Could you add comments as "This operation is called under bit spin lock !" ? Nice catch. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hioryu@xxxxxxxxxxxxxx> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>