Re: [PATCH 2/4] mm: rmap: Allow platforms without mm_cpumask to defer TLB flush

[Nadav Amit]

On Jul 7, 2022, at 5:52 AM, Barry Song <21cnbao@xxxxxxxxx> wrote:

> From: Barry Song <v-songbaohua@xxxxxxxx>
> 
> Platforms like ARM64 have hareware TLB shootdown broadcast. They
> don't maintain mm_cpumask and they just send tlbi and related
> sync instructions for TLB flush.
> So if mm_cpumask is empty, we also allow deferred TLB flush
> 
> Cc: Nadav Amit <namit@xxxxxxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxx>
> Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>>
> ---
> mm/rmap.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 5bcb334cd6f2..d320c29a4ad8 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -692,8 +692,13 @@ static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
> 	if (!(flags & TTU_BATCH_FLUSH))
> 		return false;
> 
> -	/* If remote CPUs need to be flushed then defer batch the flush */
> -	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
> +	/*
> +	 * If remote CPUs need to be flushed then defer batch the flush;
> +	 * If ARCHs like ARM64 have hardware TLB flush broadcast, thus
> +	 * they don't maintain mm_cpumask() at all, defer batch as well.
> +	 */
> +	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids ||
> +	    cpumask_empty(mm_cpumask(mm)))

The cpumask_empty() is indeed just another memory access, which is most
likely ok. But wouldn’t adding something like CONFIG_ARCH_HAS_MM_CPUMASK
make the code simpler and (slightly, certainly slightly) more performant?