[Intel-gfx] [PATCH 2/4] drm/cache: Try to be smarter about clflushing on x86

[Ben Widawsky]

Any GEM driver which has very large objects and a slow CPU is subject to very
long waits simply for clflushing incoherent objects. Generally, each individual
object is not a problem, but if you have very large objects, or very many
objects, the flushing begins to show up in profiles. Because on x86 we know the
cache size, we can easily determine when an object will use all the cache, and
forego iterating over each cacheline.

We need to be careful when using wbinvd. wbinvd() is itself potentially slow
because it requires synchronizing the flush across all CPUs so they have a
coherent view of memory. This can result in either stalling work being done on
other CPUs, or this call itself stalling while waiting for a CPU to accept the
interrupt. Also, wbinvd() also has the downside of invalidating all cachelines,
so we don't want to use it unless we're sure we already own most of the
cachelines.

The current algorithm is very naive. I think it can be tweaked more, and it
would be good if someone else gave it some thought. I am pretty confident in
i915, we can even skip the IPI in the execbuf path with minimal code change (or
perhaps just some verifying of the existing code). It would be nice to hear what
other developers who depend on this code think.

Cc: Intel GFX <intel-gfx at lists.freedesktop.org>
Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
---
 drivers/gpu/drm/drm_cache.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index d7797e8..6009c2d 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -64,6 +64,20 @@ static void drm_cache_flush_clflush(struct page *pages[],
 		drm_clflush_page(*pages++);
 	mb();
 }
+
+static bool
+drm_cache_should_clflush(unsigned long num_pages)
+{
+	const int cache_size = boot_cpu_data.x86_cache_size;
+
+	/* For now the algorithm simply checks if the number of pages to be
+	 * flushed is greater than the entire system cache. One could make the
+	 * function more aware of the actual system (ie. if SMP, how large is
+	 * the cache, CPU freq. etc. All those help to determine when to
+	 * wbinvd() */
+	WARN_ON_ONCE(!cache_size);
+	return !cache_size || num_pages < (cache_size >> 2);
+}
 #endif
 
 void
@@ -71,7 +85,7 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages)
 {
 
 #if defined(CONFIG_X86)
-	if (cpu_has_clflush) {
+	if (cpu_has_clflush && drm_cache_should_clflush(num_pages)) {
 		drm_cache_flush_clflush(pages, num_pages);
 		return;
 	}
@@ -104,7 +118,7 @@ void
 drm_clflush_sg(struct sg_table *st)
 {
 #if defined(CONFIG_X86)
-	if (cpu_has_clflush) {
+	if (cpu_has_clflush && drm_cache_should_clflush(st->nents)) {
 		struct sg_page_iter sg_iter;
 
 		mb();
@@ -128,7 +142,7 @@ void
 drm_clflush_virt_range(void *addr, unsigned long length)
 {
 #if defined(CONFIG_X86)
-	if (cpu_has_clflush) {
+	if (cpu_has_clflush && drm_cache_should_clflush(length / PAGE_SIZE)) {
 		void *end = addr + length;
 		mb();
 		for (; addr < end; addr += boot_cpu_data.x86_clflush_size)
-- 
2.1.3