On 2023/1/13 22:25, Matthew Wilcox wrote:
On Thu, Jan 12, 2023 at 04:30:00PM +0800, Kefeng Wang wrote:
Let's __do_huge_pmd_anonymous_page() take a folio and convert related
functions to use folios.
No, this is actively wrong! Andrew, please drop this patch.
If we want to support folio sizes larger than PMD size (and I think we
do), we need to be able to specify precisely which page in the folio
is to be stored at this PTE. The *interface* must remain struct page.
We can convert from page to folio within the function, but we *MUST NOT*
go the other way.
Got it,
static vm_fault_t __do_huge_pmd_anonymous_page(struct vm_fault *vmf,
- struct page *page, gfp_t gfp)
+ struct folio *folio, gfp_t gfp)
{
struct vm_area_struct *vma = vmf->vma;
+ struct page *page = &folio->page;
... ie this is bad and wrong.
@@ -834,7 +835,7 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
count_vm_event(THP_FAULT_FALLBACK);
return VM_FAULT_FALLBACK;
}
- return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp);
+ return __do_huge_pmd_anonymous_page(vmf, folio, gfp);
}
static void insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
A reasonable person might ask "But Matthew, you allocated a folio here,
then you're converting back to a struct page to call
__do_huge_pmd_anonymous_page() so isn't this a sensible patch?"
and this is why I change the parameter from page to folio(no need to go
back and forth between page and folio),
And I would say "still no". This is a question of interfaces, and
even though __do_huge_pmd_anonymous_page() is static and has precisely
one caller today that always allocates a folio of precisely PMD size,
I suspect it will need to be more visible in the future and the
conversion of the interface from page to folio misleads people.
ok, will keep page for __do_huge_pmd_anonymous_page().
