Re: [PATCH 7.2 v9 2/2] x86/tlb: skip redundant sync IPIs for native TLB flush

[Lance Yang <lance.yang@linux.dev>]

Hi Dave,

On Thu, Apr 23, 2026 at 10:56:03AM -0700, Dave Hansen wrote:
[...]
>On 4/19/26 20:08, Lance Yang wrote:
>> -	flush_tlb_mm_range(tlb->mm, start, end, stride_shift, tlb->freed_tables);
>> +	/*
>> +	 * Treat unshared_tables just like freed_tables, such that lazy-TLB
>> +	 * CPUs also receive IPIs during unsharing of page tables, allowing
>> +	 * us to safely implement tlb_table_flush_implies_ipi_broadcast().
>> +	 */
>> +	flush_tlb_mm_range(tlb->mm, start, end, stride_shift,
>> +			   tlb->freed_tables || tlb->unshared_tables);
>>  }
>
>I've been staring at this trying to make sense of it for too long.
>
>Right now, flush_tlb_mm_range() literally has an argument named
>"freed_tables" and "tlb->freed_tables" is passed there. That seems
>totally sane. It's 100% straightforward to follow.
>
>But it makes zero logical sense to me to now mix "tlb->unshared_tables"
>in there. Sure, what you _want_ is the freed_tables==1 behavior from
>tlb->unshared_tables==1, and this obviously hacks that in there, but
>it's not explained well enough and not maintainable like this. IOW, it's
>still just hack.
>
>I think what's happened here is that info->freed_tables is being
>modified from being strictly related to page table freeing, and moved
>over to a bit which tells TLB flushing implementations whether they can
>respect CPUs in lazy TLB mode.
>
>It's mentioned in the comment, but then ever reflected into the code.
>
>Shouldn't we be doing something like the attached patch? Look at how
>that maps over to the flushing side, like in the hyperv code:

Cool, thanks!

I was trying to keep the change small by passing unshared_tables through
the exsiting freed_tables argument, but that made the code a bit harder
to follow ...

>
>> -       bool do_lazy = !info->freed_tables;
>> +       bool do_lazy = !info->wake_lazy_cpus;
>>  
>>         trace_hyperv_mmu_flush_tlb_multi(cpus, info);
>>  
>> @@ -198,7 +198,7 @@ static u64 hyperv_flush_tlb_others_ex(co
>>  
>>         flush->hv_vp_set.format = HV_GENERIC_SET_SPARSE_4K;
>>         nr_bank = cpumask_to_vpset_skip(&flush->hv_vp_set, cpus,
>> -                       info->freed_tables ? NULL : cpu_is_lazy);
>> +                       info->wake_lazy_cpus ? NULL : cpu_is_lazy);
>
>That even makes the hyperv code easier to read over what was there
>before, IMNHO.
>
>Thoughts?
[...]

Yeah, renaming the flush_tlb_info bit to wake_lazy_cpus reads much
better. Will fold this into v10, Thanks for spelling it out!

Lance