On Tue 21-10-14 14:19:10, Johannes Weiner wrote:
> 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API") changed page
> migration to uncharge the old page right away. The page is locked,
> unmapped, truncated, and off the LRU. But it could race with a
> finishing writeback, which then doesn't get unaccounted properly:
>
> test_clear_page_writeback() migration
> acquire pc->mem_cgroup->move_lock
> wait_on_page_writeback()
> TestClearPageWriteback()
> mem_cgroup_migrate()
> clear PCG_USED
> if (PageCgroupUsed(pc))
> decrease memcg pages under writeback
> release pc->mem_cgroup->move_lock
>
> One solution for this would be to simply remove the PageCgroupUsed()
> check, as RCU protects the memcg anyway.
>
> However, it's more robust to acknowledge that migration is really
> modifying the charge state of alive pages in this case, and so it
> should participate in the protocol specifically designed for this.
It's been a long day so I might be missing something really obvious
here. But how can move_lock help here when the fast path (no task
migration is going on) takes only RCU read lock?
> Fixes: 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API")
> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: "3.17" <stable@xxxxxxxxxxxxxxx>
> ---
> mm/memcontrol.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 3a203c7ec6c7..b35a44e9cd37 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -6148,6 +6148,7 @@ void mem_cgroup_migrate(struct page *oldpage, struct page *newpage,
> bool lrucare)
> {
> struct page_cgroup *pc;
> + unsigned long flags;
> int isolated;
>
> VM_BUG_ON_PAGE(!PageLocked(oldpage), oldpage);
> @@ -6177,7 +6178,14 @@ void mem_cgroup_migrate(struct page *oldpage, struct page *newpage,
> if (lrucare)
> lock_page_lru(oldpage, &isolated);
>
> + /*
> + * The page is locked, unmapped, truncated, and off the LRU,
> + * but there might still be references, e.g. from finishing
> + * writeback. Follow the charge moving protocol here.
> + */
> + move_lock_mem_cgroup(pc->mem_cgroup, &flags);
> pc->flags = 0;
> + move_unlock_mem_cgroup(pc->mem_cgroup, &flags);
>
> if (lrucare)
> unlock_page_lru(oldpage, isolated);
> --
> 2.1.2
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
