| Message ID | e4a877d1f77d778a2e820b9df66f6b7422bf2276.1712796818.git-series.apopple@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | fs/dax: Fix FS DAX page reference counts | expand |
On Thu, Apr 11, 2024 at 10:57:23AM +1000, Alistair Popple wrote: > pud_huge() returns true only for a HugeTLB page. pud_devmap() is only > used by FS DAX pages. These two things are mutually exclusive so this > code is dead code and can be removed. I'm not sure this is true.. pud_huge() is mostly a misspelling of pud_leaf().. > - if (pud_huge(pud) && pud_devmap(pud)) { I suspect this should be written as: if (pud_leaf(pud) && pud_devmap(pud)) { In line with Peter's work here: https://lore.kernel.org/linux-mm/20240321220802.679544-1-peterx@redhat.com/ Jason
On Thu, Apr 11, 2024 at 09:25:30AM -0300, Jason Gunthorpe wrote: > On Thu, Apr 11, 2024 at 10:57:23AM +1000, Alistair Popple wrote: > > pud_huge() returns true only for a HugeTLB page. pud_devmap() is only > > used by FS DAX pages. These two things are mutually exclusive so this > > code is dead code and can be removed. > > I'm not sure this is true.. pud_huge() is mostly a misspelling of pud_leaf().. > > > - if (pud_huge(pud) && pud_devmap(pud)) { > > I suspect this should be written as: > > if (pud_leaf(pud) && pud_devmap(pud)) { > > In line with Peter's work here: > > https://lore.kernel.org/linux-mm/20240321220802.679544-1-peterx@redhat.com/ Just to provide more information for Alistair, this patch already switched that over to a _leaf(): https://lore.kernel.org/r/20240318200404.448346-12-peterx@redhat.com That's in mm-unstable now, so should see that in a rebase. And btw it's great to see that pxx_devmap() can go away. Thanks,
Peter Xu <peterx@redhat.com> writes: > On Thu, Apr 11, 2024 at 09:25:30AM -0300, Jason Gunthorpe wrote: >> On Thu, Apr 11, 2024 at 10:57:23AM +1000, Alistair Popple wrote: >> > pud_huge() returns true only for a HugeTLB page. pud_devmap() is only >> > used by FS DAX pages. These two things are mutually exclusive so this >> > code is dead code and can be removed. >> >> I'm not sure this is true.. pud_huge() is mostly a misspelling of pud_leaf().. >> >> > - if (pud_huge(pud) && pud_devmap(pud)) { >> >> I suspect this should be written as: >> >> if (pud_leaf(pud) && pud_devmap(pud)) { Oh that makes a lot more sense. I'd taken the comment for pud_huge() at face value (ie. that it's a hugetlbfs page) without digging further. >> In line with Peter's work here: >> >> https://lore.kernel.org/linux-mm/20240321220802.679544-1-peterx@redhat.com/ > > Just to provide more information for Alistair, this patch already switched > that over to a _leaf(): Got it, thanks (and apologies for missing my Cc on that). > https://lore.kernel.org/r/20240318200404.448346-12-peterx@redhat.com > > That's in mm-unstable now, so should see that in a rebase. > > And btw it's great to see that pxx_devmap() can go away. Yep, AFAICT pxx_devmap only exists to do this special FS DAX refcounting. Once that is fixed it can go away. > Thanks,
diff --git a/mm/hmm.c b/mm/hmm.c index 277ddca..5bbfb0e 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -411,9 +411,6 @@ static inline unsigned long pud_to_hmm_pfn_flags(struct hmm_range *range, static int hmm_vma_walk_pud(pud_t *pudp, unsigned long start, unsigned long end, struct mm_walk *walk) { - struct hmm_vma_walk *hmm_vma_walk = walk->private; - struct hmm_range *range = hmm_vma_walk->range; - unsigned long addr = start; pud_t pud; spinlock_t *ptl = pud_trans_huge_lock(pudp, walk->vma); @@ -429,39 +426,9 @@ static int hmm_vma_walk_pud(pud_t *pudp, unsigned long start, unsigned long end, return hmm_vma_walk_hole(start, end, -1, walk); } - if (pud_huge(pud) && pud_devmap(pud)) { - unsigned long i, npages, pfn; - unsigned int required_fault; - unsigned long *hmm_pfns; - unsigned long cpu_flags; - - if (!pud_present(pud)) { - spin_unlock(ptl); - return hmm_vma_walk_hole(start, end, -1, walk); - } - - i = (addr - range->start) >> PAGE_SHIFT; - npages = (end - addr) >> PAGE_SHIFT; - hmm_pfns = &range->hmm_pfns[i]; - - cpu_flags = pud_to_hmm_pfn_flags(range, pud); - required_fault = hmm_range_need_fault(hmm_vma_walk, hmm_pfns, - npages, cpu_flags); - if (required_fault) { - spin_unlock(ptl); - return hmm_vma_fault(addr, end, required_fault, walk); - } - - pfn = pud_pfn(pud) + ((addr & ~PUD_MASK) >> PAGE_SHIFT); - for (i = 0; i < npages; ++i, ++pfn) - hmm_pfns[i] = pfn | cpu_flags; - goto out_unlock; - } - /* Ask for the PUD to be split */ walk->action = ACTION_SUBTREE; -out_unlock: spin_unlock(ptl); return 0; }
pud_huge() returns true only for a HugeTLB page. pud_devmap() is only used by FS DAX pages. These two things are mutually exclusive so this code is dead code and can be removed. Signed-off-by: Alistair Popple <apopple@nvidia.com> --- mm/hmm.c | 33 --------------------------------- 1 file changed, 33 deletions(-)