Re: [PATCH 11/13] huge_memory: Remove dead vmf_insert_pXd code

[Alistair Popple]

Peter Xu <peterx@xxxxxxxxxx> writes:

> Hi, Alistair,
>
> On Thu, Jun 27, 2024 at 10:54:26AM +1000, Alistair Popple wrote:
>> Now that DAX is managing page reference counts the same as normal
>> pages there are no callers for vmf_insert_pXd functions so remove
>> them.
>> 
>> Signed-off-by: Alistair Popple <apopple@xxxxxxxxxx>
>> ---
>>  include/linux/huge_mm.h |   2 +-
>>  mm/huge_memory.c        | 165 +-----------------------------------------
>>  2 files changed, 167 deletions(-)
>> 
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 9207d8e..0fb6bff 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -37,8 +37,6 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>  		    pmd_t *pmd, unsigned long addr, pgprot_t newprot,
>>  		    unsigned long cp_flags);
>>  
>> -vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, pfn_t pfn, bool write);
>> -vm_fault_t vmf_insert_pfn_pud(struct vm_fault *vmf, pfn_t pfn, bool write);
>>  vm_fault_t dax_insert_pfn_pmd(struct vm_fault *vmf, pfn_t pfn, bool write);
>>  vm_fault_t dax_insert_pfn_pud(struct vm_fault *vmf, pfn_t pfn, bool write);
>
> There's a plan to support huge pfnmaps in VFIO, which may still make good
> use of these functions.  I think it's fine to remove them but it may mean
> we'll need to add them back when supporting pfnmaps with no memmap.

I'm ok with that. If we need them back in future it shouldn't be too
hard to add them back again. I just couldn't find any callers of them
once DAX stopped using them and the usual policy is to remove unused
functions.

> Is it still possible to make the old API generic to both service the new
> dax refcount plan, but at the meantime working for pfn injections when
> there's no page struct?

I don't think so - this new dax refcount plan relies on having a struct
page to take references on so I don't think it makes much sense to
combine it with something that doesn't have a struct page. It sounds
like the situation is the analogue of vm_insert_page()
vs. vmf_insert_pfn() - it's possible for both to exist but there's not
really anything that can be shared between the two APIs as one has a
page and the other is just a raw PFN.

> Thanks,