On Wed, Feb 21, 2024 at 09:15:51AM -0800, Vishal Moola wrote: > > > unsigned long haddr = address & huge_page_mask(h); > > > struct mmu_notifier_range range; > > > + struct vm_fault vmf = { > > > + .vma = vma, > > > + .address = haddr, > > > + .real_address = address, > > > + .flags = flags, > > > + }; > > > > We don't usually indent quite so far. One extra tab would be enough. > > > > Also, I thought we talked about creating the vmf in hugetlb_fault(), > > then passing it to hugetlb_wp() hugetlb_no_page() and handle_userfault()? > > Was there a reason to abandon that idea? > > No I haven't abandoned that idea, I intend to have a separate patchset to go > on top of this one - just keeping them separate since they are conceptually > different. I'm converting each function to use struct vm_fault first, then > shifting it to be passed throughout as an arguement while cleaning up the > excess variables laying around. In a sense working bottom-up instead > of top-down. I think you'll find it less work to create it in hugetlb_fault() first. ie patch 2 could be to hoist its creation from half-way down hugetlb_fault to the top of hugetlb_fault. Patch 3 could pass it through hugetlb_no_page() to hugetlb_handle_userfault() and remove its creation there. Now you've alreedy got it, and can make use of it in this patch which would be the new patch 4. If you want to do a cleanup patch afterwards, you could hoist the vmf creation all the way to handle_mm_fault() ;-)