[PATCH] memcg: unlock page before charging it. (WasRe: [PATCH V2] mm: Do not keep page locked during page fault while charging it for memcg

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Wed, 22 Jun 2011 14:32:04 +0200
Michal Hocko <mhocko@suse.cz> wrote:

> On Wed 22-06-11 08:15:16, Christoph Hellwig wrote:
> > > +
> > > +			/* We have to drop the page lock here because memcg
> > > +			 * charging might block for unbound time if memcg oom
> > > +			 * killer is disabled.
> > > +			 */
> > > +			unlock_page(vmf.page);
> > > +			ret = mem_cgroup_newpage_charge(page, mm, GFP_KERNEL);
> > > +			lock_page(vmf.page);
> > 
> > This introduces a completely poinless unlock/lock cycle for non-memcg
> > pagefaults.  Please make sure it only happens when actually needed.
> 
> Fair point. Thanks!
> What about the following?
> I realize that pushing more memcg logic into mm/memory.c is not nice but
> I found it better than pushing the old page into mem_cgroup_newpage_charge.
> We could also check whether the old page is in the root cgroup because
> memcg oom killer is not active there but that would add more code into
> this hot path so I guess it is not worth it.
> 
> Changes since v1
> - do not unlock page when memory controller is disabled.
> 

Great work. Then I confirmed Lutz' problem is fixed.

But I like following style rather than additional lock/unlock.
How do you think ? I tested this on the latest git tree and confirmed
the Lutz's livelock problem is fixed. And I think this should go stable tree.


==