Ben Skeggs 1001ab3
From 4733f633c4bfb0672d5bd88a8d19a03e27a3c1d0 Mon Sep 17 00:00:00 2001
Ben Skeggs 71e9b67
From: Ben Skeggs <bskeggs@redhat.com>
Ben Skeggs 71e9b67
Date: Fri, 23 Jul 2010 09:06:52 +1000
Ben Skeggs 1001ab3
Subject: [PATCH 2/2] drm-nouveau-race-fix
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
drm/nouveau: fix race condition when under memory pressure
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
rhbz#602663
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
When VRAM is running out it's possible that the client's push buffers get
Ben Skeggs 71e9b67
evicted to main memory.  When they're validated back in, the GPU may
Ben Skeggs 71e9b67
be used for the copy back to VRAM, but the existing synchronisation code
Ben Skeggs 71e9b67
only deals with inter-channel sync, not sync between PFIFO and PGRAPH on
Ben Skeggs 71e9b67
the same channel.  This leads to PFIFO fetching from command buffers that
Ben Skeggs 71e9b67
haven't quite been copied by PGRAPH yet.
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
This patch marks push buffers as so, and forces any GPU-assisted buffer
Ben Skeggs 71e9b67
moves to be done on a different channel, which triggers the correct
Ben Skeggs 71e9b67
synchronisation to happen before we submit them.
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
After discussion with another nouveau developer, it was agreed that while
Ben Skeggs 71e9b67
this patch is fine in itself, that we'd prefer to work out a nicer, but
Ben Skeggs 71e9b67
likely much more invasive, fix upstream.
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Ben Skeggs 71e9b67
---
Ben Skeggs 71e9b67
 drivers/gpu/drm/nouveau/nouveau_bo.c  |   15 +++++++++++++
Ben Skeggs 71e9b67
 drivers/gpu/drm/nouveau/nouveau_drv.h |    1 +
Ben Skeggs 71e9b67
 drivers/gpu/drm/nouveau/nouveau_gem.c |   36 +++++++++++++++++++++++---------
Ben Skeggs 71e9b67
 3 files changed, 42 insertions(+), 10 deletions(-)
Ben Skeggs 71e9b67
Ben Skeggs 71e9b67
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
Ben Skeggs 1001ab3
index 553a01d..5e62d1b 100644
Ben Skeggs 71e9b67
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
Ben Skeggs 71e9b67
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
Ben Skeggs 71e9b67
@@ -36,6 +36,21 @@
Ben Skeggs 71e9b67
 #include <linux/log2.h>
Ben Skeggs 71e9b67
 #include <linux/slab.h>
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
+int
Ben Skeggs 71e9b67
+nouveau_bo_sync_gpu(struct nouveau_bo *nvbo, struct nouveau_channel *chan)
Ben Skeggs 71e9b67
+{
Ben Skeggs 71e9b67
+	struct nouveau_fence *prev_fence = nvbo->bo.sync_obj;
Ben Skeggs 71e9b67
+	int ret;
Ben Skeggs 71e9b67
+
Ben Skeggs 71e9b67
+	if (!prev_fence || nouveau_fence_channel(prev_fence) == chan)
Ben Skeggs 71e9b67
+		return 0;
Ben Skeggs 71e9b67
+
Ben Skeggs 71e9b67
+	spin_lock(&nvbo->bo.lock);
Ben Skeggs 71e9b67
+	ret = ttm_bo_wait(&nvbo->bo, false, false, false);
Ben Skeggs 71e9b67
+	spin_unlock(&nvbo->bo.lock);
Ben Skeggs 71e9b67
+	return ret;
Ben Skeggs 71e9b67
+}
Ben Skeggs 71e9b67
+
Ben Skeggs 71e9b67
 static void
Ben Skeggs 71e9b67
 nouveau_bo_del_ttm(struct ttm_buffer_object *bo)
Ben Skeggs 71e9b67
 {
Ben Skeggs 71e9b67
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
Ben Skeggs 1001ab3
index 2eb622b..70a16f3 100644
Ben Skeggs 71e9b67
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
Ben Skeggs 71e9b67
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
Ben Skeggs 1001ab3
@@ -1167,6 +1167,7 @@ extern u16 nouveau_bo_rd16(struct nouveau_bo *nvbo, unsigned index);
Ben Skeggs 71e9b67
 extern void nouveau_bo_wr16(struct nouveau_bo *nvbo, unsigned index, u16 val);
Ben Skeggs 71e9b67
 extern u32 nouveau_bo_rd32(struct nouveau_bo *nvbo, unsigned index);
Ben Skeggs 71e9b67
 extern void nouveau_bo_wr32(struct nouveau_bo *nvbo, unsigned index, u32 val);
Ben Skeggs 71e9b67
+extern int nouveau_bo_sync_gpu(struct nouveau_bo *, struct nouveau_channel *);
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
 /* nouveau_fence.c */
Ben Skeggs 71e9b67
 struct nouveau_fence;
Ben Skeggs 71e9b67
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
Ben Skeggs 1001ab3
index 62ac673..613f878 100644
Ben Skeggs 71e9b67
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
Ben Skeggs 71e9b67
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
Ben Skeggs 71e9b67
@@ -361,16 +361,11 @@ validate_list(struct nouveau_channel *chan, struct list_head *list,
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
 	list_for_each_entry(nvbo, list, entry) {
Ben Skeggs 71e9b67
 		struct drm_nouveau_gem_pushbuf_bo *b = &pbbo[nvbo->pbbo_index];
Ben Skeggs 71e9b67
-		struct nouveau_fence *prev_fence = nvbo->bo.sync_obj;
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
-		if (prev_fence && nouveau_fence_channel(prev_fence) != chan) {
Ben Skeggs 71e9b67
-			spin_lock(&nvbo->bo.lock);
Ben Skeggs 71e9b67
-			ret = ttm_bo_wait(&nvbo->bo, false, false, false);
Ben Skeggs 71e9b67
-			spin_unlock(&nvbo->bo.lock);
Ben Skeggs 71e9b67
-			if (unlikely(ret)) {
Ben Skeggs 71e9b67
-				NV_ERROR(dev, "fail wait other chan\n");
Ben Skeggs 71e9b67
-				return ret;
Ben Skeggs 71e9b67
-			}
Ben Skeggs 71e9b67
+		ret = nouveau_bo_sync_gpu(nvbo, chan);
Ben Skeggs 71e9b67
+		if (unlikely(ret)) {
Ben Skeggs 71e9b67
+			NV_ERROR(dev, "fail pre-validate sync\n");
Ben Skeggs 71e9b67
+			return ret;
Ben Skeggs 71e9b67
 		}
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
 		ret = nouveau_gem_set_domain(nvbo->gem, b->read_domains,
Ben Skeggs 71e9b67
@@ -381,7 +376,7 @@ validate_list(struct nouveau_channel *chan, struct list_head *list,
Ben Skeggs 71e9b67
 			return ret;
Ben Skeggs 71e9b67
 		}
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
-		nvbo->channel = chan;
Ben Skeggs 71e9b67
+		nvbo->channel = (b->read_domains & (1 << 31)) ? NULL : chan;
Ben Skeggs 71e9b67
 		ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
Ben Skeggs 71e9b67
 				      false, false, false);
Ben Skeggs 71e9b67
 		nvbo->channel = NULL;
Ben Skeggs 71e9b67
@@ -390,6 +385,12 @@ validate_list(struct nouveau_channel *chan, struct list_head *list,
Ben Skeggs 71e9b67
 			return ret;
Ben Skeggs 71e9b67
 		}
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
+		ret = nouveau_bo_sync_gpu(nvbo, chan);
Ben Skeggs 71e9b67
+		if (unlikely(ret)) {
Ben Skeggs 71e9b67
+			NV_ERROR(dev, "fail post-validate sync\n");
Ben Skeggs 71e9b67
+			return ret;
Ben Skeggs 71e9b67
+		}
Ben Skeggs 71e9b67
+
Ben Skeggs 71e9b67
 		if (nvbo->bo.offset == b->presumed.offset &&
Ben Skeggs 71e9b67
 		    ((nvbo->bo.mem.mem_type == TTM_PL_VRAM &&
Ben Skeggs 71e9b67
 		      b->presumed.domain & NOUVEAU_GEM_DOMAIN_VRAM) ||
Ben Skeggs 71e9b67
@@ -615,6 +616,21 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
 	mutex_lock(&dev->struct_mutex);
Ben Skeggs 71e9b67
 
Ben Skeggs 71e9b67
+	/* Mark push buffers as being used on PFIFO, the validation code
Ben Skeggs 71e9b67
+	 * will then make sure that if the pushbuf bo moves, that they
Ben Skeggs 71e9b67
+	 * happen on the kernel channel, which will in turn cause a sync
Ben Skeggs 71e9b67
+	 * to happen before we try and submit the push buffer.
Ben Skeggs 71e9b67
+	 */
Ben Skeggs 71e9b67
+	for (i = 0; i < req->nr_push; i++) {
Ben Skeggs 71e9b67
+		if (push[i].bo_index >= req->nr_buffers) {
Ben Skeggs 71e9b67
+			NV_ERROR(dev, "push %d buffer not in list\n", i);
Ben Skeggs 71e9b67
+			ret = -EINVAL;
Ben Skeggs 71e9b67
+			goto out;
Ben Skeggs 71e9b67
+		}
Ben Skeggs 71e9b67
+
Ben Skeggs 71e9b67
+		bo[push[i].bo_index].read_domains |= (1 << 31);
Ben Skeggs 71e9b67
+	}
Ben Skeggs 71e9b67
+
Ben Skeggs 71e9b67
 	/* Validate buffer list */
Ben Skeggs 71e9b67
 	ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo, req->buffers,
Ben Skeggs 71e9b67
 					   req->nr_buffers, &op, &do_reloc);
Ben Skeggs 71e9b67
-- 
Ben Skeggs 1001ab3
1.7.2.2
Ben Skeggs 71e9b67