Re: [PATCH -V5b -mm] mm, swap: Fix race between swapoff and some swap operations

From: Huang\, Ying
Date: Wed Dec 27 2017 - 20:53:37 EST

"Huang, Ying" <ying.huang@xxxxxxxxx> writes:

> From: Huang Ying <ying.huang@xxxxxxxxx>
> When the swapin is performed, after getting the swap entry information
> from the page table, system will swap in the swap entry, without any
> lock held to prevent the swap device from being swapoff. This may
> cause the race like below,
> CPU 1 CPU 2
> ----- -----
> do_swap_page
> swapin_readahead
> __read_swap_cache_async
> swapoff swapcache_prepare
> p->swap_map = NULL __swap_duplicate
> p->swap_map[?] /* !!! NULL pointer access */
> Because swapoff is usually done when system shutdown only, the race
> may not hit many people in practice. But it is still a race need to
> be fixed.
> To fix the race, get_swap_device() is added to check whether the
> specified swap entry is valid in its swap device. If so, it will keep
> the swap entry valid via preventing the swap device from being
> swapoff, until put_swap_device() is called.
> Because swapoff() is very rare code path, to make the normal path runs
> as fast as possible, disabling preemption + stop_machine() instead of
> reference count is used to implement get/put_swap_device(). From
> get_swap_device() to put_swap_device(), the preemption is disabled, so
> stop_machine() in swapoff() will wait until put_swap_device() is
> called.
> In addition to swap_map, cluster_info, etc. data structure in the
> struct swap_info_struct, the swap cache radix tree will be freed after
> swapoff, so this patch fixes the race between swap cache looking up
> and swapoff too.
> Races between some other swap cache usages protected via disabling
> preemption and swapoff are fixed too via calling stop_machine()
> between clearing PageSwapCache() and freeing swap cache data
> structure.
> Alternative implementation could be replacing disable preemption with
> rcu_read_lock_sched and stop_machine() with synchronize_sched().
> Cc: Hugh Dickins <hughd@xxxxxxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> Cc: Minchan Kim <minchan@xxxxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
> Cc: Shaohua Li <shli@xxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
> Cc: "JÃrÃme Glisse" <jglisse@xxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Rik van Riel <riel@xxxxxxxxxx>
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Dave Jiang <dave.jiang@xxxxxxxxx>
> Cc: Aaron Lu <>
> Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> Changelog:
> v5:
> - Replace RCU with stop_machine()

2 versions (-V5a and -V5b) have been sent, one is implemented with
stop_machine(), the other is implemented with RCU-sched. RCU-sched
based version is better for real time users. Both are OK for me.

Best Regards,
Huang, Ying