Changeset View
Standalone View
sys/vm/vm_map.c
Show First 20 Lines • Show All 2,831 Lines • ▼ Show 20 Lines | while (entry->start < end) { | ||||
vm_map_simplify_entry(map, entry); | vm_map_simplify_entry(map, entry); | ||||
entry = entry->next; | entry = entry->next; | ||||
} | } | ||||
vm_map_unlock(map); | vm_map_unlock(map); | ||||
return (KERN_SUCCESS); | return (KERN_SUCCESS); | ||||
} | } | ||||
/* | /* | ||||
* vm_map_entry_in_transition: | |||||
* | |||||
* Release the map lock, and sleep until the entry is no longer in | |||||
alc: The "unlock" and "sleep" happen atomically, so I would suggest. "... map lock, and sleep until . | |||||
* transition. Awake and acquire the map lock. If the map changed while | |||||
Done Inline ActionsThis comment misses the big picture. The "getting" is really a side-effect of having to release the map lock. Specifically, while we slept, the entry could have been mangled or even destroyed (as described in a comment below). That is why we perform the lookup if the time stamp shows that the map has changed in some way. However, the expected case is that the in-transition entry still exists. alc: This comment misses the big picture. The "getting" is really a side-effect of having to… | |||||
Not Done Inline ActionsYou have a mix of two spaces and one space after periods. For consistency, I would suggest adding a second space here: "... map lock. If the ..." alc: You have a mix of two spaces and one space after periods. For consistency, I would suggest… | |||||
* another held the lock, lookup a possibly-changed entry at or after the | |||||
* 'start' position of the old entry. | |||||
*/ | |||||
static vm_map_entry_t | |||||
vm_map_entry_in_transition(vm_map_t map, vm_offset_t in_start, | |||||
vm_offset_t *io_end, bool holes_ok, vm_map_entry_t in_entry) | |||||
{ | |||||
vm_map_entry_t entry; | |||||
Done Inline Actionsvm_map.h's definition of the timestamp in struct vm_map uses the shorter spelling "u_int". I suggest that you change this to match. alc: vm_map.h's definition of the timestamp in struct vm_map uses the shorter spelling "u_int". I… | |||||
Done Inline ActionsWill do, even most other definitions of last_timestamp in *this* file use 'unsigned int'. dougm: Will do, even most other definitions of last_timestamp in *this* file use 'unsigned int'. | |||||
vm_offset_t start; | |||||
u_int last_timestamp; | |||||
Done Inline ActionsI still suggest to add assert about the map lock. kib: I still suggest to add assert about the map lock. | |||||
VM_MAP_ASSERT_LOCKED(map); | |||||
KASSERT((in_entry->eflags & MAP_ENTRY_IN_TRANSITION) != 0, | |||||
("not in-tranition map entry %p", in_entry)); | |||||
/* | |||||
* We have not yet clipped the entry. | |||||
*/ | |||||
start = MAX(in_start, in_entry->start); | |||||
in_entry->eflags |= MAP_ENTRY_NEEDS_WAKEUP; | |||||
last_timestamp = map->timestamp; | |||||
if (vm_map_unlock_and_wait(map, 0)) { | |||||
/* | |||||
* Allow interruption of user wiring/unwiring? | |||||
*/ | |||||
} | |||||
vm_map_lock(map); | |||||
if (last_timestamp + 1 == map->timestamp) | |||||
return (in_entry); | |||||
/* | |||||
* Look again for the entry because the map was modified while it was | |||||
* unlocked. Specifically, the entry may have been clipped, merged, or | |||||
* deleted. | |||||
*/ | |||||
if (!vm_map_lookup_entry(map, start, &entry)) { | |||||
if (!holes_ok) { | |||||
*io_end = start; | |||||
return (NULL); | |||||
} | |||||
entry = entry->next; | |||||
} | |||||
return (entry); | |||||
} | |||||
/* | |||||
* vm_map_unwire: | * vm_map_unwire: | ||||
* | * | ||||
* Implements both kernel and user unwiring. | * Implements both kernel and user unwiring. | ||||
*/ | */ | ||||
int | int | ||||
vm_map_unwire(vm_map_t map, vm_offset_t start, vm_offset_t end, | vm_map_unwire(vm_map_t map, vm_offset_t start, vm_offset_t end, | ||||
int flags) | int flags) | ||||
{ | { | ||||
vm_map_entry_t entry, first_entry, tmp_entry; | vm_map_entry_t entry, first_entry; | ||||
vm_offset_t saved_start; | |||||
unsigned int last_timestamp; | |||||
int rv; | int rv; | ||||
bool holes_ok, need_wakeup, user_unwire; | bool first_iteration, holes_ok, need_wakeup, user_unwire; | ||||
if (start == end) | if (start == end) | ||||
return (KERN_SUCCESS); | return (KERN_SUCCESS); | ||||
holes_ok = (flags & VM_MAP_WIRE_HOLESOK) != 0; | holes_ok = (flags & VM_MAP_WIRE_HOLESOK) != 0; | ||||
user_unwire = (flags & VM_MAP_WIRE_USER) != 0; | user_unwire = (flags & VM_MAP_WIRE_USER) != 0; | ||||
vm_map_lock(map); | vm_map_lock(map); | ||||
VM_MAP_RANGE_CHECK(map, start, end); | VM_MAP_RANGE_CHECK(map, start, end); | ||||
if (!vm_map_lookup_entry(map, start, &first_entry)) { | if (!vm_map_lookup_entry(map, start, &first_entry)) { | ||||
if (holes_ok) | if (holes_ok) | ||||
first_entry = first_entry->next; | first_entry = first_entry->next; | ||||
else { | else { | ||||
vm_map_unlock(map); | vm_map_unlock(map); | ||||
return (KERN_INVALID_ADDRESS); | return (KERN_INVALID_ADDRESS); | ||||
} | } | ||||
} | } | ||||
last_timestamp = map->timestamp; | first_iteration = true; | ||||
entry = first_entry; | entry = first_entry; | ||||
rv = KERN_SUCCESS; | rv = KERN_SUCCESS; | ||||
while (entry->start < end) { | while (entry->start < end) { | ||||
if (entry->eflags & MAP_ENTRY_IN_TRANSITION) { | if (entry->eflags & MAP_ENTRY_IN_TRANSITION) { | ||||
/* | /* | ||||
* We have not yet clipped the entry. | * We have not yet clipped the entry. | ||||
*/ | */ | ||||
saved_start = (start >= entry->start) ? start : | entry = vm_map_entry_in_transition(map, start, &end, | ||||
entry->start; | holes_ok, entry); | ||||
entry->eflags |= MAP_ENTRY_NEEDS_WAKEUP; | if (entry == NULL) { | ||||
if (vm_map_unlock_and_wait(map, 0)) { | if (first_iteration) { | ||||
/* | |||||
* Allow interruption of user unwiring? | |||||
*/ | |||||
} | |||||
vm_map_lock(map); | |||||
if (last_timestamp+1 != map->timestamp) { | |||||
/* | |||||
* Look again for the entry because the map was | |||||
* modified while it was unlocked. | |||||
* Specifically, the entry may have been | |||||
* clipped, merged, or deleted. | |||||
*/ | |||||
if (!vm_map_lookup_entry(map, saved_start, | |||||
&tmp_entry)) { | |||||
if (holes_ok) | |||||
tmp_entry = tmp_entry->next; | |||||
else { | |||||
if (saved_start == start) { | |||||
/* | |||||
* First_entry has been deleted. | |||||
*/ | |||||
vm_map_unlock(map); | vm_map_unlock(map); | ||||
return (KERN_INVALID_ADDRESS); | return (KERN_INVALID_ADDRESS); | ||||
} | } | ||||
end = saved_start; | |||||
rv = KERN_INVALID_ADDRESS; | rv = KERN_INVALID_ADDRESS; | ||||
break; | break; | ||||
Done Inline ActionsThis looks wrong. Specifically, the pointer "entry" may no longer point to a valid map entry after sleeping in vm_map_entry_in_transition(). alc: This looks wrong. Specifically, the pointer "entry" may no longer point to a valid map entry… | |||||
} | } | ||||
} | first_entry = first_iteration ? entry : NULL; | ||||
if (entry == first_entry) | |||||
first_entry = tmp_entry; | |||||
else | |||||
first_entry = NULL; | |||||
entry = tmp_entry; | |||||
} | |||||
last_timestamp = map->timestamp; | |||||
continue; | continue; | ||||
} | } | ||||
first_iteration = false; | |||||
vm_map_clip_start(map, entry, start); | vm_map_clip_start(map, entry, start); | ||||
vm_map_clip_end(map, entry, end); | vm_map_clip_end(map, entry, end); | ||||
/* | /* | ||||
* Mark the entry in case the map lock is released. (See | * Mark the entry in case the map lock is released. (See | ||||
* above.) | * above.) | ||||
*/ | */ | ||||
KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 && | KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 && | ||||
entry->wiring_thread == NULL, | entry->wiring_thread == NULL, | ||||
▲ Show 20 Lines • Show All 150 Lines • ▼ Show 20 Lines | |||||
int | int | ||||
vm_map_wire_locked(vm_map_t map, vm_offset_t start, vm_offset_t end, int flags) | vm_map_wire_locked(vm_map_t map, vm_offset_t start, vm_offset_t end, int flags) | ||||
{ | { | ||||
vm_map_entry_t entry, first_entry, tmp_entry; | vm_map_entry_t entry, first_entry, tmp_entry; | ||||
vm_offset_t faddr, saved_end, saved_start; | vm_offset_t faddr, saved_end, saved_start; | ||||
u_long npages; | u_long npages; | ||||
u_int last_timestamp; | u_int last_timestamp; | ||||
int rv; | int rv; | ||||
bool holes_ok, need_wakeup, user_wire; | bool first_iteration, holes_ok, need_wakeup, user_wire; | ||||
vm_prot_t prot; | vm_prot_t prot; | ||||
VM_MAP_ASSERT_LOCKED(map); | VM_MAP_ASSERT_LOCKED(map); | ||||
if (start == end) | if (start == end) | ||||
return (KERN_SUCCESS); | return (KERN_SUCCESS); | ||||
prot = 0; | prot = 0; | ||||
if (flags & VM_MAP_WIRE_WRITE) | if (flags & VM_MAP_WIRE_WRITE) | ||||
prot |= VM_PROT_WRITE; | prot |= VM_PROT_WRITE; | ||||
holes_ok = (flags & VM_MAP_WIRE_HOLESOK) != 0; | holes_ok = (flags & VM_MAP_WIRE_HOLESOK) != 0; | ||||
user_wire = (flags & VM_MAP_WIRE_USER) != 0; | user_wire = (flags & VM_MAP_WIRE_USER) != 0; | ||||
VM_MAP_RANGE_CHECK(map, start, end); | VM_MAP_RANGE_CHECK(map, start, end); | ||||
if (!vm_map_lookup_entry(map, start, &first_entry)) { | if (!vm_map_lookup_entry(map, start, &first_entry)) { | ||||
if (holes_ok) | if (holes_ok) | ||||
first_entry = first_entry->next; | first_entry = first_entry->next; | ||||
else | else | ||||
return (KERN_INVALID_ADDRESS); | return (KERN_INVALID_ADDRESS); | ||||
} | } | ||||
last_timestamp = map->timestamp; | first_iteration = true; | ||||
entry = first_entry; | entry = first_entry; | ||||
while (entry->start < end) { | while (entry->start < end) { | ||||
if (entry->eflags & MAP_ENTRY_IN_TRANSITION) { | if (entry->eflags & MAP_ENTRY_IN_TRANSITION) { | ||||
/* | /* | ||||
* We have not yet clipped the entry. | * We have not yet clipped the entry. | ||||
*/ | */ | ||||
saved_start = (start >= entry->start) ? start : | entry = vm_map_entry_in_transition(map, start, &end, | ||||
entry->start; | holes_ok, entry); | ||||
entry->eflags |= MAP_ENTRY_NEEDS_WAKEUP; | if (entry == NULL) { | ||||
if (vm_map_unlock_and_wait(map, 0)) { | if (first_iteration) | ||||
/* | |||||
* Allow interruption of user wiring? | |||||
*/ | |||||
} | |||||
vm_map_lock(map); | |||||
if (last_timestamp + 1 != map->timestamp) { | |||||
/* | |||||
* Look again for the entry because the map was | |||||
* modified while it was unlocked. | |||||
* Specifically, the entry may have been | |||||
* clipped, merged, or deleted. | |||||
*/ | |||||
if (!vm_map_lookup_entry(map, saved_start, | |||||
&tmp_entry)) { | |||||
if (holes_ok) | |||||
tmp_entry = tmp_entry->next; | |||||
else { | |||||
if (saved_start == start) { | |||||
/* | |||||
* first_entry has been deleted. | |||||
*/ | |||||
return (KERN_INVALID_ADDRESS); | return (KERN_INVALID_ADDRESS); | ||||
} | |||||
end = saved_start; | |||||
rv = KERN_INVALID_ADDRESS; | rv = KERN_INVALID_ADDRESS; | ||||
Done Inline ActionsDitto. alc: Ditto. | |||||
goto done; | goto done; | ||||
} | } | ||||
} | first_entry = first_iteration ? entry : NULL; | ||||
if (entry == first_entry) | |||||
first_entry = tmp_entry; | |||||
else | |||||
first_entry = NULL; | |||||
entry = tmp_entry; | |||||
} | |||||
last_timestamp = map->timestamp; | |||||
continue; | continue; | ||||
} | } | ||||
first_iteration = false; | |||||
vm_map_clip_start(map, entry, start); | vm_map_clip_start(map, entry, start); | ||||
vm_map_clip_end(map, entry, end); | vm_map_clip_end(map, entry, end); | ||||
/* | /* | ||||
* Mark the entry in case the map lock is released. (See | * Mark the entry in case the map lock is released. (See | ||||
* above.) | * above.) | ||||
*/ | */ | ||||
KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 && | KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 && | ||||
entry->wiring_thread == NULL, | entry->wiring_thread == NULL, | ||||
Show All 21 Lines | if ((entry->protection & (VM_PROT_READ | VM_PROT_EXECUTE)) == 0 | ||||
} | } | ||||
/* | /* | ||||
* Release the map lock, relying on the in-transition | * Release the map lock, relying on the in-transition | ||||
* mark. Mark the map busy for fork. | * mark. Mark the map busy for fork. | ||||
*/ | */ | ||||
saved_start = entry->start; | saved_start = entry->start; | ||||
saved_end = entry->end; | saved_end = entry->end; | ||||
last_timestamp = map->timestamp; | |||||
Done Inline ActionsWhy did you moved the last_timestamp memorization ? kib: Why did you moved the last_timestamp memorization ? | |||||
Done Inline ActionsIf I simply move the assignment back to where it was, the compiler will complain that last_timestamp is uninitialized in line 3188. I could fix that by inserting another assignment above - like after 'first_pass = FALSE'. last_timestamp is only used in this wired_count==0 block, so there's no point in setting it elsewhere, or updating the value for anywhere else in the code. dougm: If I simply move the assignment back to where it was, the compiler will complain that… | |||||
vm_map_busy(map); | vm_map_busy(map); | ||||
vm_map_unlock(map); | vm_map_unlock(map); | ||||
faddr = saved_start; | faddr = saved_start; | ||||
do { | do { | ||||
/* | /* | ||||
* Simulate a fault to get the page and enter | * Simulate a fault to get the page and enter | ||||
* it into the physical map. | * it into the physical map. | ||||
Show All 29 Lines | if ((entry->protection & (VM_PROT_READ | VM_PROT_EXECUTE)) == 0 | ||||
*/ | */ | ||||
if (rv != KERN_SUCCESS && | if (rv != KERN_SUCCESS && | ||||
faddr < entry->end) | faddr < entry->end) | ||||
vm_map_wire_entry_failure(map, | vm_map_wire_entry_failure(map, | ||||
entry, faddr); | entry, faddr); | ||||
entry = entry->next; | entry = entry->next; | ||||
} | } | ||||
} | } | ||||
last_timestamp = map->timestamp; | |||||
if (rv != KERN_SUCCESS) { | if (rv != KERN_SUCCESS) { | ||||
vm_map_wire_entry_failure(map, entry, faddr); | vm_map_wire_entry_failure(map, entry, faddr); | ||||
if (user_wire) | if (user_wire) | ||||
vm_map_wire_user_count_sub(npages); | vm_map_wire_user_count_sub(npages); | ||||
end = entry->end; | end = entry->end; | ||||
goto done; | goto done; | ||||
} | } | ||||
} else if (!user_wire || | } else if (!user_wire || | ||||
▲ Show 20 Lines • Show All 1,670 Lines • Show Last 20 Lines |
The "unlock" and "sleep" happen atomically, so I would suggest. "... map lock, and sleep until ..."