History

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

When 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_read_lock/unlock() and synchronize_rcu() instead of reference count is used to implement get/put_swap_device(). >From get_swap_device() to put_swap_device(), RCU reader side is locked, so synchronize_rcu() 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 and swapoff are fixed too via calling synchronize_rcu() between clearing PageSwapCache() and freeing swap cache data structure. Another possible method to fix this is to use preempt_off() + stop_machine() to prevent the swap device from being swapoff when its data structure is being accessed. The overhead in hot-path of both methods is similar. The advantages of RCU based method are, 1. stop_machine() may disturb the normal execution code path on other CPUs. 2. File cache uses RCU to protect its radix tree. If the similar mechanism is used for swap cache too, it is easier to share code between them. 3. RCU is used to protect swap cache in total_swapcache_pages() and exit_swap_address_space() already. The two mechanisms can be merged to simplify the logic.

Huang Ying committed 7 years ago

Commit eb085571 parent a4985834 files · +146 −39Browse files

Unified · Split

4 files
  1. include/linux/swap.h +10 −3
  2. mm/memory.c +1 −1
  3. mm/swap_state.c +13 −3
  4. mm/swapfile.c +122 −32
include/linux/swap.h+10−3
@@ -175,8 +175,9 @@ enum { · 174 unmodified lines
175175 SWP_PAGE_DISCARD = (1 << 10), /* freed swap page-cluster discards */
176176 SWP_STABLE_WRITES = (1 << 11), /* no overwrite PG_writeback pages */
177177 SWP_SYNCHRONOUS_IO = (1 << 12), /* synchronous IO is efficient */
178+ SWP_VALID = (1 << 13), /* swap is valid to be operated on? */
178179 /* add others here before... */
179− SWP_SCANNING = (1 << 13), /* refcount in scan_swap_map */
180+ SWP_SCANNING = (1 << 14), /* refcount in scan_swap_map */
180181 };
181182
182183 #define SWAP_CLUSTER_MAX 32UL
@@ -460,7 +461,7 @@ extern unsigned int count_swap_pages(int, int); · 277 unmodified lines
460461 extern sector_t map_swap_page(struct page *, struct block_device **);
461462 extern sector_t swapdev_block(int, pgoff_t);
462463 extern int page_swapcount(struct page *);
463−extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
464+extern int __swap_count(swp_entry_t entry);
464465 extern int __swp_swapcount(swp_entry_t entry);
465466 extern int swp_swapcount(swp_entry_t entry);
466467 extern struct swap_info_struct *page_swap_info(struct page *);
@@ -470,6 +471,12 @@ extern int try_to_free_swap(struct page *); · 3 unmodified lines
470471 struct backing_dev_info;
471472 extern int init_swap_address_space(unsigned int type, unsigned long nr_pages);
472473 extern void exit_swap_address_space(unsigned int type);
474+extern struct swap_info_struct *get_swap_device(swp_entry_t entry);
475+
476+static inline void put_swap_device(struct swap_info_struct *si)
477+{
478+ rcu_read_unlock();
479+}
473480
474481 #else /* CONFIG_SWAP */
475482
@@ -576,7 +583,7 @@ static inline int page_swapcount(struct page *page) · 100 unmodified lines
576583 return 0;
577584 }
578585
579−static inline int __swap_count(struct swap_info_struct *si, swp_entry_t entry)
586+static inline int __swap_count(swp_entry_t entry)
580587 {
581588 return 0;
582589 }
mm/memory.c+1−1
@@ -2805,7 +2805,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) · 2804 unmodified lines
28052805 struct swap_info_struct *si = swp_swap_info(entry);
28062806
28072807 if (si->flags & SWP_SYNCHRONOUS_IO &&
2808− __swap_count(si, entry) == 1) {
2808+ __swap_count(entry) == 1) {
28092809 /* skip swapcache */
28102810 page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma,
28112811 vmf->address);
mm/swap_state.c+13−3
@@ -310,8 +310,13 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · 309 unmodified lines
310310 unsigned long addr)
311311 {
312312 struct page *page;
313+ struct swap_info_struct *si;
313314
315+ si = get_swap_device(entry);
316+ if (!si)
317+ return NULL;
314318 page = find_get_page(swap_address_space(entry), swp_offset(entry));
319+ put_swap_device(si);
315320
316321 INC_CACHE_INFO(find_total);
317322 if (page) {
@@ -354,8 +359,8 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, · 36 unmodified lines
354359 struct vm_area_struct *vma, unsigned long addr,
355360 bool *new_page_allocated)
356361 {
357− struct page *found_page, *new_page = NULL;
358− struct address_space *swapper_space = swap_address_space(entry);
362+ struct page *found_page = NULL, *new_page = NULL;
363+ struct swap_info_struct *si;
359364 int err;
360365 *new_page_allocated = false;
361366
@@ -365,7 +370,12 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, · 3 unmodified lines
365370 * called after lookup_swap_cache() failed, re-calling
366371 * that would confuse statistics.
367372 */
368− found_page = find_get_page(swapper_space, swp_offset(entry));
373+ si = get_swap_device(entry);
374+ if (!si)
375+ break;
376+ found_page = find_get_page(swap_address_space(entry),
377+ swp_offset(entry));
378+ put_swap_device(si);
369379 if (found_page)
370380 break;
371381
mm/swapfile.c+122−32
@@ -1079,12 +1079,11 @@ fail: · 1078 unmodified lines
10791079 static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
10801080 {
10811081 struct swap_info_struct *p;
1082− unsigned long offset, type;
1082+ unsigned long offset;
10831083
10841084 if (!entry.val)
10851085 goto out;
1086− type = swp_type(entry);
1087− p = swap_type_to_swap_info(type);
1086+ p = swp_swap_info(entry);
10881087 if (!p)
10891088 goto bad_nofile;
10901089 if (!(p->flags & SWP_USED))
@@ -1187,6 +1186,69 @@ static unsigned char __swap_entry_free_locked(struct swap_info_struct *p, · 96 unmodified lines
11871186 return usage;
11881187 }
11891188
1189+/*
1190+ * Check whether swap entry is valid in the swap device. If so,
1191+ * return pointer to swap_info_struct, and keep the swap entry valid
1192+ * via preventing the swap device from being swapoff, until
1193+ * put_swap_device() is called. Otherwise return NULL.
1194+ *
1195+ * The entirety of the RCU read critical section must come before the
1196+ * return from or after the call to synchronize_rcu() in
1197+ * enable_swap_info() or swapoff(). So if "si->flags & SWP_VALID" is
1198+ * true, the si->map, si->cluster_info, etc. must be valid in the
1199+ * critical section.
1200+ *
1201+ * Notice that swapoff or swapoff+swapon can still happen before the
1202+ * rcu_read_lock() in get_swap_device() or after the rcu_read_unlock()
1203+ * in put_swap_device() if there isn't any other way to prevent
1204+ * swapoff, such as page lock, page table lock, etc. The caller must
1205+ * be prepared for that. For example, the following situation is
1206+ * possible.
1207+ *
1208+ * CPU1 CPU2
1209+ * do_swap_page()
1210+ * ... swapoff+swapon
1211+ * __read_swap_cache_async()
1212+ * swapcache_prepare()
1213+ * __swap_duplicate()
1214+ * // check swap_map
1215+ * // verify PTE not changed
1216+ *
1217+ * In __swap_duplicate(), the swap_map need to be checked before
1218+ * changing partly because the specified swap entry may be for another
1219+ * swap device which has been swapoff. And in do_swap_page(), after
1220+ * the page is read from the swap device, the PTE is verified not
1221+ * changed with the page table locked to check whether the swap device
1222+ * has been swapoff or swapoff+swapon.
1223+ */
1224+struct swap_info_struct *get_swap_device(swp_entry_t entry)
1225+{
1226+ struct swap_info_struct *si;
1227+ unsigned long offset;
1228+
1229+ if (!entry.val)
1230+ goto out;
1231+ si = swp_swap_info(entry);
1232+ if (!si)
1233+ goto bad_nofile;
1234+
1235+ rcu_read_lock();
1236+ if (!(si->flags & SWP_VALID))
1237+ goto unlock_out;
1238+ offset = swp_offset(entry);
1239+ if (offset >= si->max)
1240+ goto unlock_out;
1241+
1242+ return si;
1243+bad_nofile:
1244+ pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
1245+out:
1246+ return NULL;
1247+unlock_out:
1248+ rcu_read_unlock();
1249+ return NULL;
1250+}
1251+
11901252 static unsigned char __swap_entry_free(struct swap_info_struct *p,
11911253 swp_entry_t entry, unsigned char usage)
11921254 {
@@ -1358,11 +1420,18 @@ int page_swapcount(struct page *page) · 165 unmodified lines
13581420 return count;
13591421 }
13601422
1361−int __swap_count(struct swap_info_struct *si, swp_entry_t entry)
1423+int __swap_count(swp_entry_t entry)
13621424 {
1425+ struct swap_info_struct *si;
13631426 pgoff_t offset = swp_offset(entry);
1427+ int count = 0;
13641428
1365− return swap_count(si->swap_map[offset]);
1429+ si = get_swap_device(entry);
1430+ if (si) {
1431+ count = swap_count(si->swap_map[offset]);
1432+ put_swap_device(si);
1433+ }
1434+ return count;
13661435 }
13671436
13681437 static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry)
@@ -1387,9 +1456,11 @@ int __swp_swapcount(swp_entry_t entry) · 18 unmodified lines
13871456 int count = 0;
13881457 struct swap_info_struct *si;
13891458
1390− si = __swap_info_get(entry);
1391− if (si)
1459+ si = get_swap_device(entry);
1460+ if (si) {
13921461 count = swap_swapcount(si, entry);
1462+ put_swap_device(si);
1463+ }
13931464 return count;
13941465 }
13951466
@@ -2335,9 +2406,9 @@ static int swap_node(struct swap_info_struct *p) · 939 unmodified lines
23352406 return bdev ? bdev->bd_disk->node_id : NUMA_NO_NODE;
23362407 }
23372408
2338−static void _enable_swap_info(struct swap_info_struct *p, int prio,
2339− unsigned char *swap_map,
2340− struct swap_cluster_info *cluster_info)
2409+static void setup_swap_info(struct swap_info_struct *p, int prio,
2410+ unsigned char *swap_map,
2411+ struct swap_cluster_info *cluster_info)
23412412 {
23422413 int i;
23432414
@@ -2362,7 +2433,11 @@ static void _enable_swap_info(struct swap_info_struct *p, int prio, · 18 unmodified lines
23622433 }
23632434 p->swap_map = swap_map;
23642435 p->cluster_info = cluster_info;
2365− p->flags |= SWP_WRITEOK;
2436+}
2437+
2438+static void _enable_swap_info(struct swap_info_struct *p)
2439+{
2440+ p->flags |= SWP_WRITEOK | SWP_VALID;
23662441 atomic_long_add(p->pages, &nr_swap_pages);
23672442 total_swap_pages += p->pages;
23682443
@@ -2389,7 +2464,17 @@ static void enable_swap_info(struct swap_info_struct *p, int prio, · 20 unmodified lines
23892464 frontswap_init(p->type, frontswap_map);
23902465 spin_lock(&swap_lock);
23912466 spin_lock(&p->lock);
2392− _enable_swap_info(p, prio, swap_map, cluster_info);
2467+ setup_swap_info(p, prio, swap_map, cluster_info);
2468+ spin_unlock(&p->lock);
2469+ spin_unlock(&swap_lock);
2470+ /*
2471+ * Guarantee swap_map, cluster_info, etc. fields are valid
2472+ * between get/put_swap_device() if SWP_VALID bit is set
2473+ */
2474+ synchronize_rcu();
2475+ spin_lock(&swap_lock);
2476+ spin_lock(&p->lock);
2477+ _enable_swap_info(p);
23932478 spin_unlock(&p->lock);
23942479 spin_unlock(&swap_lock);
23952480 }
@@ -2398,7 +2483,8 @@ static void reinsert_swap_info(struct swap_info_struct *p) · 2 unmodified lines
23982483 {
23992484 spin_lock(&swap_lock);
24002485 spin_lock(&p->lock);
2401− _enable_swap_info(p, p->prio, p->swap_map, p->cluster_info);
2486+ setup_swap_info(p, p->prio, p->swap_map, p->cluster_info);
2487+ _enable_swap_info(p);
24022488 spin_unlock(&p->lock);
24032489 spin_unlock(&swap_lock);
24042490 }
@@ -2501,6 +2587,17 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) · 96 unmodified lines
25012587
25022588 reenable_swap_slots_cache_unlock();
25032589
2590+ spin_lock(&swap_lock);
2591+ spin_lock(&p->lock);
2592+ p->flags &= ~SWP_VALID; /* mark swap device as invalid */
2593+ spin_unlock(&p->lock);
2594+ spin_unlock(&swap_lock);
2595+ /*
2596+ * wait for swap operations protected by get/put_swap_device()
2597+ * to complete
2598+ */
2599+ synchronize_rcu();
2600+
25042601 flush_work(&p->discard_work);
25052602
25062603 destroy_swap_extents(p);
@@ -3265,17 +3362,11 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage) · 758 unmodified lines
32653362 unsigned char has_cache;
32663363 int err = -EINVAL;
32673364
3268− if (non_swap_entry(entry))
3269− goto out;
3270−
3271− p = swp_swap_info(entry);
3365+ p = get_swap_device(entry);
32723366 if (!p)
3273− goto bad_file;
3274−
3275− offset = swp_offset(entry);
3276− if (unlikely(offset >= p->max))
32773367 goto out;
32783368
3369+ offset = swp_offset(entry);
32793370 ci = lock_cluster_or_swap_info(p, offset);
32803371
32813372 count = p->swap_map[offset];
@@ -3321,11 +3412,9 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage) · 39 unmodified lines
33213412 unlock_out:
33223413 unlock_cluster_or_swap_info(p, ci);
33233414 out:
3415+ if (p)
3416+ put_swap_device(p);
33243417 return err;
3325−
3326−bad_file:
3327− pr_err("swap_dup: %s%08lx\n", Bad_file, entry.val);
3328− goto out;
33293418 }
33303419
33313420 /*
@@ -3417,6 +3506,7 @@ int add_swap_count_continuation(swp_entry_t entry, gfp_t gfp_mask) · 85 unmodified lines
34173506 struct page *list_page;
34183507 pgoff_t offset;
34193508 unsigned char count;
3509+ int ret = 0;
34203510
34213511 /*
34223512 * When debugging, it's easier to use __GFP_ZERO here; but it's better
@@ -3424,15 +3514,15 @@ int add_swap_count_continuation(swp_entry_t entry, gfp_t gfp_mask) · 1 unmodified line
34243514 */
34253515 page = alloc_page(gfp_mask | __GFP_HIGHMEM);
34263516
3427− si = swap_info_get(entry);
3517+ si = get_swap_device(entry);
34283518 if (!si) {
34293519 /*
34303520 * An acceptable race has occurred since the failing
3431− * __swap_duplicate(): the swap entry has been freed,
3432− * perhaps even the whole swap_map cleared for swapoff.
3521+ * __swap_duplicate(): the swap device may be swapoff
34333522 */
34343523 goto outer;
34353524 }
3525+ spin_lock(&si->lock);
34363526
34373527 offset = swp_offset(entry);
34383528
@@ -3450,9 +3540,8 @@ int add_swap_count_continuation(swp_entry_t entry, gfp_t gfp_mask) · 11 unmodified lines
34503540 }
34513541
34523542 if (!page) {
3453− unlock_cluster(ci);
3454− spin_unlock(&si->lock);
3455− return -ENOMEM;
3543+ ret = -ENOMEM;
3544+ goto out;
34563545 }
34573546
34583547 /*
@@ -3504,10 +3593,11 @@ out_unlock_cont: · 45 unmodified lines
35043593 out:
35053594 unlock_cluster(ci);
35063595 spin_unlock(&si->lock);
3596+ put_swap_device(si);
35073597 outer:
35083598 if (page)
35093599 __free_page(page);
3510− return 0;
3600+ return ret;
35113601 }
35123602
35133603 /*

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t