On 12/14/2016 02:57 AM, Jan Kara wrote: > 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 > Yep. My mistake. I misunderstood. Will fix. -- 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>