Re: [PATCH v8 19/20] fs/dax: Properly refcount fs dax pages
From: David Hildenbrand <hidden>
Date: 2025-02-20 11:51:48
Also in:
linux-arm-kernel, linux-cxl, linux-doc, linux-ext4, linux-fsdevel, linux-mm, linux-xfs, lkml, loongarch, nvdimm
quoted
quoted
-static inline unsigned long dax_folio_share_put(struct folio *folio) +static inline unsigned long dax_folio_put(struct folio *folio) { - return --folio->page.share; + unsigned long ref; + int order, i; + + if (!dax_folio_is_shared(folio)) + ref = 0; + else + ref = --folio->share; +out of interest, what synchronizes access to folio->share?Actually that's an excellent question as I hadn't looked too closely at this given I wasn't changing the overall flow with regards to synchronization, merely representation of the "shared" state. So I don't have a good answer for you off the top of my head - Dan maybe you can shed some light here?
Not that I understand what that dax-shared thing is or does, but the non-atomic update on a folio_put path looked "surprising".
quoted
quoted
diff --git a/include/linux/dax.h b/include/linux/dax.h index 2333c30..dcc9fcd 100644 --- a/include/linux/dax.h +++ b/include/linux/dax.h@@ -209,7 +209,7 @@ int dax_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,[...]quoted
diff --git a/mm/huge_memory.c b/mm/huge_memory.c index d189826..1a0d6a8 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c@@ -2225,7 +2225,7 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, tlb->fullmm); arch_check_zapped_pmd(vma, orig_pmd); tlb_remove_pmd_tlb_entry(tlb, pmd, addr); - if (vma_is_special_huge(vma)) { + if (!vma_is_dax(vma) && vma_is_special_huge(vma)) {I wonder if we actually want to remove the vma_is_dax() check from vma_is_special_huge(), and instead add it to the remaining callers of vma_is_special_huge() that still need it -- if any need it. Did we sanity-check which callers of vma_is_special_huge() still need it? Is there still reason to have that DAX check in vma_is_special_huge()?If by "we" you mean "me" then yes :) There are still a few callers of it, mainly for page splitting.
Heh, "you or any of the reviewers" :) So IIUC, the existing users still need the DAX check I assume (until that part is cleaned up, below).
quoted
But vma_is_special_huge() is rather confusing from me ... the whole vma_is_special_huge() thing should probably be removed. That's a cleanup for another day, though.But after double checking I have come to the same conclusion as you - it should be removed. I will add that to my ever growing clean-up series that can go on top of this one.
Nice! -- Cheers, David / dhildenb