Re: [patch 1/6] mm/vmalloc: Prevent stale TLBs in fully utilized blocks

[Baoquan He <bhe@redhat.com>]

On 05/24/23 at 02:44pm, Thomas Gleixner wrote:
> On Wed, May 24 2023 at 19:24, Baoquan He wrote:
> > On 05/24/23 at 11:51am, Thomas Gleixner wrote:
> >    vb_free(Y)
> >      vb->dirty += order;
> >      if (vb->dirty == VMAP_BBMAP_BITS) // Condition is _false_
> >         free_vmap_block(); 
> >         -->free_vmap_area_noflush()
> >            -->merge_or_add_vmap_area(va,
> >                 &purge_vmap_area_root, &purge_vmap_area_list);
> 
> This is irrelevant. The path is _NOT_ taken. You even copied the
> comment:

Ah, just copied the whole, didn't notice that. I am not a scrupulous
person.

> 
>        if (vb->dirty == VMAP_BBMAP_BITS) // Condition is _false_
> 
> Did you actually read what I wrote?
> 
> Again: It _CANNOT_ be on the purge list because it has active mappings:
> 
> 1  X = vb_alloc()
>    ...  
>    Y = vb_alloc()
>      vb->free -= order;               // Free space goes to 0
>      if (!vb->vb_free)
> 2      list_del(vb->free_list);       // Block is removed from free list
>    ...
>    vb_free(Y)
>      vb->dirty += order;
> 3    if (vb->dirty == VMAP_BBMAP_BITS) // Condition is _false_
>                                        // because #1 $X is still mapped
>                                        // so block is _NOT_ freed and
>                                        // _NOT_ put on the purge list

So what if $X is unmapped via vb_free($X)? Does the condition satisfied
and can the vb put into purge list?

In your above example, $Y's flush is deferred, but not missed?


> 
> 4   unmap_aliases()
>      walk_free_list()           // Does not find it because of #2
>      walk_purge_list()          // Does not find it because of #3
> 
> If the resulting flush range is not covering the $Y TLBs then stale TLBs
> stay around.

OK, your mean the TLB of $Y will stay around after vb_free() until
the whole vb becomes dirty, and fix that in this patch, you are right.
vm_unmap_aliases() may need try to flush all unmapped ranges in
this case but failed on $Y, while the page which is being reused has the
old alias of $Y.

My thought was attracted to the repeated flush of vmap_block va on purge
list.

By the way, you don't fix issue that in vm_reset_perms(), the direct map 
range will be accumulated with vb va and purge va and could produce
flushing range including huge gap, do you still plan to fix that? I
remember you said you will use array to gather ranges and flush them one
by one.

> 
> The xarray walk finds it and guarantees that the TLBs are gone when
> unmap_aliases() returns, which is the whole purpose of that function.
> 
> Thanks,
> 
>         tglx
>