Re: Excessive TLB flush ranges

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Christoph Hellwig <hch@lst.de>,
	Uladzislau Rezki <urezki@gmail.com>,
	Lorenzo Stoakes <lstoakes@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Baoquan He <bhe@redhat.com>, John Ogness <jogness@linutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	Mark Rutland <mark.rutland@arm.com>,
	Marc Zyngier <maz@kernel.org>,
	x86@kernel.org
Subject: Re: Excessive TLB flush ranges
Date: Tue, 16 May 2023 09:19:32 +0100	[thread overview]
Message-ID: <ZGM8lKpX2zxbNRl/@shell.armlinux.org.uk> (raw)
In-Reply-To: <87r0rg93z5.ffs@tglx>

On Tue, May 16, 2023 at 08:37:18AM +0200, Thomas Gleixner wrote:
> On Mon, May 15 2023 at 22:31, Russell King wrote:
> > On Mon, May 15, 2023 at 11:11:45PM +0200, Thomas Gleixner wrote:
> >> But that's not necessarily true for ARM32 as there are no IPIs involved
> >> on the machine we are using, which is a dual-core Cortex-A9.
> >> 
> >> So I came up with the hack below, which is equally fast as the full
> >> flush variant while the performance impact on the other CPUs is minimally
> >> lower according to perf.
> >> 
> >> That probably should have another argument which tells how many TLBs
> >> this flush affects, i.e. 3 in this example, so an architecture can
> >> sensibly decide whether it wants to use flush all or not.
> >> @@ -1747,7 +1748,12 @@ static bool __purge_vmap_area_lazy(unsig
> >>  		list_last_entry(&local_purge_list,
> >>  			struct vmap_area, list)->va_end);
> >>  
> >> -	flush_tlb_kernel_range(start, end);
> >> +	if (tmp.va_end > tmp.va_start)
> >> +		list_add(&tmp.list, &local_purge_list);
> >> +	flush_tlb_kernel_vas(&local_purge_list);
> >> +	if (tmp.va_end > tmp.va_start)
> >> +		list_del(&tmp.list);
> >
> > So basically we end up iterating over each VA range, which seems
> > sensible if the range is large and we have to iterate over it page
> > by page.
> 
> Right.
> 
> > In the case you have, are "start" and "end" set on function entry
> > to a range, or are they set to ULONG_MAX,0 ? What I'm wondering is
> > whether we could get away with just having flush_tlb_kernel_vas().
> >
> > Whether that's acceptable to others is a different question :)
> 
> As I said flush_tlb_kernel_vas() should be
> 
> void flush_tlb_kernel_vas(struct list_head *list, unsigned int num_entries):
> 
> So that an architecture can decide whether it's worth to do walk the
> entries or whether it resorts to a flush all.

Is "num_entries" what an arch would want to use? How would it use that?
It doesn't tell an arch whether there is a large range of many list
entries, or a single entry covering a large range.

Wouldn't passing "start" and "end" allow an arch to check for a small
range, and if the range is small, just call flush_tlb_kernel_range().
If the range is larger, then it can walk the list to decide whether
it makes sense to flush by list entry. If it deems that it would be
too much, then it could then flush all tlb entries.

The down-side to that approach is its more arch-side code of course.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel