On Mon, Feb 10, 2025 at 07:45:09PM +0100, David Hildenbrand wrote:
> On 04.02.25 23:48, Alistair Popple wrote:
> > Currently DAX folio/page reference counts are managed differently to normal
> > pages. To allow these to be managed the same as normal pages introduce
> > vmf_insert_folio_pmd. This will map the entire PMD-sized folio and take
> > references as it would for a normally mapped page.
> >
> > This is distinct from the current mechanism, vmf_insert_pfn_pmd, which
> > simply inserts a special devmap PMD entry into the page table without
> > holding a reference to the page for the mapping.
> >
> > It is not currently useful to implement a more generic vmf_insert_folio()
> > which selects the correct behaviour based on folio_order(). This is because
> > PTE faults require only a subpage of the folio to be PTE mapped rather than
> > the entire folio. It would be possible to add this context somewhere but
> > callers already need to handle PTE faults and PMD faults separately so a
> > more generic function is not useful.
> >
> > Signed-off-by: Alistair Popple <apopple@xxxxxxxxxx>
>
> Nit: patch subject ;)
>
> >
> > ---
> >
> > Changes for v7:
> >
> > - Fix bad pgtable handling for PPC64 (Thanks Dan and Dave)
>
> Is it? ;) insert_pfn_pmd() still doesn't consume a "pgtable_t *"
>
> But maybe I am missing something ...
At a high-level all I'm trying to do (perhaps badly) is pull the ptl locking one
level up the callstack.
As far as I can tell the pgtable is consumed here:
static int insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
pmd_t *pmd, pfn_t pfn, pgprot_t prot, bool write,
pgtable_t pgtable)
[...]
if (pgtable) {
pgtable_trans_huge_deposit(mm, pmd, pgtable);
mm_inc_nr_ptes(mm);
pgtable = NULL;
}
[...]
return 0;
Now I can see I failed to clean up the useless pgtable = NULL asignment, which
is confusing because I'm not trying to look at pgtable in the caller (ie.
vmf_insert_pfn_pmd()/vmf_insert_folio_pmd()) to determine if it needs freeing.
So I will remove this assignment.
Instead callers just look at the return code from insert_pfn_pmd() - if there
was an error pgtable_trans_huge_deposit(pgtable) wasn't called and if the caller
passed a pgtable it should be freed. Otherwise if insert_pfn_pmd() succeeded
then callers can assume the pgtable was consumed by pgtable_trans_huge_deposit()
and therefore should not be freed.
Hopefully that all makes sense, but maybe I've missed something obvious too...
- Alistair
> --
> Cheers,
>
> David / dhildenb
>
