Skip to contentSkip to conversation

mm, swap: Fix race between swapoff and some swap operations

PATCH New Huang, Ying 1 participant 1 message Dec 28, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mm, swap: Fix race between swapoff and some swap operations

From: Huang Ying <[email protected]>

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, RCU-sched instead of reference count is used to implement get/put_swap_device(). From get_swap_device() to put_swap_device(), the RCU-sched read lock is held, so synchronize_sched() 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 synchronize_sched() between clearing PageSwapCache() and freeing swap cache data structure.

Cc Hugh Dickins <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Minchan Kim <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Tim Chen <[email protected]>
Cc Shaohua Li <[email protected]>
Cc Mel Gorman <[email protected]>
Cc "JA(C)rA'me Glisse" <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrea Arcangeli <[email protected]>
Cc David Rientjes <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Jan Kara <[email protected]>
Cc Dave Jiang <[email protected]>
Cc Aaron Lu <[email protected]>

Changelog:

v5:

- Replace RCU with RCU-sched to fix some races between swap cache usage and swapoff.

v4:

- Use synchronize_rcu() in enable_swap_info() to reduce overhead of normal paths further.

v3:

- Re-implemented with RCU to reduce the overhead of normal paths

v2:

- Re-implemented with SRCU to reduce the overhead of normal paths.

- Avoid to check whether the swap device has been swapoff in get_swap_device(). Because we can check the origin of the swap entry to make sure the swap device hasn't bee swapoff.

---
 include/linux/swap.h |  13 ++++--
 mm/memory.c          |   2 +-
 mm/swap_state.c      |  16 +++++--
 mm/swapfile.c        | 123 ++++++++++++++++++++++++++++++++++++++-------------
 4 files changed, 117 insertions(+), 37 deletions(-)
include/linux/swap.h +10 -3
index 2417d288e016..4d91e003e2e5 100644
@@ -172,8 +172,9 @@ enum {
 	SWP_PAGE_DISCARD = (1 << 9),	/* freed swap page-cluster discards */
 	SWP_STABLE_WRITES = (1 << 10),	/* no overwrite PG_writeback pages */
 	SWP_SYNCHRONOUS_IO = (1 << 11),	/* synchronous IO is efficient */
+	SWP_VALID	= (1 << 12),	/* swap is valid to be operated on? */
 					/* add others here before... */
-	SWP_SCANNING	= (1 << 12),	/* refcount in scan_swap_map */
+	SWP_SCANNING	= (1 << 13),	/* refcount in scan_swap_map */
 };
 
 #define SWAP_CLUSTER_MAX 32UL
@@ -460,7 +461,7 @@ extern unsigned int count_swap_pages(int, int);
 extern sector_t map_swap_page(struct page *, struct block_device **);
 extern sector_t swapdev_block(int, pgoff_t);
 extern int page_swapcount(struct page *);
-extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
+extern int __swap_count(swp_entry_t entry);
 extern int __swp_swapcount(swp_entry_t entry);
 extern int swp_swapcount(swp_entry_t entry);
 extern struct swap_info_struct *page_swap_info(struct page *);
@@ -470,6 +471,12 @@ extern int try_to_free_swap(struct page *);
 struct backing_dev_info;
 extern int init_swap_address_space(unsigned int type, unsigned long nr_pages);
 extern void exit_swap_address_space(unsigned int type);
+extern struct swap_info_struct *get_swap_device(swp_entry_t entry);
+
+static inline void put_swap_device(struct swap_info_struct *si)
+{
+	rcu_read_unlock_sched();
+}
 
 #else /* CONFIG_SWAP */
 
@@ -575,7 +582,7 @@ static inline int page_swapcount(struct page *page)
 	return 0;
 }
 
-static inline int __swap_count(struct swap_info_struct *si, swp_entry_t entry)
+static inline int __swap_count(swp_entry_t entry)
 {
 	return 0;
 }
mm/memory.c +1 -1
index 36dd3a66aa5a..756b6fc94bcd 100644
@@ -2909,7 +2909,7 @@ int do_swap_page(struct vm_fault *vmf)
 		struct swap_info_struct *si = swp_swap_info(entry);
 
 		if (si->flags & SWP_SYNCHRONOUS_IO &&
-				__swap_count(si, entry) == 1) {
+		    __swap_count(entry) == 1) {
 			/* skip swapcache */
 			page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma,
 							vmf->address);
mm/swap_state.c +13 -3
index 0b8ae361981f..8dde719e973c 100644
@@ -337,8 +337,13 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 			       unsigned long addr)
 {
 	struct page *page;
+	struct swap_info_struct *si;
 
+	si = get_swap_device(entry);
+	if (!si)
+		return NULL;
 	page = find_get_page(swap_address_space(entry), swp_offset(entry));
+	put_swap_device(si);
 
 	INC_CACHE_INFO(find_total);
 	if (page) {
@@ -376,8 +381,8 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
 			struct vm_area_struct *vma, unsigned long addr,
 			bool *new_page_allocated)
 {
-	struct page *found_page, *new_page = NULL;
-	struct address_space *swapper_space = swap_address_space(entry);
+	struct page *found_page = NULL, *new_page = NULL;
+	struct swap_info_struct *si;
 	int err;
 	*new_page_allocated = false;
 
@@ -387,7 +392,12 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
 		 * called after lookup_swap_cache() failed, re-calling
 		 * that would confuse statistics.
 		 */
-		found_page = find_get_page(swapper_space, swp_offset(entry));
+		si = get_swap_device(entry);
+		if (!si)
+			break;
+		found_page = find_get_page(swap_address_space(entry),
+					   swp_offset(entry));
+		put_swap_device(si);
 		if (found_page)
 			break;
 
mm/swapfile.c +93 -30

228 lines not on this page · raw message

Signature · 7 lines
2.15.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>