Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 17 Jul 2018 13:39:06 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     dave.hansen@intel.com, linux-kernel@vger.kernel.org,
        mingo@kernel.org, songliubraving@fb.com, hpa@zytor.com,
        tglx@linutronix.de, torvalds@linux-foundation.org, riel@surriel.com
Cc:     linux-tip-commits@vger.kernel.org
Subject: Re: [tip:x86/mm] x86/mm/tlb: Only send page table free TLB flush to
 lazy TLB CPUs
Message-ID: <20180717113906.GV2476@hirez.programming.kicks-ass.net>
References: <20180716190337.26133-6-riel@surriel.com>
 <tip-64482aafe55fc7e84d0741c356f8176ee7bde357@git.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <tip-64482aafe55fc7e84d0741c356f8176ee7bde357@git.kernel.org>
User-Agent: Mutt/1.10.0 (2018-05-17)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jul 17, 2018 at 02:35:41AM -0700, tip-bot for Rik van Riel wrote:
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 26542cc17043..e4156e37aa71 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -712,15 +712,50 @@ void tlb_flush_remove_tables_local(void *arg)
>  	}
>  }
>  
> +static void mm_fill_lazy_tlb_cpu_mask(struct mm_struct *mm,
> +				      struct cpumask *lazy_cpus)
> +{
> +	int cpu;
> +
> +	for_each_cpu(cpu, mm_cpumask(mm)) {
> +		if (!per_cpu(cpu_tlbstate.is_lazy, cpu))
> +			cpumask_set_cpu(cpu, lazy_cpus);

That really wants to be __cpumask_set_cpu(). Using LOCK prefix
instructions to set local bits is kinda pointless and expensive.

> +	}
> +}
> +
>  void tlb_flush_remove_tables(struct mm_struct *mm)
>  {
>  	int cpu = get_cpu();
> +	cpumask_var_t lazy_cpus;
> +
> +	if (cpumask_any_but(mm_cpumask(mm), cpu) >= nr_cpu_ids) {
> +		put_cpu();
> +		return;
> +	}
> +
> +	if (!zalloc_cpumask_var(&lazy_cpus, GFP_ATOMIC)) {
> +		/*
> +		 * If the cpumask allocation fails, do a brute force flush
> +		 * on all the CPUs that have this mm loaded.
> +		 */
> +		smp_call_function_many(mm_cpumask(mm),
> +				tlb_flush_remove_tables_local, (void *)mm, 1);
> +		put_cpu();
> +		return;
> +	}
> +
>  	/*
> -	 * XXX: this really only needs to be called for CPUs in lazy TLB mode.
> +	 * CPUs with !is_lazy either received a TLB flush IPI while the user
> +	 * pages in this address range were unmapped, or have context switched
> +	 * and reloaded %CR3 since then.
> +	 *
> +	 * Shootdown IPIs at page table freeing time only need to be sent to
> +	 * CPUs that may have out of date TLB contents.
>  	 */
> -	if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids)
> -		smp_call_function_many(mm_cpumask(mm), tlb_flush_remove_tables_local, (void *)mm, 1);
> -
> +	mm_fill_lazy_tlb_cpu_mask(mm, lazy_cpus);
> +	smp_call_function_many(lazy_cpus,
> +				tlb_flush_remove_tables_local, (void *)mm, 1);
> +	free_cpumask_var(lazy_cpus);
>  	put_cpu();
>  }

Also, was there a reason to re-implement on_each_cpu_cond() ? (which btw
also wants that __cpumask_set_bit fix).