Re: [PATCH 11/51] memcg: implement mem_cgroup_css_from_page()

[Johannes Weiner]

On Sun, May 24, 2015 at 05:24:40PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Fri, May 22, 2015 at 07:28:31PM -0400, Johannes Weiner wrote:
> > replace_page_cache() can clear page->mem_cgroup even when the page
> > still has references, so unfortunately you must hold the page lock
> > when calling this function.
> > 
> > I haven't checked how you use this - chances are you always have the
> > page locked anyways - but it probably needs a comment.
> 
> Hmmm... as replace_page_cache_page() is used only by fuse and fuse's
> bdi doesn't go through the usual writeback accounting which is
> necessary for cgroup writeback support anyway, so I don't think this
> presents an actual problem.  I'll add a warning in
> replace_page_cache_page() which triggers when it's used on a bdi which
> has cgroup writeback enabled and add comments explaining what's going
> on.

Okay, so that's no problem then as long as it's documented.

In the long term, it would probably still be a good idea to restore
the invariant that page->mem_cgroup never changes on live pages.  For
the old interface that ship has sailed as live pages can move around
different cgroups; in unified hierarchy, however, we currently only
move charges when migrating pages between page frames.  That can be
switched to duplicating the charge instead and leaving the old page
alone until the final put - which is expected to occur soon after.

Accounting the same page twice for a short period during migration
should be an acceptable trade-off when considering how much simpler it
makes the synchronization rules.  We only have to make sure to clearly
mark interfaces and functions that are only safe for use with unified
hierarchy code.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/