Re: [PATCH v2 02/12] mm: hugetlb_vmemmap: unify VMEMMAP_{SPLIT,REMAP}_NO_TLB_FLUSH

[Mike Rapoport]

Hi Muchun,

> VMEMMAP_SPLIT_NO_TLB_FLUSH and VMEMMAP_REMAP_NO_TLB_FLUSH do not
> describe fully independent choices. A split-only walk can defer the
> PMD-split flush to a later global flush, and a remap-only walk can
> defer the final remap flush. However, when a PMD split is followed
> by PTE remapping, the split flush must happen before the PTEs are
> changed, because stale large-page translations must not overlap with
> new small-page translations that map different physical pages.
> 
> Replace the two flags with a single VMEMMAP_DEFER_TLB_FLUSH flag. The
> new name reflects that only safely batchable flushes are deferred. Keep
> split-only walks able to defer the flush, but force the split flush
> whenever a remap_pte callback is present. Callers that batch HVO work
> can still defer the final remap flush and issue their own global flush.
> 
> Current callers keep the same flushing behavior.
> 
> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> Reviewed-by: Lance Yang <lance.yang@xxxxxxxxx>
> Reviewed-by: Qi Zheng <qi.zheng@xxxxxxxxx>
> ---
> v2:
> - Rename the unified flag to VMEMMAP_DEFER_TLB_FLUSH and update its
>   comment (suggested by Qi Zheng)
> - Update the commit message for the new flag name
> - Collect Reviewed-by from Lance Yang and Qi Zheng
> ---
>  mm/hugetlb_vmemmap.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 4b73d6ec8dcea..63e899cfaac3a 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -42,10 +42,8 @@ struct vmemmap_remap_walk {
>  	struct list_head	*vmemmap_pages;
>  
>  
> -/* Skip the TLB flush when we split the PMD */
> -#define VMEMMAP_SPLIT_NO_TLB_FLUSH	BIT(0)
> -/* Skip the TLB flush when we remap the PTE */
> -#define VMEMMAP_REMAP_NO_TLB_FLUSH	BIT(1)
> +/* Defer safely batchable TLB flushes to the caller. */
> +#define VMEMMAP_DEFER_TLB_FLUSH	BIT(0)
>  	unsigned long		flags;
>  };
>  
> @@ -85,7 +83,12 @@ static int vmemmap_split_pmd(pmd_t *pmd, struct page *head, unsigned long start,
>  		/* Make pte visible before pmd. See comment in pmd_install(). */
>  		smp_wmb();
>  		pmd_populate_kernel(&init_mm, pmd, pgtable);
> -		if (!(walk->flags & VMEMMAP_SPLIT_NO_TLB_FLUSH))
> +		/*
> +		 * Modern MMUs allow overlapping TLB entries with different mapping
> +		 * granularities only if they map the same physical address. So
> +		 * remapping must not proceed without first flushing the TLB.

Are we certain that *all* modern MMUs for *all* architecures that 
support hugetlb are fine with skipping TLB flush after a split?

AFAIU, x86::set_memory always flushes TLBs after a large leaf is split.

-- 
Sincerely yours,
Mike.