Re: [Intel-gfx] [PATCH v3] drm/i915: Handle incomplete Z_FINISH for compressed error states

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/10/2018 09:24, Chris Wilson wrote:
The final call to zlib_deflate(Z_FINISH) may require more output
space to be allocated and so needs to re-invoked. Failure to do so in
the current code leads to incomplete zlib streams (albeit intact due to
the use of Z_SYNC_FLUSH) resulting in the occasional short object
capture.

v2: Check against overrunning our pre-allocated page array
v3: Drop Z_SYNC_FLUSH entirely

Testcase: igt/i915-error-capture.js
Fixes: 0a97015d45ee ("drm/i915: Compress GPU objects in error state")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # v4.10+
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gpu_error.c | 88 +++++++++++++++++++--------
  drivers/gpu/drm/i915/i915_gpu_error.h |  1 +
  2 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 3d5554f14dfd..705ff122100f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -232,6 +232,20 @@ static bool compress_init(struct compress *c)
  	return true;
  }
+static void *compress_next_page(struct drm_i915_error_object *dst)
+{
+	unsigned long page;
+
+	if (dst->page_count >= dst->num_pages)
+		return ERR_PTR(-ENOSPC);
+
+	page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
+	if (!page)
+		return ERR_PTR(-ENOMEM);
+
+	return dst->pages[dst->page_count++] = (void *)page;
+}
+
  static int compress_page(struct compress *c,
  			 void *src,
  			 struct drm_i915_error_object *dst)
@@ -245,19 +259,14 @@ static int compress_page(struct compress *c,
do {
  		if (zstream->avail_out == 0) {
-			unsigned long page;
-
-			page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
-			if (!page)
-				return -ENOMEM;
+			zstream->next_out = compress_next_page(dst);
+			if (IS_ERR(zstream->next_out))
+				return PTR_ERR(zstream->next_out);
- dst->pages[dst->page_count++] = (void *)page;
-
-			zstream->next_out = (void *)page;
  			zstream->avail_out = PAGE_SIZE;
  		}
- if (zlib_deflate(zstream, Z_SYNC_FLUSH) != Z_OK)
+		if (zlib_deflate(zstream, Z_NO_FLUSH) != Z_OK)
  			return -EIO;
  	} while (zstream->avail_in);
@@ -268,19 +277,42 @@ static int compress_page(struct compress *c,
  	return 0;
  }
-static void compress_fini(struct compress *c,
+static int compress_flush(struct compress *c,
  			  struct drm_i915_error_object *dst)
  {
  	struct z_stream_s *zstream = &c->zstream;
- if (dst) {
-		zlib_deflate(zstream, Z_FINISH);
-		dst->unused = zstream->avail_out;
-	}
+	do {
+		switch (zlib_deflate(zstream, Z_FINISH)) {
+		case Z_OK: /* more space requested */
+			zstream->next_out = compress_next_page(dst);
+			if (IS_ERR(zstream->next_out))
+				return PTR_ERR(zstream->next_out);
+
+			zstream->avail_out = PAGE_SIZE;
+			break;
+
+		case Z_STREAM_END:
+			goto end;
+
+		default: /* any error */
+			return -EIO;
+		}
+	} while (1);
+
+end:
+	memset(zstream->next_out, 0, zstream->avail_out);
+	dst->unused = zstream->avail_out;
+	return 0;
+}
+
+static void compress_fini(struct compress *c,
+			  struct drm_i915_error_object *dst)
+{
+	struct z_stream_s *zstream = &c->zstream;
zlib_deflateEnd(zstream);
  	kfree(zstream->workspace);
-
  	if (c->tmp)
  		free_page((unsigned long)c->tmp);
  }
@@ -319,6 +351,12 @@ static int compress_page(struct compress *c,
  	return 0;
  }
+static int compress_flush(struct compress *c,
+			  struct drm_i915_error_object *dst)
+{
+	return 0;
+}
+
  static void compress_fini(struct compress *c,
  			  struct drm_i915_error_object *dst)
  {
@@ -917,6 +955,7 @@ i915_error_object_create(struct drm_i915_private *i915,
  	unsigned long num_pages;
  	struct sgt_iter iter;
  	dma_addr_t dma;
+	int ret;
if (!vma)
  		return NULL;
@@ -930,6 +969,7 @@ i915_error_object_create(struct drm_i915_private *i915,
dst->gtt_offset = vma->node.start;
  	dst->gtt_size = vma->node.size;
+	dst->num_pages = num_pages;
  	dst->page_count = 0;
  	dst->unused = 0;
@@ -938,28 +978,26 @@ i915_error_object_create(struct drm_i915_private *i915,
  		return NULL;
  	}
+ ret = -EINVAL;
  	for_each_sgt_dma(dma, iter, vma->pages)  >   		void __iomem *s;
-		int ret;
ggtt->vm.insert_page(&ggtt->vm, dma, slot, I915_CACHE_NONE, 0); s = io_mapping_map_atomic_wc(&ggtt->iomap, slot);
  		ret = compress_page(&compress, (void  __force *)s, dst);
  		io_mapping_unmap_atomic(s);
-
  		if (ret)
-			goto unwind;
+			break;
  	}
-	goto out;
-unwind:
-	while (dst->page_count--)
-		free_page((unsigned long)dst->pages[dst->page_count]);
-	kfree(dst);
-	dst = NULL;
+	if (ret || compress_flush(&compress, dst)) {
+		while (dst->page_count--)
+			free_page((unsigned long)dst->pages[dst->page_count]);
+		kfree(dst);
+		dst = NULL;
+	}
-out:
  	compress_fini(&compress, dst);
  	ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE);
  	return dst;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
index f893a4e8b783..8710fb18ed74 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -135,6 +135,7 @@ struct i915_gpu_state {
  		struct drm_i915_error_object {
  			u64 gtt_offset;
  			u64 gtt_size;
+			int num_pages;
  			int page_count;
  			int unused;
  			u32 *pages[0];


Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux