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

David Hildenbrand (Arm)
2026-04-23T19:45:03Z

On 4/23/26 19:56, 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:
>
>> - 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?

Looks better!

--
Cheers,

David