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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
