From b3fed13e9dba0b74972266f78bc6df6c51613b1a Mon Sep 17 00:00:00 2001 From: David Schultz Date: Fri, 5 Nov 2004 05:36:56 +0000 Subject: Close a race in swapoff(). Here are the gory details: In order to avoid livelock, swapoff() skips over objects with a nonzero pip count and makes another pass if necessary. Since it is impossible to know which objects we care about, it would choose an arbitrary object with a nonzero pip count and wait for it before making another pass, the theory being that this object would finish paging about as quickly as the ones we care about. Unfortunately, we may have slept since we acquired a reference to this object. Hack around this problem by tsleep()ing on the pointer anyway, but timeout after a fixed interval. More elegant solutions are possible, but the ones I considered unnecessarily complicate this rare case. Also, kill some nits that seem to have crept into the swapoff() code in the last 75 revisions or so: - Don't pass both sp and sp->sw_used to swap_pager_swapoff(), since the latter can be derived from the former. - Replace swp_pager_find_dev() with something simpler. There's no need to iterate over the entire list of swap devices just to determine if a given block is assigned to the one we're interested in. - Expand the scope of the swhash_mtx in a couple of places so that it isn't released and reacquired once for every hash bucket. - Don't drop the swhash_mtx while holding a reference to an object. We need to lock the object first. Unfortunately, doing so would violate the established lock order, so use VM_OBJECT_TRYLOCK() and try again on a subsequent pass if the object is already locked. - Refactor swp_pager_force_pagein() and swap_pager_swapoff() a bit. --- sys/vm/swap_pager.c | 123 ++++++++++++++++++++++------------------------------ 1 file changed, 53 insertions(+), 70 deletions(-) diff --git a/sys/vm/swap_pager.c b/sys/vm/swap_pager.c index b4c6a6a862fc..da7f1c03d6e1 100644 --- a/sys/vm/swap_pager.c +++ b/sys/vm/swap_pager.c @@ -236,7 +236,7 @@ static boolean_t swap_pager_haspage(vm_object_t object, vm_pindex_t pindex, int *before, int *after); static void swap_pager_init(void); static void swap_pager_unswapped(vm_page_t); -static void swap_pager_swapoff(struct swdevt *sp, int *sw_used); +static void swap_pager_swapoff(struct swdevt *sp); struct pagerops swappagerops = { .pgo_init = swap_pager_init, /* early system initialization of pager */ @@ -609,19 +609,11 @@ done: return (blk); } -static struct swdevt * -swp_pager_find_dev(daddr_t blk) +static int +swp_pager_isondev(daddr_t blk, struct swdevt *sp) { - struct swdevt *sp; - mtx_lock(&sw_dev_mtx); - TAILQ_FOREACH(sp, &swtailq, sw_list) { - if (blk >= sp->sw_first && blk < sp->sw_end) { - mtx_unlock(&sw_dev_mtx); - return (sp); - } - } - panic("Swapdev not found"); + return (blk >= sp->sw_first && blk < sp->sw_end); } static void @@ -1597,27 +1589,24 @@ swap_pager_isswapped(vm_object_t object, struct swdevt *sp) if (object->type != OBJT_SWAP) return (0); + mtx_lock(&swhash_mtx); for (bcount = 0; bcount < object->un_pager.swp.swp_bcount; bcount++) { struct swblock *swap; - mtx_lock(&swhash_mtx); if ((swap = *swp_pager_hash(object, index)) != NULL) { for (i = 0; i < SWAP_META_PAGES; ++i) { - daddr_t v = swap->swb_pages[i]; - if (v == SWAPBLK_NONE) - continue; - if (swp_pager_find_dev(v) == sp) { + if (swp_pager_isondev(swap->swb_pages[i], sp)) { mtx_unlock(&swhash_mtx); - return 1; + return (1); } } } - mtx_unlock(&swhash_mtx); index += SWAP_META_PAGES; if (index > 0x20000000) panic("swap_pager_isswapped: failed to locate all swap meta blocks"); } - return 0; + mtx_unlock(&swhash_mtx); + return (0); } /* @@ -1635,19 +1624,12 @@ swap_pager_isswapped(vm_object_t object, struct swdevt *sp) * revert to the one-by-one behavior for now. Sigh. */ static __inline void -swp_pager_force_pagein(struct swblock *swap, int idx) +swp_pager_force_pagein(vm_object_t object, vm_pindex_t pindex) { - vm_object_t object; vm_page_t m; - vm_pindex_t pindex; - object = swap->swb_object; - pindex = swap->swb_index; - mtx_unlock(&swhash_mtx); - - VM_OBJECT_LOCK(object); vm_object_pip_add(object, 1); - m = vm_page_grab(object, pindex + idx, VM_ALLOC_NORMAL|VM_ALLOC_RETRY); + m = vm_page_grab(object, pindex, VM_ALLOC_NORMAL|VM_ALLOC_RETRY); if (m->valid == VM_PAGE_BITS_ALL) { vm_object_pip_subtract(object, 1); vm_page_lock_queues(); @@ -1656,12 +1638,10 @@ swp_pager_force_pagein(struct swblock *swap, int idx) vm_page_wakeup(m); vm_page_unlock_queues(); vm_pager_page_unswapped(m); - VM_OBJECT_UNLOCK(object); return; } - if (swap_pager_getpages(object, &m, 1, 0) != - VM_PAGER_OK) + if (swap_pager_getpages(object, &m, 1, 0) != VM_PAGER_OK) panic("swap_pager_force_pagein: read from swap failed");/*XXX*/ vm_object_pip_subtract(object, 1); vm_page_lock_queues(); @@ -1670,10 +1650,8 @@ swp_pager_force_pagein(struct swblock *swap, int idx) vm_page_wakeup(m); vm_page_unlock_queues(); vm_pager_page_unswapped(m); - VM_OBJECT_UNLOCK(object); } - /* * swap_pager_swapoff: * @@ -1682,60 +1660,65 @@ swp_pager_force_pagein(struct swblock *swap, int idx) * marked as allocated and the device must be flagged SW_CLOSING. * There may be no processes swapped out to the device. * - * The sw_used parameter points to the field in the swdev structure - * that contains a count of the number of blocks still allocated - * on the device. If we encounter objects with a nonzero pip count - * in our scan, we use this number to determine if we're really done. - * * This routine may block. */ static void -swap_pager_swapoff(struct swdevt *sp, int *sw_used) +swap_pager_swapoff(struct swdevt *sp) { - struct swblock **pswap; struct swblock *swap; vm_object_t waitobj; - daddr_t v; int i, j; GIANT_REQUIRED; full_rescan: waitobj = NULL; + mtx_lock(&swhash_mtx); for (i = 0; i <= swhash_mask; i++) { /* '<=' is correct here */ restart: - pswap = &swhash[i]; - mtx_lock(&swhash_mtx); - while ((swap = *pswap) != NULL) { + for (swap = swhash[i]; swap != NULL; swap = swap->swb_hnext) { + vm_object_t object = swap->swb_object; + vm_pindex_t pindex = swap->swb_index; for (j = 0; j < SWAP_META_PAGES; ++j) { - v = swap->swb_pages[j]; - if (v != SWAPBLK_NONE && - swp_pager_find_dev(v) == sp) - break; + if (swp_pager_isondev(swap->swb_pages[j], sp)) { + /* avoid deadlock */ + if (!VM_OBJECT_TRYLOCK(object)) { + waitobj = object; + break; + } else { + mtx_unlock(&swhash_mtx); + swp_pager_force_pagein(object, + pindex + j); + VM_OBJECT_UNLOCK(object); + mtx_lock(&swhash_mtx); + goto restart; + } + } } - if (j < SWAP_META_PAGES) { - swp_pager_force_pagein(swap, j); - goto restart; - } else if (swap->swb_object->paging_in_progress) { - if (!waitobj) - waitobj = swap->swb_object; - } - pswap = &swap->swb_hnext; + if (object->paging_in_progress) + waitobj = object; } - mtx_unlock(&swhash_mtx); } - if (waitobj && *sw_used) { - /* - * We wait on an arbitrary object to clock our rescans - * to the rate of paging completion. - */ - VM_OBJECT_LOCK(waitobj); - vm_object_pip_wait(waitobj, "swpoff"); - VM_OBJECT_UNLOCK(waitobj); - goto full_rescan; + mtx_unlock(&swhash_mtx); + if (waitobj && sp->sw_used) { + /* + * The most likely reason we will have to do another pass is + * that something is being paged out to the device being + * removed. This can't happen forever because new allocations + * will not be made on this device, but we still need to wait + * for the activity to finish. We have no way of knowing + * which objects we need to wait for, so we pick an arbitrary + * object that is paging and hope that it finishes paging at + * about the same time as one of the objects we care about. + * + * XXX Unfortunately, our waitobj reference might not be valid + * anymore, so we also retry after 50 ms. + */ + tsleep(waitobj, PVM, "swpoff", hz / 20); + goto full_rescan; } - if (*sw_used) - panic("swapoff: failed to locate %d swap blocks", *sw_used); + if (sp->sw_used) + panic("swapoff: failed to locate %d swap blocks", sp->sw_used); } /************************************************************************ @@ -2237,7 +2220,7 @@ found: #ifndef NO_SWAPPING vm_proc_swapin_all(sp); #endif /* !NO_SWAPPING */ - swap_pager_swapoff(sp, &sp->sw_used); + swap_pager_swapoff(sp); sp->sw_close(td, sp); sp->sw_id = NULL; -- cgit v1.3