Benjamin Herrenschmidt wrote: >>No, that's probably the safest approach we can use until NOPAGE_RETRY >>arrives. >>Only I was not sure it'd be safe to call io_remap_pfn_range() from >>within nopage, in case it modifies some internal mm structs that the >>kernel nopage() code >>expects to be untouched. >> >> > >It does a couple of things that I don't like and lacks the re-test of >the PTE as I explained in my other email. I really want to provide a >function for doing that tho, one page at a time. > > > >>Once NOPAGE_RETRY arrives, (hopefully with a schedule() call attached to >>it), >> >> > >What about schedule ? NOPAGE_RETRY returns all the way back to >userland... So things like signals can be handled etc... and the >faulting instruction is re-executed. If there is a need for >rescheduling, that will be handled by the kernel on the return path to >userland as with signals. > >I want that for other things on cell which might also be useful for you, >for example, the no_page() handler for cell might wait a long time to be >able to schedule in a free HW SPE to service the fault. With >NOPAGE_RETRY, I can make that wait interruptible (test for signals) and >return to userland _without_ filling the PTE if a signal is pending, >thus causing signals to be serviced and the faulting instruction >re-executed. > > OK. I was thinking about the case where NOPAGE_RETRY was used to release the mmap semaphore for the process doing io_remap_pfn_range() Having the process yield CPU would make it more probable that it wasn't regrabbed by the process doing nopage.
> > >>it's possible, however, that repopulating the whole vma using >>io_remap_pfn_range() outside nopage, just after doing the copying is >>more efficient. >> >> > >Might well be when switching to vram but that means knowing about all >VMAs and thus all clients... possible but I was trying to avoid it. > > Although this means keeping track of vmas, the mmap_sems >can be taken and released one at a time, without any locking problems. > > > >Yup. > > > >>I agree the single-page approach looks nicer, though. It's somewhat ugly >>to force one's way into another process' memory space. >> >> > >It is but it works :) It's typically done by things like ptrace, or some >drivers DMA'ing to user memory, that's for example what get_user_pages() >allows you to do. So it should work as long as you are only faulting >pages, or invalidating mappings. We (ab)use that on cell too as SPEs run >in the host process address space. They have an MMU that we point to the >page tables of the process owning the context running on them. That >means we might have take interrupts on the host to service faults for an >SPE which is running another mm :) > >It might be more efficient performance wise however to do the full remap >of the entire vram on the first no_page() to it when the object is in >vram. That can be done safely with a simple change to >io_remap_pfn_range() to make it safe against racing with itself, >basically by having remap_pte_range() return 0 instead of BUG()'ing if >the PTE has been populated after the lock. Should be pretty trivial and >we shouldn't break anything since that was a BUG() case. That would work >around what I'm explaining in another email that it currently needs the >write mmap_sem because it doesn't handle races with another faulting >path (that is with itself basically). > >Ben. > > > > 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. * So basically when driver nopage is called we should _never_ have a valid PTE. 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). 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. 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. /Thomas ------------------------------------------------------------------------- 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
