Re: [PATCH for-7.2 14/21] accel/tcg: Hoist get_page_addr_code out of tb_lookup

[Richard Henderson]

On 8/17/22 08:27, Ilya Leoshkevich wrote:
> On Wed, 2022-08-17 at 08:15 -0500, Richard Henderson wrote:
> > On 8/17/22 06:08, Ilya Leoshkevich wrote:
> > > @@ -2243,6 +2250,13 @@ void page_set_flags(target_ulong start,
> > > target_ulong end, int flags)
> > >                (flags & PAGE_WRITE) &&
> > >                p->first_tb) {
> > >                tb_invalidate_phys_page(addr, 0);
> > > +        } else {
> > > +            TranslationBlock *tb;
> > > +            int n;
> > > +
> > > +            PAGE_FOR_EACH_TB(p, tb, n) {
> > > +                cpu_tb_jmp_cache_remove(tb);
> > > +            }
> > >            }
> >
> > Here you would use tb_jmp_cache_clear_page(), which should be moved
> > out of cputlb.c.
>
> That was actually the first thing I tried.
>
> Unfortunately tb_jmp_cache_clear_page() relies on
> tb_jmp_cache_hash_func() returning the same top bits for addresses on
> the same page.  This is not the case for qemu-user: there this property
> was traded for better hashing with quite impressive performance
> improvements (6f1653180f570).

Oh my.  Well, we could

(1) revert that patch because the premise is wrong,
(2) go with your per-tb clearing,
(3) clear the whole thing with cpu_tb_jmp_cache_clear

Ideally we'd have some benchmark numbers to inform the choice...


r~