Re: [V2 patch 3/6] mm/vmalloc: Prevent flushing dirty space over and over

[Lorenzo Stoakes <lstoakes@gmail.com>]

On Thu, May 25, 2023 at 02:57:05PM +0200, Thomas Gleixner wrote:
> vmap blocks which have active mappings cannot be purged. Allocations which
> have been freed are accounted for in vmap_block::dirty_min/max, so that
> they can be detected in _vm_unmap_aliases() as potentially stale TLBs.
>
> If there are several invocations of _vm_unmap_aliases() then each of them
> will flush the dirty range. That's pointless and just increases the
> probability of full TLB flushes.
>
> Avoid that by resetting the flush range after accounting for it. That's
> safe versus other invocations of _vm_unmap_aliases() because this is all
> serialized with vmap_purge_lock.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Baoquan He <bhe@redhat.com>
> ---
>  mm/vmalloc.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2226,7 +2226,7 @@ static void vb_free(unsigned long addr,
>
>  	spin_lock(&vb->lock);
>
> -	/* Expand dirty range */
> +	/* Expand the not yet TLB flushed dirty range */
>  	vb->dirty_min = min(vb->dirty_min, offset);
>  	vb->dirty_max = max(vb->dirty_max, offset + (1UL << order));
>
> @@ -2264,7 +2264,7 @@ static void _vm_unmap_aliases(unsigned l
>  			 * space to be flushed.
>  			 */
>  			if (!purge_fragmented_block(vb, vbq, &purge_list) &&
> -			    vb->dirty && vb->dirty != VMAP_BBMAP_BITS) {
> +			    vb->dirty_max && vb->dirty != VMAP_BBMAP_BITS) {
>  				unsigned long va_start = vb->va->va_start;
>  				unsigned long s, e;
>
> @@ -2274,6 +2274,10 @@ static void _vm_unmap_aliases(unsigned l
>  				start = min(s, start);
>  				end   = max(e, end);
>
> +				/* Prevent that this is flushed again */

Sorry to be absurdly nitty, and I know there's other instances of this in
vmalloc.c but it'd be nice to correct the grammar here -> 'prevent this
from being flushed again'. However, far from urgent!

> +				vb->dirty_min = VMAP_BBMAP_BITS;
> +				vb->dirty_max = 0;
> +
>  				flush = 1;
>  			}
>  			spin_unlock(&vb->lock);
>
>

This is a really good catch, feel free to add:-

Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com>