On 03.03.2020 18:48, Andrey Ryabinin wrote:
> Instead of trylock'ing the page, use the lru_lock to prevent the
> racing against mem_cgroup_migrate(). So the memcg_move_account() doesn't
> depend on hung IO.
> 
> The trylock seems to be needed to prevent mem_cgroup_migrate() to see
> disbalanced pc->mem_cgroup && pc->flags state. We could achieve the same
> by using lru_lock instead. memcg_move_account() only works with isolated
> from LRU pages, so it can race only with mem_cgroup_migrate() called
> with lrucare=true, otherwise the page is not on LRU so the 
> memcg_move_account()
> can't see it.
> 
> https://jira.sw.ru/browse/PSBM-101639
> https://jira.sw.ru/browse/PSBM-101757
> https://jira.sw.ru/browse/PSBM-94117
> Signed-off-by: Andrey Ryabinin <aryabi...@virtuozzo.com>
> ---
>  mm/memcontrol.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f4629123e7fb..bfb5bbdf9ecb 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3939,7 +3939,7 @@ static int mem_cgroup_move_account(struct page *page,
>                                  struct mem_cgroup *to)
>  {
>       unsigned long flags;
> -     int ret;
> +     int ret, isolated;
>  
>       VM_BUG_ON(from == to);
>       VM_BUG_ON_PAGE(PageLRU(page), page);
> @@ -3958,8 +3958,7 @@ static int mem_cgroup_move_account(struct page *page,
>        * of its source page while we change it: page migration takes
>        * both pages off the LRU, but page cache replacement doesn't.
>        */
> -     if (!trylock_page(page))
> -             goto out;
> +     lock_page_lru(page, &isolated);

Does this race with try_get_mem_cgroup_from_page()?
  
>       ret = -EINVAL;
>       if (!PageCgroupUsed(pc) || pc->mem_cgroup != from)
> @@ -3992,7 +3991,7 @@ static int mem_cgroup_move_account(struct page *page,
>       memcg_check_events(from, page);
>       local_irq_restore(flags);
>  out_unlock:
> -     unlock_page(page);
> +     unlock_page_lru(page, isolated);
>  out:
>       return ret;
>  }
> @@ -7754,6 +7753,7 @@ void mem_cgroup_migrate(struct page *oldpage, struct 
> page *newpage,
>  {
>       unsigned int nr_pages = 1;
>       struct page_cgroup *pc;
> +     struct mem_cgroup *memcg;
>       int isolated;
>  
>       VM_BUG_ON_PAGE(!PageLocked(oldpage), oldpage);
> @@ -7787,17 +7787,18 @@ void mem_cgroup_migrate(struct page *oldpage, struct 
> page *newpage,
>       if (lrucare)
>               lock_page_lru(oldpage, &isolated);
>  
> +     memcg = READ_ONCE(pc->mem_cgroup);
>       pc->flags = 0;
>  
>       if (lrucare)
>               unlock_page_lru(oldpage, isolated);
>  
>       local_irq_disable();
> -     mem_cgroup_charge_statistics(pc->mem_cgroup, oldpage, -nr_pages);
> -     memcg_check_events(pc->mem_cgroup, oldpage);
> +     mem_cgroup_charge_statistics(memcg, oldpage, -nr_pages);
> +     memcg_check_events(memcg, oldpage);
>       local_irq_enable();
>  
> -     commit_charge(newpage, pc->mem_cgroup, nr_pages, lrucare);
> +     commit_charge(newpage, memcg, nr_pages, lrucare);
>  }
>  
>  static int __init cgroup_memory(char *s)
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to