[Devel] [PATCH rh7] mm/memcg: Fix race on mem cgroup offline/kmem deactivation.

[Andrey Ryabinin]

The following race could happen

  Initial state: kmem = 0, cgroup is online

             CPU0:
   charge_kmem(1); //kmem = 1
   memcg_uncharge_kmem()
   {
   ...
        kmem = page_counter_uncharge(&memcg->kmem, nr_pages); //kmem = 0

----------------------------------------------------------------------

             CPU1:

    charge_kmem(1); //kmem=1 again
    ....
    mem_cgroup_css_offline();
       memcg_deactivate_kmem() {
           ...
           css_get();
           memcg_kmem_mark_dead(memcg);
           if (page_counter_read(&memcg->kmem)) //kmem=1, thus return
                   return;
       }
----------------------------------------------------------------------

               CPU0:

        /* Not down to 0 */
        if (kmem)
                return;
        if (memcg_kmem_test_and_clear_dead(memcg))
                css_put(&memcg->css);              //kill memcg with kmem != 0

   }

Fix this by making a fake kmem charge on cgroup creation, and uncharging it
back during the offline stage. This way we guarantee that kmem is never 0
while cgroup is online, so the race above isn't possible anymore.

https://jira.sw.ru/browse/PSBM-98148
Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
Suggested-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
 mm/memcontrol.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4846e7a9bf63..08aa00e0661a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4794,6 +4794,43 @@ static int __memcg_activate_kmem(struct mem_cgroup *memcg,
 	memcg->kmemcg_id = memcg_id;
 	set_bit(KMEM_ACCOUNTED_ACTIVE, &memcg->kmem_account_flags);
 	set_bit(KMEM_ACCOUNTED_ACTIVATED, &memcg->kmem_account_flags);
+
+	/*
+	 * Fake charge to keep kmem > 0 while cgroup is online. This prevents
+	 * premature cgroup destruction due to the following race:
+	 * CPU0:
+	 *       charge_kmem(1); //kmem = 1
+	 *       memcg_uncharge_kmem()
+	 *       {
+	 *       ...
+	 *            kmem = page_counter_uncharge(&memcg->kmem, nr_pages); //kmem = 0
+	 *
+	 *    ----------------------------------------------------------------------
+	 *
+	 *                 CPU1:
+	 *
+	 *        charge_kmem(1); //kmem=1 again
+	 *        ....
+	 *        mem_cgroup_css_offline();
+	 *           memcg_deactivate_kmem() {
+	 *               ...
+	 *               css_get();
+	 *               memcg_kmem_mark_dead(memcg);
+	 *               if (page_counter_read(&memcg->kmem)) //kmem=1, thus return
+	 *                       return;
+	 *           }
+	 *    ----------------------------------------------------------------------
+	 *
+	 *                   CPU0:
+	 *
+	 *            if (kmem)
+	 *                    return;
+	 *            if (memcg_kmem_test_and_clear_dead(memcg))
+	 *                    css_put(&memcg->css);              //kill memcg with kmem != 0
+	 *
+	 *       }
+	 */
+	page_counter_charge(&memcg->kmem, 1);
 out:
 	return err;
 }
@@ -6112,7 +6149,8 @@ static void memcg_deactivate_kmem(struct mem_cgroup *memcg)
 
 	memcg_kmem_mark_dead(memcg);
 
-	if (page_counter_read(&memcg->kmem))
+	/* Uncharge fake charge from __memcg_activate_kmem() */
+	if (page_counter_uncharge(&memcg->kmem, 1))
 		return;
 
 	/*
-- 
2.26.2