Changeset View
Standalone View
vm_reserv.c
Show First 20 Lines • Show All 1,024 Lines • ▼ Show 20 Lines | |||||
* population count and map are reset to their initial state. | * population count and map are reset to their initial state. | ||||
* | * | ||||
* The given reservation must not be in the partially populated reservation | * The given reservation must not be in the partially populated reservation | ||||
* queue. The free page queue lock must be held. | * queue. The free page queue lock must be held. | ||||
*/ | */ | ||||
static void | static void | ||||
vm_reserv_break(vm_reserv_t rv) | vm_reserv_break(vm_reserv_t rv) | ||||
{ | { | ||||
int begin_zeroes, hi, i, lo; | u_long changes; | ||||
int bitpos, hi, i, lo; | |||||
pho: Is this the right ordering of "int" and "u_long"? | |||||
vm_reserv_assert_locked(rv); | vm_reserv_assert_locked(rv); | ||||
CTR5(KTR_VM, "%s: rv %p object %p popcnt %d inpartpop %d", | CTR5(KTR_VM, "%s: rv %p object %p popcnt %d inpartpop %d", | ||||
__FUNCTION__, rv, rv->object, rv->popcnt, rv->inpartpopq); | __FUNCTION__, rv, rv->object, rv->popcnt, rv->inpartpopq); | ||||
vm_reserv_remove(rv); | vm_reserv_remove(rv); | ||||
rv->pages->psind = 0; | rv->pages->psind = 0; | ||||
i = hi = 0; | hi = lo = -1; | ||||
do { | for (i = 0; i <= NPOPMAP; i++) { | ||||
/* Find the next 0 bit. Any previous 0 bits are < "hi". */ | if (i == NPOPMAP) | ||||
lo = ffsl(~(((1UL << hi) - 1) | rv->popmap[i])); | changes = lo != hi; | ||||
Not Done Inline ActionsI suggest quoting "changes" to make it more clear that you're referring to a variable. markj: I suggest quoting "changes" to make it more clear that you're referring to a variable. | |||||
if (lo == 0) { | else { | ||||
/* Redundantly clears bits < "hi". */ | changes = rv->popmap[i]; | ||||
changes ^= (changes << 1) | (lo == hi); | |||||
rv->popmap[i] = 0; | rv->popmap[i] = 0; | ||||
rv->popcnt -= NBPOPMAP - hi; | |||||
while (++i < NPOPMAP) { | |||||
lo = ffsl(~rv->popmap[i]); | |||||
if (lo == 0) { | |||||
rv->popmap[i] = 0; | |||||
rv->popcnt -= NBPOPMAP; | |||||
} else | |||||
break; | |||||
} | } | ||||
if (i == NPOPMAP) | while (changes != 0) { | ||||
break; | bitpos = ffsl(changes) - 1; | ||||
Not Done Inline ActionsStyle: the comment formatting is off: /* * If the ... markj: Style: the comment formatting is off:
```
/*
* If the ...
``` | |||||
hi = 0; | changes ^= 1UL << bitpos; | ||||
} | if (lo == hi) | ||||
KASSERT(lo > 0, ("vm_reserv_break: lo is %d", lo)); | lo = NBPOPMAP * i + bitpos; | ||||
Not Done Inline ActionsI can't see how this works for a fully populated reservation. Suppose NPOMAP is 1. On both iterations of the outer loop, changes will be 1. On the first iteration, lo == hi == 0 and we set lo = 0. On the second iteration we will set lo = 64 but won't free any pages before exiting the loop. What am I missing? markj: I can't see how this works for a fully populated reservation. Suppose NPOMAP is 1. On both… | |||||
Done Inline ActionsBy "fully populated", I assume that you mean that all bits of rv->popmap[i] are set for all i. On the first iteration of the loop: so we do not set lo to 0, but it remains 0. I agree that we won't free any pages, but there are no pages to be freed, and this matches the current behavior. dougm: By "fully populated", I assume that you mean that all bits of rv->popmap[i] are set for all i. | |||||
Done Inline ActionsJust in case "fully populated" means all zeroes, we'll set lo to 0 on the first loop, and hi to 64 on the second, before freeing all 64 pages. dougm: Just in case "fully populated" means all zeroes, we'll set lo to 0 on the first loop, and hi to… | |||||
Not Done Inline ActionsSorry for the noise, I had an inverted sense of what this function is supposed to do. markj: Sorry for the noise, I had an inverted sense of what this function is supposed to do. | |||||
/* Convert from ffsl() to ordinary bit numbering. */ | else { | ||||
lo--; | hi = NBPOPMAP * i + bitpos; | ||||
if (lo > 0) { | |||||
/* Redundantly clears bits < "hi". */ | |||||
rv->popmap[i] &= ~((1UL << lo) - 1); | |||||
rv->popcnt -= lo - hi; | |||||
} | |||||
begin_zeroes = NBPOPMAP * i + lo; | |||||
/* Find the next 1 bit. */ | |||||
do | |||||
hi = ffsl(rv->popmap[i]); | |||||
while (hi == 0 && ++i < NPOPMAP); | |||||
if (i != NPOPMAP) | |||||
/* Convert from ffsl() to ordinary bit numbering. */ | |||||
hi--; | |||||
vm_domain_free_lock(VM_DOMAIN(rv->domain)); | vm_domain_free_lock(VM_DOMAIN(rv->domain)); | ||||
vm_phys_free_contig(&rv->pages[begin_zeroes], NBPOPMAP * i + | vm_phys_free_contig(&rv->pages[lo], hi - lo); | ||||
hi - begin_zeroes); | |||||
vm_domain_free_unlock(VM_DOMAIN(rv->domain)); | vm_domain_free_unlock(VM_DOMAIN(rv->domain)); | ||||
} while (i < NPOPMAP); | lo = hi; | ||||
KASSERT(rv->popcnt == 0, | } | ||||
("vm_reserv_break: reserv %p's popcnt is corrupted", rv)); | } | ||||
} | |||||
rv->popcnt = 0; | |||||
counter_u64_add(vm_reserv_broken, 1); | counter_u64_add(vm_reserv_broken, 1); | ||||
} | } | ||||
/* | /* | ||||
* Breaks all reservations belonging to the given object. | * Breaks all reservations belonging to the given object. | ||||
*/ | */ | ||||
void | void | ||||
vm_reserv_break_all(vm_object_t object) | vm_reserv_break_all(vm_object_t object) | ||||
▲ Show 20 Lines • Show All 419 Lines • Show Last 20 Lines |
Is this the right ordering of "int" and "u_long"?