Every call to vm_map_entry_splay passes an address that is already the address of a node in the tree, or checks the map upper and lower bounds first. Except one place, in vm_map_lookup_entry. This change adds a check there, and avoids splaying in some cases.
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
The origin of this patch is that I am developing changes to vm_map_entry_splay that require the address to lie within the map range, and a Peter Holm test reveals that not to be the case in at least one instance:
vm_map_entry_splay(0,0,465,0,465,...) at vm_map_entry_splay+0x215/frame 0xdab0282c
vm_map_lookup_entry(dc5481c8,0,dab0295c,0,0,...) at vm_map_lookup_entry+0xaf/frame 0xdab02868
vm_map_lookup(dab02958,0,11,dab0295c,dab0294c,...) at vm_map_lookup+0x88/frame 0xdab02908
vm_fault_hold(dc5481c8,0,1,0,0,...) at vm_fault_hold+0xd1/frame 0xdab029e8
vm_fault(dc5481c8,0,1,0,0,...) at vm_fault+0x83/frame 0xdab02a10
trap_pfault(0,7fffffff,dc52644c,c1b89ea4,d83f9a00,...) at trap_pfault+0x109/frame 0xdab02a58
Alan observes that a change to vm_map_lookup_entry like the one proposed here is redundant along many code paths, and that perhaps I should be proposing a change to vm_map_lookup, or vm_fault_hold, or vm_fault, instead, along with changes to other paths to vm_map_lookup_entry that aren't bounds-checked. For that reason, this proposed change might be sitting idle for a while.
Don't you need an assert in vm_map_entry_splay() then, that the address to splay is in the range ?
The map is not an argument to vm_map_entry_splay, so the range data is not available to form an assertion.
Nothing prevents you from adding the corresponding argument, or perhaps better, from creating an inline wrapper function which takes three args, checks the address against limits and then call the splay. In the non-debug builds, where KASSERT is compiled to an empty code, it should be optimized out completely.