Re: [PATCH v5 3/8] x86/mm/tlb: Open-code on_each_cpu_cond_mask() for tlb_is_not_lazy()

[Nadav Amit]

> On Feb 18, 2021, at 12:16 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> 
> On Tue, Feb 09, 2021 at 02:16:48PM -0800, Nadav Amit wrote:
>> +		/*
>> +		 * Although we could have used on_each_cpu_cond_mask(),
>> +		 * open-coding it has performance advantages, as it eliminates
>> +		 * the need for indirect calls or retpolines. In addition, it
>> +		 * allows to use a designated cpumask for evaluating the
>> +		 * condition, instead of allocating one.
>> +		 *
>> +		 * This code works under the assumption that there are no nested
>> +		 * TLB flushes, an assumption that is already made in
>> +		 * flush_tlb_mm_range().
>> +		 *
>> +		 * cond_cpumask is logically a stack-local variable, but it is
>> +		 * more efficient to have it off the stack and not to allocate
>> +		 * it on demand. Preemption is disabled and this code is
>> +		 * non-reentrant.
>> +		 */
>> +		struct cpumask *cond_cpumask = this_cpu_ptr(&flush_tlb_mask);
>> +		int cpu;
>> +
>> +		cpumask_clear(cond_cpumask);
>> +
>> +		for_each_cpu(cpu, cpumask) {
>> +			if (tlb_is_not_lazy(cpu))
>> +				__cpumask_set_cpu(cpu, cond_cpumask);
>> +		}
>> +		smp_call_function_many(cond_cpumask, flush_tlb_func, (void *)info, 1);
> 
> No need for the cast here, which would also avoid the pointlessly
> overly long line.

Actually, there is - to remove the const qualifier. You might argue it is
ugly, but that’s the way it is also how it is done right now.

In general, thanks for the feedback (I will reply after I follow your
feedback). I do have a general question - I thought it was decided that
clarity should be preferred over following the 80-column limit. Please let
me know if I misunderstood.