Changeset View
Standalone View
sys/vm/swap_pager.c
Show First 20 Lines • Show All 1,644 Lines • ▼ Show 20 Lines | |||||
int | int | ||||
swap_pager_nswapdev(void) | swap_pager_nswapdev(void) | ||||
{ | { | ||||
return (nswapdev); | return (nswapdev); | ||||
} | } | ||||
/* | static void | ||||
* SWP_PAGER_FORCE_PAGEIN() - force a swap block to be paged in | swp_pager_force_dirty(vm_page_t m) | ||||
alc: I would suggest the name swp_pager_force_dirty. In particular, the page may not be in the… | |||||
* | |||||
* This routine dissociates the page at the given index within an object | |||||
* from its backing store, paging it in if it does not reside in memory. | |||||
* If the page is paged in, it is marked dirty and placed in the laundry | |||||
* queue. The page is marked dirty because it no longer has backing | |||||
* store. It is placed in the laundry queue because it has not been | |||||
* accessed recently. Otherwise, it would already reside in memory. | |||||
* | |||||
* We also attempt to swap in all other pages in the swap block. | |||||
* However, we only guarantee that the one at the specified index is | |||||
* paged in. | |||||
* | |||||
* XXX - The code to page the whole block in doesn't work, so we | |||||
* revert to the one-by-one behavior for now. Sigh. | |||||
*/ | |||||
static inline void | |||||
swp_pager_force_pagein(vm_object_t object, vm_pindex_t pindex) | |||||
{ | { | ||||
Done Inline ActionsThere must be an empty line after the '{' if function does not declare locals. Think of it as a delimiter between empty locals block and body. kib: There must be an empty line after the '{' if function does not declare locals. Think of it as… | |||||
vm_page_t m; | |||||
Done Inline Actionsobject is actually m->object, there is no need to pass it as the argument. kib: object is actually m->object, there is no need to pass it as the argument. | |||||
vm_object_pip_add(object, 1); | vm_object_pip_wakeup(m->object); | ||||
m = vm_page_grab(object, pindex, VM_ALLOC_NORMAL); | |||||
if (m->valid == VM_PAGE_BITS_ALL) { | |||||
vm_object_pip_wakeup(object); | |||||
vm_page_dirty(m); | vm_page_dirty(m); | ||||
#ifdef INVARIANTS | #ifdef INVARIANTS | ||||
vm_page_lock(m); | vm_page_lock(m); | ||||
if (m->wire_count == 0 && m->queue == PQ_NONE) | if (m->wire_count == 0 && m->queue == PQ_NONE) | ||||
panic("page %p is neither wired nor queued", m); | panic("page %p is neither wired nor queued", m); | ||||
vm_page_unlock(m); | vm_page_unlock(m); | ||||
#endif | #endif | ||||
vm_page_xunbusy(m); | vm_page_xunbusy(m); | ||||
vm_pager_page_unswapped(m); | vm_pager_page_unswapped(m); | ||||
return; | |||||
} | } | ||||
if (swap_pager_getpages(object, &m, 1, NULL, NULL) != VM_PAGER_OK) | static void | ||||
panic("swap_pager_force_pagein: read from swap failed");/*XXX*/ | swp_pager_force_launder(vm_page_t m) | ||||
Done Inline ActionsI would suggest the name swp_pager_force_launder. alc: I would suggest the name swp_pager_force_launder. | |||||
vm_object_pip_wakeup(object); | { | ||||
Done Inline ActionsSame two comments. kib: Same two comments. | |||||
vm_object_pip_wakeup(m->object); | |||||
vm_page_dirty(m); | vm_page_dirty(m); | ||||
vm_page_lock(m); | vm_page_lock(m); | ||||
vm_page_launder(m); | vm_page_launder(m); | ||||
vm_page_unlock(m); | vm_page_unlock(m); | ||||
vm_page_xunbusy(m); | vm_page_xunbusy(m); | ||||
vm_pager_page_unswapped(m); | vm_pager_page_unswapped(m); | ||||
} | } | ||||
/* | /* | ||||
* SWP_PAGER_FORCE_PAGEIN() - force a swap block to be paged in | |||||
* | |||||
* Returns the number of pages that are paged in. The maximum number of | |||||
* pages this function can page in at a time is SWB_NPAGES. | |||||
* | |||||
* This routine dissociates the page at the given index within an object | |||||
* from its backing store, paging it in if it does not reside in memory. | |||||
* If the page is paged in, it is marked dirty and placed in the laundry | |||||
* queue. The page is marked dirty because it no longer has backing | |||||
* store. It is placed in the laundry queue because it has not been | |||||
* accessed recently. Otherwise, it would already reside in memory. | |||||
* | |||||
* We also attempt to swap in all other pages in the swap block. | |||||
* However, we only guarantee that the one at the specified index is | |||||
* paged in. | |||||
*/ | |||||
static int | |||||
Done Inline ActionsUpdate the comment to state what the return value signified. dougm: Update the comment to state what the return value signified. | |||||
swp_pager_force_pagein(vm_object_t object, vm_pindex_t pindex) | |||||
{ | |||||
vm_page_t ma[SWB_NPAGES]; | |||||
int i, j, npages; | |||||
Done Inline ActionsThe return type of swap_pager_haspage is boolean_t, so 'rv' should be of that type and should not be compared to 1. I don't understand what means 'rv' has as a variable name. 'haspage' would mean something, for example. dougm: The return type of swap_pager_haspage is boolean_t, so 'rv' should be of that type and should… | |||||
Done Inline ActionsI moved the call inside KASSERT, not saving return value to a local variable. ota_j.email.ne.jp: I moved the call inside KASSERT, not saving return value to a local variable. | |||||
boolean_t rv; | |||||
Done Inline ActionsVariables of this type would conventionally be named "ma". alc: Variables of this type would conventionally be named "ma". | |||||
Done Inline ActionsThis line seems to choose to use the idea I suggested, which you said didn't work. dougm: This line seems to choose to use the idea I suggested, which you said didn't work. | |||||
rv = swap_pager_haspage(object, pindex, NULL, &npages); | |||||
Done Inline ActionsThe name 'error' is confusing. How about 'haspage'? dougm: The name 'error' is confusing. How about 'haspage'?
Otherwise, you seem to be saying that you… | |||||
Done Inline Actionsswap_pager_haspage() call here is to find how many EXTRA and adjacent pages available behind the current one. The return value is passed back to the caller via the function variable, npages here, and return code indicates if the function call is successful or not. At this line of code, we know pindex was paged out and we know we need to take at least one page back from the device. So, actually and after iterations of thinking and discussion, I think our best action is npages = 1 and continue. I originally added KASSERT because the piece I used as a reference had KASSERT. At this point, I think we'd better drop it. ota_j.email.ne.jp: swap_pager_haspage() call here is to find how many EXTRA and adjacent pages available behind… | |||||
KASSERT(rv == TRUE, ("%s: missing page %p", __func__, ma)); | |||||
Done Inline ActionsI don't understand what 'm' has to do with the unexpected result from swap_pager_haspage, so I don't know why it is written out here. haspage returns TRUE or FALSE, so don't compare it to 1. dougm: I don't understand what 'm' has to do with the unexpected result from swap_pager_haspage, so I… | |||||
Done Inline ActionsThe name 'rv' is used for 'return value' or 'reservation' in this code, and this use is neither of those. How about "hasPage" or something like that? 'ma' is just the address of your local array and tells no one anything useful in an error log. Either object, or pindex, or both might be useful, but not 'ma'. Remove the '== TRUE' part. dougm: The name 'rv' is used for 'return value' or 'reservation' in this code, and this use is neither… | |||||
Done Inline ActionsIf 'npages' is set within a KASSERT expression, then it won't be set if assertions aren't turned on, and the program will be broken. I suggested earlier that you just return '1' in this case; you haven't commented on that. dougm: If 'npages' is set within a KASSERT expression, then it won't be set if assertions aren't… | |||||
Done Inline ActionsI somehow thought KASSERT would be on all times and would be easier moving the evaluation inside. That's why I saw uninitialized error on some code trees but not all trees. I will fix these tomorrow. ota_j.email.ne.jp: I somehow thought KASSERT would be on all times and would be easier moving the evaluation… | |||||
npages += 1; | |||||
Not Done Inline ActionsMove this line back before the call to vm_page_get_pages(). alc: Move this line back before the call to vm_page_get_pages(). | |||||
npages = vm_page_grab_pages(object, pindex, VM_ALLOC_NORMAL, ma, npages); | |||||
dougmUnsubmitted Done Inline ActionsThis breaks the 80-column limit. dougm: This breaks the 80-column limit. | |||||
Done Inline ActionsIs the value of npages returned form vm_page_grab_pages always going to match the value of npages passed in? dougm: Is the value of npages returned form vm_page_grab_pages always going to match the value of… | |||||
Done Inline ActionsThe npages passed in is the maximum. It may return lower number. On the other hand, I had had few extra condition checks to see how it would work. This call here always returned the same number. ota_j.email.ne.jp: The npages passed in is the maximum. It may return lower number.
On the other hand, I had had… | |||||
Not Done Inline ActionsThe "else" clause should be reconsidered. swap_pager_haspage(object, pindex) More generally, I was surprised to see swap_pager_haspage() called here. The caller to this function already has found the sb structure from which npages will be computed, so I was surprised that it wasn't computed by the caller and passed in. alc: The "else" clause should be reconsidered. swap_pager_haspage(object, pindex)
should never… | |||||
Done Inline ActionsIndeed, "the caller has determined that there is, in fact, an allocated swap block at pindex of object" is the true statement and thus old code didn't call swap_pager_haspage() but rather called vm_page_grab() to prepare vm_object_t. I added this call to also find how many EXTRA pages continues after the one we know so that we can call swap_pager_getpages() for more than one page. It sounds like we need to add a panic() if swap_pager_haspage() returns false, instead. Or should I KASSERT? ota_j.email.ne.jp: Indeed, "the caller has determined that there is, in fact, an allocated swap block at pindex of… | |||||
vm_object_pip_add(object, npages); | |||||
Done Inline ActionsIf you invoked this as dougm: If you invoked this as
npages = vm_page_grab_pages(object, pindex, VM_ALLOC_NORMAL |… | |||||
Done Inline ActionsThis vm_page_grab_pages() call corresponds to "m = vm_page_grab(object, pindex, VM_ALLOC_NORMAL);" call in old line 1676. I do not think NOWAIT will be a good idea if one "swapoff" a swap device out of many during a heavy slashing/paging period. I recall these 2 pairs of functions, vm_page_grab* and swap_pager_getpages, need to be called in pair. I think I removed one of them and resulted in a panic. It was like a year ago when I made the first change. I will try if I can remove swap_pager_haspage call. ota_j.email.ne.jp: This vm_page_grab_pages() call corresponds to "m = vm_page_grab(object, pindex… | |||||
for (i = j = 0;; i++) { | |||||
Done Inline Actionsstyle(9) doesn't require a blank line here. It can be deleted. alc: style(9) doesn't require a blank line here. It can be deleted. | |||||
if (i < npages && ma[i]->valid != VM_PAGE_BITS_ALL) | |||||
Done Inline ActionsI find the nested loops a bit confusing, and this a bit simpler: if (i < npages && m[i]->valid != VM_PAGE_BITS_ALL) continue; if (j < i && swap_pager_getpages(object, &m[j], i - j, NULL, NULL) != VM_PAGER_OK) panic("swp_pager_force_pagein: read from swap failed"); while (j < i) swp_pager_launder_vmpage(m[j++]); if (i == npages) break; swp_pager_activate_vmpage(m[j++]); } dougm: I find the nested loops a bit confusing, and this a bit simpler:
for (i = j = 0;; i++) {
if… | |||||
continue; | |||||
if (j < i && | |||||
(swap_pager_getpages(object, &ma[j], i - j, NULL, NULL) != VM_PAGER_OK)) | |||||
dougmUnsubmitted Done Inline Actions4 space indent for line wrap. dougm: 4 space indent for line wrap.
And this goes way past 80 columns. | |||||
panic("swp_pager_force_pagein: read from swap failed"); | |||||
Done Inline ActionsContinue is not needed there. I would write this as for (i = 0; i < npages; i++) { if (m[i]->valid != VM_PAGE_BITS_ALL) break; swp_pager_activate_vmpage(m[i]); } kib: Continue is not needed there. I would write this as
```
for (i = 0; i < npages; i++) {
if… | |||||
Done Inline Actionsswp_pager_force_pagein() used to only act on a single page before this change. Now, it does for multiple npages. i < npages loop is to mark already in memory pages to "active" one by one. We don't need to page-in for these. Once find a page that we need to page-in, we need to find a continuous region of not-on-memory pages to page-in from backing store as we want to pass as big "count" as possible. There may be more than on regions within npages. I'm changing to if-else statement and see if that makes easier to read. ota_j.email.ne.jp: swp_pager_force_pagein() used to only act on a single page before this change.
Now, it does… | |||||
while (j < i) | |||||
swp_pager_force_launder(ma[j++]); | |||||
if (i == npages) | |||||
Done Inline ActionsBetter add {} around loop body, because it is multiline. kib: Better add {} around loop body, because it is multiline. | |||||
Not Done Inline ActionsThere is a space after ma[i] that should be deleted. alc: There is a space after ma[i] that should be deleted. | |||||
break; | |||||
Done Inline ActionsThe variable 'count' is not necessary. dougm: The variable 'count' is not necessary. | |||||
swp_pager_force_dirty(ma[j++]); | |||||
Not Done Inline ActionsIt makes sense to add assert that the object is locked. kib: It makes sense to add assert that the object is locked. | |||||
Not Done Inline ActionsIs that one of VM_OBJECT_ASSERT_WLOCKED(object) or VM_OBJECT_ASSERT_LOCKED(object) ota_j.email.ne.jp: Is that one of VM_OBJECT_ASSERT_WLOCKED(object) or VM_OBJECT_ASSERT_LOCKED(object) | |||||
Done Inline ActionsAdd a space between "if" and "(". dougm: Add a space between "if" and "(". | |||||
} | |||||
Done Inline Actionsswap->swp in panic message dougm: swap->swp in panic message | |||||
return (npages); | |||||
Done Inline Actionsreturn (npages); kib: return (npages); | |||||
} | |||||
Done Inline ActionsWhy is this call needed ? kib: Why is this call needed ? | |||||
Done Inline ActionsTo be honest, I don't know. My objective was to call swap_pager_getpages() with more than 1 page to page-in faster. I kept the rest of operations same. Existing code calls swp_pager_force_dirty() equivalent if (m->valid == VM_PAGE_BITS_ALL). Otherwise, swp_pager_force_launder() is called for each of vm_page_t m. ota_j.email.ne.jp: To be honest, I don't know. My objective was to call swap_pager_getpages() with more than 1… | |||||
Done Inline ActionsSorry, I misread the code, I thought that only one page is dirtied. The previous pages in the run are dirtied with the call to swp_pager_force_launder(). The reason for dirtying is that clean pages in swap objects are either zero or have a copy in swap, by definition of clean. So if we are removing the swap, we must ensure that the user content is not going away. kib: Sorry, I misread the code, I thought that only one page is dirtied. The previous pages in the… | |||||
Done Inline ActionsPerhaps XXX comment is useless. kib: Perhaps XXX comment is useless. | |||||
Done Inline ActionsBreaks the 80 column limit. dougm: Breaks the 80 column limit. | |||||
/* | |||||
Done Inline ActionsNo need for {}, loop body is single-line. kib: No need for {}, loop body is single-line. | |||||
* swap_pager_swapoff: | * swap_pager_swapoff: | ||||
Not Done Inline ActionsDeep inside of swp_pager_force_pagein() we will release the object lock, allowing concurrent activity to potentially deallocate sb. So, subsequent lines should not use sb. Instead, the loop should be terminated early and the trie lookup repeated on the old "pi" value. alc: Deep inside of swp_pager_force_pagein() we will release the object lock, allowing concurrent… | |||||
* | * | ||||
* Page in all of the pages that have been paged out to the | * Page in all of the pages that have been paged out to the | ||||
Not Done Inline ActionsI don't see the need for "i = 0" here. alc: I don't see the need for "i = 0" here. | |||||
* given device. The corresponding blocks in the bitmap must be | * given device. The corresponding blocks in the bitmap must be | ||||
Not Done Inline Actionsn_blks must not exceed MAXPHYS / PAGE_SIZE. alc: n_blks must not exceed MAXPHYS / PAGE_SIZE. | |||||
* marked as allocated and the device must be flagged SW_CLOSING. | * marked as allocated and the device must be flagged SW_CLOSING. | ||||
* There may be no processes swapped out to the device. | * There may be no processes swapped out to the device. | ||||
* | * | ||||
* This routine may block. | * This routine may block. | ||||
*/ | */ | ||||
static void | static void | ||||
swap_pager_swapoff(struct swdevt *sp) | swap_pager_swapoff(struct swdevt *sp) | ||||
{ | { | ||||
struct swblk *sb; | struct swblk *sb; | ||||
vm_object_t object; | vm_object_t object; | ||||
vm_pindex_t pi; | vm_pindex_t pi; | ||||
int i, retries; | int freedpages, i, offset, retries; | ||||
sx_assert(&swdev_syscall_lock, SA_XLOCKED); | sx_assert(&swdev_syscall_lock, SA_XLOCKED); | ||||
retries = 0; | retries = 0; | ||||
full_rescan: | full_rescan: | ||||
mtx_lock(&vm_object_list_mtx); | mtx_lock(&vm_object_list_mtx); | ||||
TAILQ_FOREACH(object, &vm_object_list, object_list) { | TAILQ_FOREACH(object, &vm_object_list, object_list) { | ||||
if (object->type != OBJT_SWAP) | if (object->type != OBJT_SWAP) | ||||
Show All 13 Lines | TAILQ_FOREACH(object, &vm_object_list, object_list) { | ||||
* initialization. We must not access pctrie below | * initialization. We must not access pctrie below | ||||
* unless we checked that our object is swap and not | * unless we checked that our object is swap and not | ||||
* dead. | * dead. | ||||
*/ | */ | ||||
atomic_thread_fence_acq(); | atomic_thread_fence_acq(); | ||||
if (object->type != OBJT_SWAP) | if (object->type != OBJT_SWAP) | ||||
goto next_obj; | goto next_obj; | ||||
freedpages = 0; | |||||
for (pi = 0; (sb = SWAP_PCTRIE_LOOKUP_GE( | for (pi = 0; (sb = SWAP_PCTRIE_LOOKUP_GE( | ||||
&object->un_pager.swp.swp_blks, pi)) != NULL; ) { | &object->un_pager.swp.swp_blks, pi)) != NULL; ) { | ||||
pi = sb->p + SWAP_META_PAGES; | pi = sb->p + SWAP_META_PAGES; | ||||
for (i = 0; i < SWAP_META_PAGES; i++) { | /* | ||||
if (sb->d[i] == SWAPBLK_NONE) | * Last swp_pager_force_pagein() call already paged-in | ||||
dougmUnsubmitted Done Inline ActionsI believe that the comment is true if the freedpages>=SWAP_META_PAGES condition is true, but not otherwise. So move the comment to the block after the "if". dougm: I believe that the comment is true if the freedpages>=SWAP_META_PAGES condition is true, but… | |||||
* this block and thus we skip one. | |||||
Done Inline ActionsWhy do you need offset variable at all ? Isn't freedpages alone contains the necessary information ? kib: Why do you need offset variable at all ? Isn't freedpages alone contains the necessary… | |||||
Done Inline ActionsThis was a case when we already paged-in all of the array elements of a d[] and I wanted to skip the below loop. Combined with my comment and intention below, I can simplify. I will make change, test and then upload. ota_j.email.ne.jp: This was a case when we already paged-in all of the array elements of a d[] and I wanted to… | |||||
Done Inline ActionsI didn't need to use offset at this location. ota_j.email.ne.jp: I didn't need to use offset at this location. | |||||
*/ | |||||
if (freedpages >= SWAP_META_PAGES) { | |||||
freedpages -= SWAP_META_PAGES; | |||||
continue; | continue; | ||||
if (swp_pager_isondev(sb->d[i], sp)) | } | ||||
swp_pager_force_pagein(object, | for (i = freedpages, freedpages = 0; i < SWAP_META_PAGES;) { | ||||
if (swp_pager_isondev(sb->d[i], sp)) { | |||||
freedpages = swp_pager_force_pagein(object, | |||||
sb->p + i); | sb->p + i); | ||||
offset = min(freedpages, SWAP_META_PAGES - i); | |||||
Done Inline ActionsIt's not as if I understand this code, but still... Suppose that on the i=SWAP_META_PAGES-1 iteration of this loop, isondev is TRUE and freedpages is set to one million. Then offset will be set to SWAP_META_PAGES=1, and on the next loop test i will be 2*SWAP_META_PAGES-2, not SWAP_META_PAGES. So I know that we're leaving the loop and i won't be examined again, but is it bad that i is so big? Shouldn't offset be limited so that i can't go past SWAP_META_PAGES, and freedpages can't be reduced so much? dougm: It's not as if I understand this code, but still...
Suppose that on the i=SWAP_META_PAGES-1… | |||||
Done Inline ActionsIt looks you got the idea based on the comment below. This was because swp_pager_force_pagein() can page-in more pages than sb->d[] can hold. For example, sb-d[] is 8-element array on i386. But swp_pager_force_pagein() can page-in up to 32 pages. The "offset" variable was originally to track how many arrays to skip. ota_j.email.ne.jp: It looks you got the idea based on the comment below.
This was because swp_pager_force_pagein… | |||||
Done Inline ActionsThis is updated to use while loop and depend on outer for loop. Basically, the idea is to advance array index based on the number of page-ins completed. With new code, a single page-in operation can pull pages back more than the size of d[] array. ota_j.email.ne.jp: This is updated to use while loop and depend on outer for loop.
New code looks simpler. | |||||
freedpages -= offset; | |||||
dougmUnsubmitted Done Inline ActionsWould it be simpler to make the following changes:
i += swp_pager_force_pagein(..); after the for loop, write freedpages = i - SWAP_META_PAGES; Then you don't have to worry about the value of freedpages within the loop. Also, what if swp_pager_force_pagein return 1, rather than failing an assertion, when swap_pager_haspage failed? Could you then drop the swp_pager_isondev test here and just call swp_pager_force_pagein in every iteration? dougm: Would it be simpler to make the following changes:
1. instead of using "offset", just write
i… | |||||
ota_j.email.ne.jpAuthorUnsubmitted Done Inline ActionsThanks for this input. I made suggested changes and then realized I can further simplify. ota_j.email.ne.jp: Thanks for this input. I made suggested changes and then realized I can further simplify. | |||||
i += offset; | |||||
Done Inline Actions() are not needed. The '-1' seems to undo the loop step, then change the loop body to if (sb->d[i] != SWAPBLK_NONE && swp_pager_isondev(...)) { ... } else i++; and remove -1. kib: () are not needed. The '-1' seems to undo the loop step, then change the loop body to
```
if… | |||||
Done Inline ActionsI wanted to get out of the inner loop. I didn't need the exact number of pages... ota_j.email.ne.jp: I wanted to get out of the inner loop. I didn't need the exact number of pages... | |||||
Done Inline Actionsswp_pager_isondev() never returns true for SWAPBLK_NONE case. ota_j.email.ne.jp: swp_pager_isondev() never returns true for SWAPBLK_NONE case.
I think I can drop the comparison… | |||||
} else | |||||
i++; | |||||
} | } | ||||
} | } | ||||
next_obj: | next_obj: | ||||
VM_OBJECT_WUNLOCK(object); | VM_OBJECT_WUNLOCK(object); | ||||
mtx_lock(&vm_object_list_mtx); | mtx_lock(&vm_object_list_mtx); | ||||
} | } | ||||
mtx_unlock(&vm_object_list_mtx); | mtx_unlock(&vm_object_list_mtx); | ||||
dougmUnsubmitted Done Inline ActionsCan freedpages be > 0 when you get here? If so, will this be a problem? If not, can you add an assertion? dougm: Can freedpages be > 0 when you get here? If so, will this be a problem? If not, can you add… | |||||
ota_j.email.ne.jpAuthorUnsubmitted Done Inline ActionsI deleted freepages variable. I need to check again. ota_j.email.ne.jp: I deleted freepages variable. I need to check again. | |||||
ota_j.email.ne.jpAuthorUnsubmitted Done Inline ActionsNew variable here is now 'i' and this must be 0 when code reaches here. In order for this 'i' to be greater than 0, we must have paged in more than we thought we paged out, based on our sb->d[]. This function is only called by "swapoff" command to detach a swap device. Should I add something like KASSERT( i > 0, "we paged-in more than we had paged-out" )? ota_j.email.ne.jp: New variable here is now 'i' and this must be 0 when code reaches here.
In order for this 'i'… | |||||
dougmUnsubmitted Done Inline ActionsIf you know that i must be 0, then you should add KASSERT(i == 0 ...). dougm: If you know that i must be 0, then you should add KASSERT(i == 0 ...). | |||||
ota_j.email.ne.jpAuthorUnsubmitted Done Inline ActionsOkay, I will add KASSERT. ota_j.email.ne.jp: Okay, I will add KASSERT. | |||||
if (sp->sw_used) { | if (sp->sw_used) { | ||||
/* | /* | ||||
* Objects may be locked or paging to the device being | * Objects may be locked or paging to the device being | ||||
* removed, so we will miss their pages and need to | * removed, so we will miss their pages and need to | ||||
* make another pass. We have marked this device as | * make another pass. We have marked this device as | ||||
* SW_CLOSING, so the activity should finish soon. | * SW_CLOSING, so the activity should finish soon. | ||||
*/ | */ | ||||
▲ Show 20 Lines • Show All 580 Lines • ▼ Show 20 Lines | #endif | ||||
if (vm_free_count() + swap_pager_avail < nblks + nswap_lowat) | if (vm_free_count() + swap_pager_avail < nblks + nswap_lowat) | ||||
return (ENOMEM); | return (ENOMEM); | ||||
/* | /* | ||||
* Prevent further allocations on this device. | * Prevent further allocations on this device. | ||||
*/ | */ | ||||
mtx_lock(&sw_dev_mtx); | mtx_lock(&sw_dev_mtx); | ||||
sp->sw_flags |= SW_CLOSING; | sp->sw_flags |= SW_CLOSING; | ||||
swap_pager_avail -= blist_fill(sp->sw_blist, 0, nblks); | swap_pager_avail -= blist_fill(sp->sw_blist, 0, nblks); | ||||
Not Done Inline ActionsIt seems like this line is now part of a different patch being reviewed elsewhere. In that case, please remove it from here. dougm: It seems like this line is now part of a different patch being reviewed elsewhere. In that… | |||||
swap_total -= nblks; | swap_total -= nblks; | ||||
mtx_unlock(&sw_dev_mtx); | mtx_unlock(&sw_dev_mtx); | ||||
/* | /* | ||||
* Page in the contents of the device and close it. | * Page in the contents of the device and close it. | ||||
*/ | */ | ||||
swap_pager_swapoff(sp); | swap_pager_swapoff(sp); | ||||
▲ Show 20 Lines • Show All 564 Lines • Show Last 20 Lines |
I would suggest the name swp_pager_force_dirty. In particular, the page may not be in the active queue.