Re: [PATCH] support tmpfs hugepage PMD is not split when COW

[David Hildenbrand <david@redhat.com>]

On 10.01.24 10:20, Chen Haixiang wrote:
> Transparent hugepages in tmpfs can enhance TLB efficiency by reducing
> TLB misses. However, during Copy-On-Write (COW) memory faults, these
> hugepages may be split. In some scenarios, preventing this splitting
> is desirable. We might introduce a shmem_huge_fault to inhibit this
> behavior, along with a mount parameter to enable or disable this function.
> 

I'm confused, can you describe the problem a bit better, because ...

> Signed-off-by: Chen Haixiang <chenhaixiang3@huawei.com>
> ---


[...]

>   
> +static vm_fault_t shmem_huge_fault(struct vm_fault *vmf, pmd_t orig_pmd)
> +{
> +	vm_fault_t ret = VM_FAULT_FALLBACK;
> +	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
> +	struct folio *old_folio, *new_folio;
> +	pmd_t entry;
> +	int gfp_flags = GFP_HIGHUSER_MOVABLE | __GFP_COMP;
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct shmem_sb_info *sbinfo = NULL;
> +	struct inode *inode = file_inode(vma->vm_file);
> +	struct shmem_inode_info *info = SHMEM_I(inode);
> +
> +	sbinfo = SHMEM_SB(info->vfs_inode.i_sb);
> +
> +	if (sbinfo->no_split == 0)
> +		return VM_FAULT_FALLBACK;
> +
> +	/* ShmemPmdMapped in tmpfs will not split huge pmd */
> +	if (!(vmf->flags & FAULT_FLAG_WRITE)
> +			|| (vma->vm_flags & VM_SHARED))
> +		return VM_FAULT_FALLBACK;

We do have a private (COW) mapping at this point, and

> +
> +	new_folio = vma_alloc_folio(gfp_flags, HPAGE_PMD_ORDER,
> +			vmf->vma, haddr, true);
> +	if (!new_folio)
> +		ret = VM_FAULT_FALLBACK;
> +
> +	vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd);
> +	if (pmd_none(*vmf->pmd)) {
> +		ret = VM_FAULT_FALLBACK;
> +		goto out;
> +	}
> +	if (!pmd_same(*vmf->pmd, orig_pmd)) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	if (!new_folio) {
> +		count_vm_event(THP_FAULT_FALLBACK);
> +		ret = VM_FAULT_FALLBACK;
> +		goto out;
> +	}
> +	old_folio = page_folio(pmd_page(*vmf->pmd));
> +	page_remove_rmap(&old_folio->page, vma, true);
> +	pmdp_huge_clear_flush(vma, haddr, vmf->pmd);
> +
> +	__folio_set_locked(new_folio);
> +	__folio_set_swapbacked(new_folio);
> +	__folio_mark_uptodate(new_folio);

We allocate a fresh folio here and

> +
> +	flush_icache_pages(vma, &new_folio->page, HPAGE_PMD_NR);
> +	entry = mk_huge_pmd(&new_folio->page, vma->vm_page_prot);
> +	entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> +
> +	page_add_file_rmap(&new_folio->page, vma, true);

it is not an anonymous one?

... but your are making it writable? In a private mapping?

> +	set_pmd_at(vma->vm_mm, haddr, vmf->pmd, entry);
> +	update_mmu_cache_pmd(vma, haddr, vmf->pmd);
> +	count_vm_event(THP_FILE_MAPPED);

And still acoount it as a file folio? But never add it so some pagecache 
structure?


I'm probably missing something, or something is completely wrong here.

-- 
Cheers,

David / dhildenb