On Wed, Jul 4, 2012 at 12:48 AM, Jakub Jermar <[email protected]> wrote:
> On 06/29/2012 05:20 AM, Adam Hraska wrote:
>> Bugs
>> ----
>> I am afraid traversing the page tables without proper
>> locking exposes us to several nasty time-dependent bugs
>> if page_mapping_find() is not coded with extreme care.
>> A glance at the hash-based as well as tree-base page
>> table reveals that we would at a minimum have to insert
>> a number of memory barriers into the code (check out
>> ht_mapping_insert() and pt_mapping_insert()). Otherwise,
>> a TLB miss handler might eg see a new page table level/
>> node before it is fully initialized (the compiler or cpu
>> may reorder writes and loads). Or have I forgotten to
>> consider something?
>
> I added some write_barrier()'s to pt_mapping_insert() in
> mainline,825.1.173, to ht_mapping_insert() in mainline,825.1.174, and
> some read_barrier()'s to pt_mapping_find() in mainline,825.1.175.
Looks good to me. Thank you.
We could add one more barrier to pt_mapping_make_global()
in order to be extra safe:
void pt_mapping_make_global(uintptr_t base, size_t size)
{
// snip
for (addr = ALIGN_DOWN(base, ptl0_step); addr - 1 < base + size - 1;
addr += ptl0_step) {
uintptr_t l1;
l1 = (uintptr_t) frame_alloc(order, FRAME_KA | FRAME_LOWMEM);
memsetb((void *) l1, FRAME_SIZE << order, 0);
SET_PTL1_ADDRESS(ptl0, PTL0_INDEX(addr), KA2PA(l1));
/* Make memsetb() visible before setting the PAGE_PRESENT flag.
*/
write_barrier(); // <---- THIS IS NEW
SET_PTL1_FLAGS(ptl0, PTL0_INDEX(addr),
PAGE_PRESENT | PAGE_USER | PAGE_CACHEABLE |
PAGE_EXEC | PAGE_WRITE | PAGE_READ);
}
}
We do not have to worry about pt_mapping_remove()
because it is always protected by the TLB shootdown
procedure. As a result whenever we remove page table
mappings we can be sure that all cpus are waiting
for the shootdown to complete and they will not be
searching the page table (or causing new page faults).
Sorry it took me so long to review the change.
Adam
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
