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
