On Tue 13-12-16 11:29:54, Dave Jiang wrote: > > > On 12/13/2016 05:15 AM, Jan Kara wrote: > > On Thu 08-12-16 14:34:21, Dave Jiang wrote: > >> Instead of passing in multiple parameters in the pmd_fault() handler, > >> a vmf can be passed in just like a fault() handler. This will simplify > >> code and remove the need for the actual pmd fault handlers to allocate a > >> vmf. Related functions are also modified to do the same. > >> > >> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx> > >> Reviewed-by: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx> > > > > I like the idea however see below: > > > >> @@ -1377,21 +1376,20 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address, > >> if (iomap.offset + iomap.length < pos + PMD_SIZE) > >> goto unlock_entry; > >> > >> - vmf.pgoff = pgoff; > >> - vmf.flags = flags; > >> - vmf.gfp_mask = mapping_gfp_mask(mapping) | __GFP_IO; > >> + vmf->pgoff = pgoff; > >> + vmf->gfp_mask = mapping_gfp_mask(mapping) | __GFP_IO; > > > > But now it's really unexpected that you change pgoff and gfp_mask because > > that will propagate back to the caller and if we return VM_FAULT_FALLBACK > > we may fault in wrong PTE because of this. So dax_iomap_pmd_fault() should > > not modify the passed gfp_mask, just make its callers clear __GFP_FS from > > it because *they* are responsible for acquiring locks / transactions that > > block __GFP_FS allocations. They are also responsible for restoring > > original gfp_mask once dax_iomap_pmd_fault() returns. > > Ok will fix. > > > > > dax_iomap_pmd_fault() needs to modify pgoff however it must restore it to > > the original value before it returns. > > Need clarification here. Do you mean "If" dax_iomap_pmd_fault() needs to > modify.... and right now it doesn't appear to need to modify pgoff so > nothing needs to be done? Thanks. How come? I can see: pgoff = linear_page_index(vma, pmd_addr); a few lines above - we need to modify pgoff to contain huge page aligned file index instead of only page aligned... Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>