Changeset View
Changeset View
Standalone View
Standalone View
sys/vm/vm_reserv.c
Show First 20 Lines • Show All 1,338 Lines • ▼ Show 20 Lines | if (pa + size > high) { | ||||
/* This entire reservation is too high; go to next. */ | /* This entire reservation is too high; go to next. */ | ||||
continue; | continue; | ||||
} | } | ||||
if (vm_reserv_trylock(rv) == 0) { | if (vm_reserv_trylock(rv) == 0) { | ||||
TAILQ_INSERT_AFTER(queue, rv, marker, partpopq); | TAILQ_INSERT_AFTER(queue, rv, marker, partpopq); | ||||
vm_reserv_domain_unlock(domain); | vm_reserv_domain_unlock(domain); | ||||
vm_reserv_lock(rv); | vm_reserv_lock(rv); | ||||
if (!rv->inpartpopq || | if (TAILQ_PREV(marker, vm_reserv_queue, partpopq) != | ||||
TAILQ_NEXT(rv, partpopq) != marker) { | rv) { | ||||
alc: Umm, shouldn't this really be using TAILQ_PREV("marker") == rv? Unless TRASHIT() is guaranteed… | |||||
Done Inline ActionsIt's true that TAILQ_REMOVE() will leave the "next" field unchanged, but if the reservation is removed from the queue, !rv->inpartpopq will be true and we'll short-circuit. I think you're right that we could change this to if (TAILQ_PREV(marker, vm_reserv_queue, partpoq) != rv), but is the current code in fact incorrect? markj: It's true that TAILQ_REMOVE() will leave the "next" field unchanged, but if the reservation is… | |||||
Not Done Inline ActionsNo, it's going to work. I shouldn't have implied otherwise. Let me instead say that using TAILQ_PREV("marker") is just going to be easier to reason about. alc: No, it's going to work. I shouldn't have implied otherwise. Let me instead say that using… | |||||
vm_reserv_unlock(rv); | vm_reserv_unlock(rv); | ||||
vm_reserv_domain_lock(domain); | vm_reserv_domain_lock(domain); | ||||
rvn = TAILQ_NEXT(marker, partpopq); | rvn = TAILQ_NEXT(marker, partpopq); | ||||
TAILQ_REMOVE(queue, marker, partpopq); | TAILQ_REMOVE(queue, marker, partpopq); | ||||
continue; | continue; | ||||
} | } | ||||
vm_reserv_domain_lock(domain); | vm_reserv_domain_lock(domain); | ||||
TAILQ_REMOVE(queue, marker, partpopq); | TAILQ_REMOVE(queue, marker, partpopq); | ||||
} | } | ||||
vm_reserv_domain_unlock(domain); | vm_reserv_domain_unlock(domain); | ||||
if (vm_reserv_test_contig(rv, npages, low, high, | if (vm_reserv_test_contig(rv, npages, low, high, | ||||
alignment, boundary)) { | alignment, boundary)) { | ||||
vm_reserv_domain_scan_unlock(domain); | vm_reserv_domain_scan_unlock(domain); | ||||
vm_reserv_reclaim(rv); | vm_reserv_reclaim(rv); | ||||
vm_reserv_unlock(rv); | vm_reserv_unlock(rv); | ||||
return (true); | return (true); | ||||
} | } | ||||
vm_reserv_unlock(rv); | |||||
vm_reserv_domain_lock(domain); | vm_reserv_domain_lock(domain); | ||||
rvn = TAILQ_NEXT(rv, partpopq); | |||||
vm_reserv_unlock(rv); | |||||
} | } | ||||
vm_reserv_domain_unlock(domain); | vm_reserv_domain_unlock(domain); | ||||
vm_reserv_domain_scan_unlock(domain); | vm_reserv_domain_scan_unlock(domain); | ||||
return (false); | return (false); | ||||
} | } | ||||
/* | /* | ||||
* Transfers the reservation underlying the given page to a new object. | * Transfers the reservation underlying the given page to a new object. | ||||
▲ Show 20 Lines • Show All 125 Lines • Show Last 20 Lines |
Umm, shouldn't this really be using TAILQ_PREV("marker") == rv? Unless TRASHIT() is guaranteed to be enabled, can't TAILQ_REMOVE("rv") leave rv's "next" field unchanged? See the old function vm_pageout_fallback_object_lock() in 11.x.
Further, I believe that fixing this makes testing !rv->inpartpopq redundant.