Changeset View
Standalone View
sys/vm/vm_page.c
Show First 20 Lines • Show All 2,512 Lines • ▼ Show 20 Lines | |||||
#endif | #endif | ||||
mtx_unlock(&vm_page_queue_free_mtx); | mtx_unlock(&vm_page_queue_free_mtx); | ||||
if (order == VM_NFREEORDER) | if (order == VM_NFREEORDER) | ||||
error = EINVAL; | error = EINVAL; | ||||
} | } | ||||
} | } | ||||
if (m_mtx != NULL) | if (m_mtx != NULL) | ||||
mtx_unlock(m_mtx); | mtx_unlock(m_mtx); | ||||
if ((m = SLIST_FIRST(&free)) != NULL) { | vm_page_free_pages_toq(&free, false); | ||||
kib: IMO no point in checking the emptiness since vm_page_free_zero_pages() does. | |||||
mtx_lock(&vm_page_queue_free_mtx); | |||||
do { | |||||
SLIST_REMOVE_HEAD(&free, plinks.s.ss); | |||||
vm_page_free_phys(m); | |||||
} while ((m = SLIST_FIRST(&free)) != NULL); | |||||
vm_page_free_wakeup(); | |||||
mtx_unlock(&vm_page_queue_free_mtx); | |||||
} | |||||
return (error); | return (error); | ||||
} | } | ||||
#define NRUNS 16 | #define NRUNS 16 | ||||
CTASSERT(powerof2(NRUNS)); | CTASSERT(powerof2(NRUNS)); | ||||
#define RUN_INDEX(count) ((count) & (NRUNS - 1)) | #define RUN_INDEX(count) ((count) & (NRUNS - 1)) | ||||
▲ Show 20 Lines • Show All 498 Lines • ▼ Show 20 Lines | |||||
{ | { | ||||
if (!vm_page_free_prep(m, false)) | if (!vm_page_free_prep(m, false)) | ||||
return; | return; | ||||
mtx_lock(&vm_page_queue_free_mtx); | mtx_lock(&vm_page_queue_free_mtx); | ||||
vm_page_free_phys(m); | vm_page_free_phys(m); | ||||
vm_page_free_wakeup(); | vm_page_free_wakeup(); | ||||
mtx_unlock(&vm_page_queue_free_mtx); | mtx_unlock(&vm_page_queue_free_mtx); | ||||
} | |||||
/* | |||||
* vm_page_free_pages_toq: | |||||
* | |||||
* Returns a list of pages to the free list, disassociating it | |||||
* from any VM object. In other words, this is equivalent to | |||||
Done Inline Actions"In other words" markj: "In other words" | |||||
* calling vm_page_free_toq() for each page of a list of VM objects. | |||||
* | |||||
* The objects must be locked. The pages must be locked if it is | |||||
* managed. | |||||
*/ | |||||
void | |||||
vm_page_free_pages_toq(struct spglist *free, bool update_wire_count) | |||||
Done Inline ActionsUse bool instead of boolean_t, for the new code. kib: Use bool instead of boolean_t, for the new code. | |||||
Done Inline ActionsThe name is a bit misleading, since the input pages need not be zero-filled. Maybe vm_page_free_pages_toq() for consistency? markj: The name is a bit misleading, since the input pages need not be zero-filled. Maybe… | |||||
{ | |||||
vm_page_t m; | |||||
Done Inline ActionsStyle: "struct spglist *free" markj: Style: "struct spglist *free" | |||||
struct pglist pgl; | |||||
int count; | |||||
Done Inline ActionsMove pgl declaration before count. kib: Move pgl declaration before count. | |||||
if (SLIST_EMPTY(free)) | |||||
return; | |||||
count = 0; | |||||
TAILQ_INIT(&pgl); | |||||
Done Inline ActionsThis is structured somewhat oddly. Why not: while ((m = SLIST_FIRST(free)) != NULL) { SLIST_REMOVE_HEAD(free, plinks.s.ss); if (vm_page_free_prep(m, false)) ... } vm_page_free_phys_pglist(&pgl); markj: This is structured somewhat oddly. Why not:
```
while ((m = SLIST_FIRST(free)) != NULL) {… | |||||
Not Done Inline ActionsEMPTY check is moved outside and changed to while loop. ota_j.email.ne.jp: EMPTY check is moved outside and changed to while loop. | |||||
while ((m = SLIST_FIRST(free)) != NULL) { | |||||
count++; | |||||
Done Inline ActionsShouldn't we be incrementing count only for pages that are added to pgl? markj: Shouldn't we be incrementing count only for pages that are added to pgl? | |||||
Not Done Inline Actionspmap_free_zero_pages() adds the count of vm_page_free_toq() calls to atomic_subtract_int(&vm_cnt.v_wire_count, count). At least, this makes code backward compatible. I also wondered, though... ota_j.email.ne.jp: pmap_free_zero_pages() adds the count of vm_page_free_toq() calls to atomic_subtract_int… | |||||
Done Inline Actions'{' should be put on the previous line. kib: '{' should be put on the previous line. | |||||
SLIST_REMOVE_HEAD(free, plinks.s.ss); | |||||
if (vm_page_free_prep(m, false)) | |||||
TAILQ_INSERT_TAIL(&pgl, m, listq); | |||||
} | |||||
vm_page_free_phys_pglist(&pgl); | |||||
if (update_wire_count && count > 0) | |||||
Done Inline ActionsStyle: parens around return value. markj: Style: parens around return value. | |||||
Not Done Inline ActionsReturn value is removed as wire count is adjusted by this function. ota_j.email.ne.jp: Return value is removed as wire count is adjusted by this function. | |||||
Done Inline ActionsI do not see a point of checking the emptiness of the list, it is done in vm_page_free_phys_pglist(). kib: I do not see a point of checking the emptiness of the list, it is done in… | |||||
atomic_subtract_int(&vm_cnt.v_wire_count, count); | |||||
} | } | ||||
/* | /* | ||||
* vm_page_wire: | * vm_page_wire: | ||||
* | * | ||||
* Mark this page as wired down by yet | * Mark this page as wired down by yet | ||||
* another map, removing it from paging queues | * another map, removing it from paging queues | ||||
* as necessary. | * as necessary. | ||||
▲ Show 20 Lines • Show All 944 Lines • Show Last 20 Lines |
IMO no point in checking the emptiness since vm_page_free_zero_pages() does.