On Wed 16-11-16 18:21:08, Peter Zijlstra wrote: > On Wed, Nov 16, 2016 at 12:01:01PM +0100, Jan Kara wrote: > > On Wed 16-11-16 11:51:32, Peter Zijlstra wrote: > > > > Now, I'm entirely out of touch wrt DAX, so I've not idea what that > > > needs/wants. > > > > Yeah, DAX does not have 'struct page' for its pages so it directly installs > > PFNs in the page tables. As a result it needs to know about page tables and > > stuff. > > Not convinced, a physical address should then be the equivalent of a > struct page. You still don't need access to the actual pages tables. The > VM core can then convert the physical address to a PFN and stuff it in > the PTE entry. > > > Now I've abstracted knowledge about that into helper functions back > > in mm/ but still we need to pass the information through the ->fault handler > > into those helpers and vm_fault structure is simply natural for that. > > So far we have tried to avoid that but the result was not pretty (special > > return codes from DAX ->fault handlers essentially leaking information > > about DAX internal locking into mm/ code to direct generic mm code to do > > the right thing for DAX). > > Its probably the DAX locking bit I'm missing, because I cannot see why > VM_FAULT_DAX_LOCKED is 'broken' -- also, I'd have called that > VM_FAULT_PFN or similar and not have used the full entry but only the > PFN bits from it. The problem really is in the locking. Currently fault code expects to get a locked page from the ->fault() handler (or it locks the returned page on its own). When we don't have struct page, we don't have a page lock. So we use different locks to synchronize against truncate, or other page faults of the same page by a different process. And modification of page tables needs to be protected by these locks in the same way as it needs to be protected by the page lock for normal pages. So I find it to be bigger layering violation for generic mm layer to know how DAX decided to serialize racing page faults, than for DAX layer to call helpers from mm layer to update page tables when it holds appropriate locks. Essentially we just switch from a model "Return data from a fault handler so that generic layer can finish the page fault" to a model "Call helper from the fault handler passing it necessary information to finish the page fault". That model is more flexible (I agree it offers more opportunities for abuse as well) but overall result looks IMHO cleaner. And I would add that modifying page tables from a fault handler is nothing new - basically every video driver does it. They just generally don't have that comprehensive support for mmap so they get away with less exported functionality. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
