> Hmm, the comments to handle_pte_fault(), which is calling do_nopage > gives some insight.. > > * Note the "page_table_lock". It is to protect against kswapd removing > * pages from under us. Note that kswapd only ever _removes_ pages, never > * adds them. As such, once we have noticed that the page is not present, > * we can drop the lock early. > * > * The adding of pages is protected by the MM semaphore (which we hold), > * so we don't need to worry about a page being suddenly been added into > * our VM. > *
This comment is a bit stale I think :) For example, the PTL is no longer used for faulting in PTEs, in favor of a more fine grained lock. Also, fauling only takes the mmap_sem for reading, which can be taken multiple times. It's only taken for writing (which excludes other writers and all readers) when modifying the VMA list itself. > So basically when driver nopage is called we should _never_ have a valid > PTE. No, we can have two no_page racing. > This makes the extra check in do_nopage() after the call to driver > nopage() somewhat mysterious, (but fortunate). Perhaps the intention is > for driver nopage() to be able to temporarily release the MM semaphore. > (Which would be even more fortunate). It's a rwsem, it can be taken multiple times for reading. Only once the PTE lock has been taken (ex page table lock, now a "PTE lock" whose actual granularity is arch dependent) then you know for sure that nobody else will be mucking around with -this- specific PTE. Which is why you need to re-check after taking the lock. The mmap_sem only protects against the whole VMA being teared down or modified (though truncate doesn't take it neither, thus the truncate count trick which ends up in a retry if we raced with it). > In any case, if the comments hold, we should never hit the BUG() > statement in io_remap_pfn_range(), but it also seems clear that the code > doesn't really expect ptes to be added. Unfortunately, the comment is misleading. I suppose I should submit a patch changing or removing it one of these days... > Taking this to the lkml for some clarification might be a good idea. > > On a totally different subject, the previous discussion we had about > having pages outside of the kernel virtual map (highmem pages) for > example, might be somewhat tricky with the current definition of > alloc_gatt_pages and free_gatt_pages, which both returns kernel virtual > addresses. Would be nice to have them return struct page* instead. Yes. Currently, we can get to struct page with virt_to_page, which is what we do in drm_vm.h for platforms where the AGP aperture cannot be accessed as MMIO and thus requires a no_page() for faulting the individual pages in (which is what we do on ppc btw). But that will not work with pages that aren't coming from the virtual mapping. Thus it might indeed be a good idea to change the AGP allocation to return struct page. Ben. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
