| Message ID | 20230525124504.692056496@linutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/vmalloc: Assorted fixes and improvements | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
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>
--- 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 */ + vb->dirty_min = VMAP_BBMAP_BITS; + vb->dirty_max = 0; + flush = 1; } spin_unlock(&vb->lock);