From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Subject: mm/vmscan.c: add checks for incorrect handling of current->reclaim_state 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@xxxxxxxxx> Cc: Kirill Tkhai <ktkhai@xxxxxxxxxxxxx> Cc: Michal Hocko <mhocko@xxxxxxxx> Cc: Johannes Weiner <hannes@xxxxxxxxxxx> Cc: Vladimir Davydov <vdavydov.dev@xxxxxxxxx> Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- mm/vmscan.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) --- a/mm/vmscan.c~mm-vmscanc-add-checks-for-incorrect-handling-of-current-reclaim_state +++ a/mm/vmscan.c @@ -241,6 +241,18 @@ static void unregister_memcg_shrinker(st } #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 z 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(str }; 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(str 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_pag .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_pag 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 *pgda .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 .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_ */ 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_ } 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); _