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

From: Uladzislau Rezki <urezki@gmail.com>
To: Baoquan He <bhe@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>,
	Uladzislau Rezki <urezki@gmail.com>,
	Lorenzo Stoakes <lstoakes@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [patch 1/6] mm/vmalloc: Prevent stale TLBs in fully utilized blocks
Date: Wed, 24 May 2023 13:36:14 +0200	[thread overview]
Message-ID: <ZG32rp2HwZi3jQKl@pc636> (raw)
In-Reply-To: <ZG3z++d6q+owmfhe@MiWiFi-R3L-srv>

On Wed, May 24, 2023 at 07:24:43PM +0800, Baoquan He wrote:
> On 05/24/23 at 11:51am, Thomas Gleixner wrote:
> > On Wed, May 24 2023 at 17:25, Baoquan He wrote:
> > > On 05/23/23 at 04:02pm, Thomas Gleixner wrote:
> > >> _vm_unmap_aliases() is used to ensure that no unflushed TLB entries for a
> > >> page are left in the system. This is required due to the lazy TLB flush
> > >> mechanism in vmalloc.
> > >> 
> > >> This is tried to achieve by walking the per CPU free lists, but those do
> > >> not contain fully utilized vmap blocks because they are removed from the
> > >> free list once the blocks free space became zero.
> > >
> > > The problem description is not accurate. This is tried to achieve for
> > > va associated with vmap_block by walking the per CPU free lists, those
> > > fully utilized vmap blocks can still be flushed in __purge_vmap_area_lazy()
> > > by calculating the [min:max] of purge_vmap_area_list, because va of
> > > vmap_blocks will be added to purge_vmap_area_list too via vb_free().
> > 
> > No. The fully utilized block cannot be purged when there are still
> > active mappings on it. Again:
> > 
> >   X = vb_alloc()
> > ...  
> >   Y = vb_alloc()
> >     vb->free -= order;
> >     if (!vb->vb_free)
> >        list_del(vb->free_list);
> > ...
> >   vb_free(Y)
> >     vb->dirty += order;
> >     if (vb->dirty == VMAP_BBMAP_BITS) // Condition is _false_
> >        free_block(); 
> 
> 
>    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);
> 
> The last mapped region will be freed and added to purge list via
> vb_free(), it will be flushed with other va in purge list. When it's
> mapped via vb_alloc(), it's detached from vbq->free list. When it's
> freed via vb_alloc(), it's added to purge list, and flushed, the thing
> is duplicated flushing, no missing flush seen here?
> 
> > 
> > So because $X is not yet unmapped the block is neither on the free list
> > nor on purge_vmap_area_list.
> 
> Yeah, because $X is not yet unmapped, the block could have unmapped part
> flushed, or unflushed. For unflushed part, it's got flushed with $X
> altogether in the purge list.
> 
If we gurantee that vb is fully flushed, then just do not add it it
purge list:

@@ -2086,12 +2090,7 @@ static void free_vmap_block(struct vmap_block *vb)
        BUG_ON(tmp != vb);

        z = addr_to_cvz(vb->va->va_start);
-
-       spin_lock(&z->busy.lock);
-       unlink_va(vb->va, &z->busy.root);
-       spin_unlock(&z->busy.lock);
-
-       free_vmap_area_noflush(vb->va);
+       free_vmap_area(vb->va);
        kfree_rcu(vb, rcu_head);
 }

and directly return back into global vmap heap.

--
Uladzislau Rezki

