Commit 78ecba081224a2db5876b6b81cfed0b78f58adc7
Committed by
Linus Torvalds
1 parent
83d1674a94
Exists in
master
and in
7 other branches
mm: fix ever-decreasing swap priority
Vegard Nossum has noticed the ever-decreasing negative priority in a swapon /swapoff loop, which eventually would misprioritize when int wraps positive. Not worth spending much code on, but probably better fixed. It's easy to handle the swapping on and off of just one area, but there's not much point if a pair or more still misbehave. To handle the general case, swapoff should compact negative priorities, keeping them always from -1 to -MAX_SWAPFILES. That's a change, but should cause no regression, since these negative (unspecified) priorities are disjoint from the the positive specified priorities 0 to 32767. One small functional difference, which seems appropriate: when swapoff fails to free all swap from a negative priority area, that area is now reinserted at lowest priority, rather than at its original priority. In moving down swapon's setting of priority, I notice that an area is visible to /proc/swaps when it has swap_map set, yet that was being set before all the visible fields were properly filled in: corrected. Signed-off-by: Hugh Dickins <hugh@veritas.com> Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Reported-by: Vegard Nossum <vegard.nossum@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Showing 1 changed file with 25 additions and 24 deletions Side-by-side Diff
mm/swapfile.c
... | ... | @@ -37,6 +37,7 @@ |
37 | 37 | unsigned int nr_swapfiles; |
38 | 38 | long total_swap_pages; |
39 | 39 | static int swap_overflow; |
40 | +static int least_priority; | |
40 | 41 | |
41 | 42 | static const char Bad_file[] = "Bad swap file entry "; |
42 | 43 | static const char Unused_file[] = "Unused swap file entry "; |
... | ... | @@ -1260,6 +1261,11 @@ |
1260 | 1261 | /* just pick something that's safe... */ |
1261 | 1262 | swap_list.next = swap_list.head; |
1262 | 1263 | } |
1264 | + if (p->prio < 0) { | |
1265 | + for (i = p->next; i >= 0; i = swap_info[i].next) | |
1266 | + swap_info[i].prio = p->prio--; | |
1267 | + least_priority++; | |
1268 | + } | |
1263 | 1269 | nr_swap_pages -= p->pages; |
1264 | 1270 | total_swap_pages -= p->pages; |
1265 | 1271 | p->flags &= ~SWP_WRITEOK; |
1266 | 1272 | |
... | ... | @@ -1272,9 +1278,14 @@ |
1272 | 1278 | if (err) { |
1273 | 1279 | /* re-insert swap space back into swap_list */ |
1274 | 1280 | spin_lock(&swap_lock); |
1275 | - for (prev = -1, i = swap_list.head; i >= 0; prev = i, i = swap_info[i].next) | |
1281 | + if (p->prio < 0) | |
1282 | + p->prio = --least_priority; | |
1283 | + prev = -1; | |
1284 | + for (i = swap_list.head; i >= 0; i = swap_info[i].next) { | |
1276 | 1285 | if (p->prio >= swap_info[i].prio) |
1277 | 1286 | break; |
1287 | + prev = i; | |
1288 | + } | |
1278 | 1289 | p->next = i; |
1279 | 1290 | if (prev < 0) |
1280 | 1291 | swap_list.head = swap_list.next = p - swap_info; |
... | ... | @@ -1447,7 +1458,6 @@ |
1447 | 1458 | unsigned int type; |
1448 | 1459 | int i, prev; |
1449 | 1460 | int error; |
1450 | - static int least_priority; | |
1451 | 1461 | union swap_header *swap_header = NULL; |
1452 | 1462 | int swap_header_version; |
1453 | 1463 | unsigned int nr_good_pages = 0; |
... | ... | @@ -1455,7 +1465,7 @@ |
1455 | 1465 | sector_t span; |
1456 | 1466 | unsigned long maxpages = 1; |
1457 | 1467 | int swapfilesize; |
1458 | - unsigned short *swap_map; | |
1468 | + unsigned short *swap_map = NULL; | |
1459 | 1469 | struct page *page = NULL; |
1460 | 1470 | struct inode *inode = NULL; |
1461 | 1471 | int did_down = 0; |
1462 | 1472 | |
1463 | 1473 | |
... | ... | @@ -1474,22 +1484,10 @@ |
1474 | 1484 | } |
1475 | 1485 | if (type >= nr_swapfiles) |
1476 | 1486 | nr_swapfiles = type+1; |
1487 | + memset(p, 0, sizeof(*p)); | |
1477 | 1488 | INIT_LIST_HEAD(&p->extent_list); |
1478 | 1489 | p->flags = SWP_USED; |
1479 | - p->swap_file = NULL; | |
1480 | - p->old_block_size = 0; | |
1481 | - p->swap_map = NULL; | |
1482 | - p->lowest_bit = 0; | |
1483 | - p->highest_bit = 0; | |
1484 | - p->cluster_nr = 0; | |
1485 | - p->inuse_pages = 0; | |
1486 | 1490 | p->next = -1; |
1487 | - if (swap_flags & SWAP_FLAG_PREFER) { | |
1488 | - p->prio = | |
1489 | - (swap_flags & SWAP_FLAG_PRIO_MASK)>>SWAP_FLAG_PRIO_SHIFT; | |
1490 | - } else { | |
1491 | - p->prio = --least_priority; | |
1492 | - } | |
1493 | 1491 | spin_unlock(&swap_lock); |
1494 | 1492 | name = getname(specialfile); |
1495 | 1493 | error = PTR_ERR(name); |
1496 | 1494 | |
1497 | 1495 | |
... | ... | @@ -1632,19 +1630,20 @@ |
1632 | 1630 | goto bad_swap; |
1633 | 1631 | |
1634 | 1632 | /* OK, set up the swap map and apply the bad block list */ |
1635 | - if (!(p->swap_map = vmalloc(maxpages * sizeof(short)))) { | |
1633 | + swap_map = vmalloc(maxpages * sizeof(short)); | |
1634 | + if (!swap_map) { | |
1636 | 1635 | error = -ENOMEM; |
1637 | 1636 | goto bad_swap; |
1638 | 1637 | } |
1639 | 1638 | |
1640 | 1639 | error = 0; |
1641 | - memset(p->swap_map, 0, maxpages * sizeof(short)); | |
1640 | + memset(swap_map, 0, maxpages * sizeof(short)); | |
1642 | 1641 | for (i = 0; i < swap_header->info.nr_badpages; i++) { |
1643 | 1642 | int page_nr = swap_header->info.badpages[i]; |
1644 | 1643 | if (page_nr <= 0 || page_nr >= swap_header->info.last_page) |
1645 | 1644 | error = -EINVAL; |
1646 | 1645 | else |
1647 | - p->swap_map[page_nr] = SWAP_MAP_BAD; | |
1646 | + swap_map[page_nr] = SWAP_MAP_BAD; | |
1648 | 1647 | } |
1649 | 1648 | nr_good_pages = swap_header->info.last_page - |
1650 | 1649 | swap_header->info.nr_badpages - |
... | ... | @@ -1654,7 +1653,7 @@ |
1654 | 1653 | } |
1655 | 1654 | |
1656 | 1655 | if (nr_good_pages) { |
1657 | - p->swap_map[0] = SWAP_MAP_BAD; | |
1656 | + swap_map[0] = SWAP_MAP_BAD; | |
1658 | 1657 | p->max = maxpages; |
1659 | 1658 | p->pages = nr_good_pages; |
1660 | 1659 | nr_extents = setup_swap_extents(p, &span); |
... | ... | @@ -1672,6 +1671,12 @@ |
1672 | 1671 | |
1673 | 1672 | mutex_lock(&swapon_mutex); |
1674 | 1673 | spin_lock(&swap_lock); |
1674 | + if (swap_flags & SWAP_FLAG_PREFER) | |
1675 | + p->prio = | |
1676 | + (swap_flags & SWAP_FLAG_PRIO_MASK) >> SWAP_FLAG_PRIO_SHIFT; | |
1677 | + else | |
1678 | + p->prio = --least_priority; | |
1679 | + p->swap_map = swap_map; | |
1675 | 1680 | p->flags = SWP_ACTIVE; |
1676 | 1681 | nr_swap_pages += nr_good_pages; |
1677 | 1682 | total_swap_pages += nr_good_pages; |
1678 | 1683 | |
1679 | 1684 | |
... | ... | @@ -1707,12 +1712,8 @@ |
1707 | 1712 | destroy_swap_extents(p); |
1708 | 1713 | bad_swap_2: |
1709 | 1714 | spin_lock(&swap_lock); |
1710 | - swap_map = p->swap_map; | |
1711 | 1715 | p->swap_file = NULL; |
1712 | - p->swap_map = NULL; | |
1713 | 1716 | p->flags = 0; |
1714 | - if (!(swap_flags & SWAP_FLAG_PREFER)) | |
1715 | - ++least_priority; | |
1716 | 1717 | spin_unlock(&swap_lock); |
1717 | 1718 | vfree(swap_map); |
1718 | 1719 | if (swap_file) |