mm/vmscan.c: add checks for incorrect handling of current->reclaim_state
authorAndrew Morton <akpm@linux-foundation.org>
Tue, 16 Jul 2019 23:26:15 +0000 (16:26 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 17 Jul 2019 02:23:21 +0000 (19:23 -0700)
Six sites are presently altering current->reclaim_state.  There is a
risk that one function stomps on a caller's value.  Use a helper
function to catch such errors.

Cc: Yafang Shao <laoar.shao@gmail.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
mm/vmscan.c

index 88d740db3216bc0bdc97983ff94863997df82aa6..44df66a98f2adbbd2c18c38dec8eed9e45b4a6ce 100644 (file)
@@ -241,6 +241,18 @@ static void unregister_memcg_shrinker(struct shrinker *shrinker)
 }
 #endif /* CONFIG_MEMCG_KMEM */
 
+static void set_task_reclaim_state(struct task_struct *task,
+                                  struct reclaim_state *rs)
+{
+       /* Check for an overwrite */
+       WARN_ON_ONCE(rs && task->reclaim_state);
+
+       /* Check for the nulling of an already-nulled member */
+       WARN_ON_ONCE(!rs && !task->reclaim_state);
+
+       task->reclaim_state = rs;
+}
+
 #ifdef CONFIG_MEMCG
 static bool global_reclaim(struct scan_control *sc)
 {
@@ -3194,13 +3206,13 @@ unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
        if (throttle_direct_reclaim(sc.gfp_mask, zonelist, nodemask))
                return 1;
 
-       current->reclaim_state = &sc.reclaim_state;
+       set_task_reclaim_state(current, &sc.reclaim_state);
        trace_mm_vmscan_direct_reclaim_begin(order, sc.gfp_mask);
 
        nr_reclaimed = do_try_to_free_pages(zonelist, &sc);
 
        trace_mm_vmscan_direct_reclaim_end(nr_reclaimed);
-       current->reclaim_state = NULL;
+       set_task_reclaim_state(current, NULL);
 
        return nr_reclaimed;
 }
@@ -3223,7 +3235,7 @@ unsigned long mem_cgroup_shrink_node(struct mem_cgroup *memcg,
        };
        unsigned long lru_pages;
 
-       current->reclaim_state = &sc.reclaim_state;
+       set_task_reclaim_state(current, &sc.reclaim_state);
        sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |
                        (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);
 
@@ -3241,7 +3253,7 @@ unsigned long mem_cgroup_shrink_node(struct mem_cgroup *memcg,
 
        trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
 
-       current->reclaim_state = NULL;
+       set_task_reclaim_state(current, NULL);
        *nr_scanned = sc.nr_scanned;
 
        return sc.nr_reclaimed;
@@ -3270,7 +3282,7 @@ unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
                .may_shrinkslab = 1,
        };
 
-       current->reclaim_state = &sc.reclaim_state;
+       set_task_reclaim_state(current, &sc.reclaim_state);
        /*
         * Unlike direct reclaim via alloc_pages(), memcg's reclaim doesn't
         * take care of from where we get pages. So the node where we start the
@@ -3291,7 +3303,7 @@ unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
        psi_memstall_leave(&pflags);
 
        trace_mm_vmscan_memcg_reclaim_end(nr_reclaimed);
-       current->reclaim_state = NULL;
+       set_task_reclaim_state(current, NULL);
 
        return nr_reclaimed;
 }
@@ -3493,7 +3505,7 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int classzone_idx)
                .may_unmap = 1,
        };
 
-       current->reclaim_state = &sc.reclaim_state;
+       set_task_reclaim_state(current, &sc.reclaim_state);
        psi_memstall_enter(&pflags);
        __fs_reclaim_acquire();
 
@@ -3675,7 +3687,7 @@ out:
        snapshot_refaults(NULL, pgdat);
        __fs_reclaim_release();
        psi_memstall_leave(&pflags);
-       current->reclaim_state = NULL;
+       set_task_reclaim_state(current, NULL);
 
        /*
         * Return the order kswapd stopped reclaiming at as
@@ -3940,17 +3952,16 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim)
                .hibernation_mode = 1,
        };
        struct zonelist *zonelist = node_zonelist(numa_node_id(), sc.gfp_mask);
-       struct task_struct *p = current;
        unsigned long nr_reclaimed;
        unsigned int noreclaim_flag;
 
        fs_reclaim_acquire(sc.gfp_mask);
        noreclaim_flag = memalloc_noreclaim_save();
-       p->reclaim_state = &sc.reclaim_state;
+       set_task_reclaim_state(current, &sc.reclaim_state);
 
        nr_reclaimed = do_try_to_free_pages(zonelist, &sc);
 
-       p->reclaim_state = NULL;
+       set_task_reclaim_state(current, NULL);
        memalloc_noreclaim_restore(noreclaim_flag);
        fs_reclaim_release(sc.gfp_mask);
 
@@ -4139,7 +4150,7 @@ static int __node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned in
         */
        noreclaim_flag = memalloc_noreclaim_save();
        p->flags |= PF_SWAPWRITE;
-       p->reclaim_state = &sc.reclaim_state;
+       set_task_reclaim_state(p, &sc.reclaim_state);
 
        if (node_pagecache_reclaimable(pgdat) > pgdat->min_unmapped_pages) {
                /*
@@ -4151,7 +4162,7 @@ static int __node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned in
                } while (sc.nr_reclaimed < nr_pages && --sc.priority >= 0);
        }
 
-       p->reclaim_state = NULL;
+       set_task_reclaim_state(p, NULL);
        current->flags &= ~PF_SWAPWRITE;
        memalloc_noreclaim_restore(noreclaim_flag);
        fs_reclaim_release(sc.gfp_mask);