lkml.org 
[lkml]   [2014]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] mm: memcontrol: fix missed end-writeback accounting
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@cmpxchg.org>
> Cc: "3.17" <stable@vger.kernel.org>
> ---
> 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


\
 
 \ /
  Last update: 2014-10-22 20:01    [W:0.684 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site