diff mbox series

[02/11] drm/ttm: cleanup io_mem interface with nouveau

Message ID 20200721073245.2484-2-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [01/11] drm: remove optional dummy function from drivers using TTM | expand

Commit Message

Christian König July 21, 2020, 7:32 a.m. UTC 
Nouveau is the only user of this functionality and evicting io space
on -EAGAIN is really a misuse of the return code.

Instead switch to using -ENOSPC here which makes much more sense and
simplifies the code.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
 drivers/gpu/drm/ttm/ttm_bo_util.c    | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

Comments

Daniel Vetter July 21, 2020, 8:50 a.m. UTC | #1 
On Tue, Jul 21, 2020 at 09:32:36AM +0200, Christian König wrote:
> Nouveau is the only user of this functionality and evicting io space
> on -EAGAIN is really a misuse of the return code.
> 
> Instead switch to using -ENOSPC here which makes much more sense and
> simplifies the code.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Going from EAGAIN to something else could unbreak something that's held
together as a almost livelock restarting ioctls or whatever. So I looked
for that a bit:

- mmap path seems fine, all errors from the io_reserve stuff here get
  remapped to sigbus

- but everywhere else we just pass down the errno it seems, and nouveau
  has a bunch of kmaps all around (gpu relocs on pre-nv50 is probably the
  big one, if we ignore the memcpy bo move fallback). I haven't found
  anything that indicates those chips don't have the ioremapping hw, so
  there's some risk I think. Otoh I also don't see anything that would
  unbreak the lifelook, so feels minimally.

With that impact to pushbuf ioctl documented in the commit message somehow
this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
>  drivers/gpu/drm/ttm/ttm_bo_util.c    | 4 ++--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 61355cfb7335..a48652826f67 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1505,8 +1505,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
>  			if (ret != 1) {
>  				if (WARN_ON(ret == 0))
>  					return -EINVAL;
> -				if (ret == -ENOSPC)
> -					return -EAGAIN;
>  				return ret;
>  			}
>  
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 5e0f3a9caedc..7d2c50fef456 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -116,7 +116,7 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
>  	struct ttm_buffer_object *bo;
>  
>  	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
> -		return -EAGAIN;
> +		return -ENOSPC;
>  
>  	bo = list_first_entry(&man->io_reserve_lru,
>  			      struct ttm_buffer_object,
> @@ -143,7 +143,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
>  	    mem->bus.io_reserved_count++ == 0) {
>  retry:
>  		ret = bdev->driver->io_mem_reserve(bdev, mem);
> -		if (ret == -EAGAIN) {
> +		if (ret == -ENOSPC) {
>  			ret = ttm_mem_io_evict(man);
>  			if (ret == 0)
>  				goto retry;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Chauhan, Madhav July 22, 2020, 7:05 a.m. UTC | #2 
[AMD Public Use]

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Tuesday, July 21, 2020 1:03 PM
To: dri-devel@lists.freedesktop.org
Cc: Chauhan, Madhav <Madhav.Chauhan@amd.com>; tzimmermann@suse.de; michael.j.ruhl@intel.com
Subject: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau

Nouveau is the only user of this functionality and evicting io space on -EAGAIN is really a misuse of the return code.

Instead switch to using -ENOSPC here which makes much more sense and simplifies the code.

Signed-off-by: Christian König <christian.koenig@amd.com>

Complete remaining cleanup patches (Patch 2- 11) looks fine,
Patch 2-11: Reviewed-by: Madhav Chauhan <madhav.chauhan@amd.com>

Regards,
Madhav
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
 drivers/gpu/drm/ttm/ttm_bo_util.c    | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 61355cfb7335..a48652826f67 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1505,8 +1505,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
 			if (ret != 1) {
 				if (WARN_ON(ret == 0))
 					return -EINVAL;
-				if (ret == -ENOSPC)
-					return -EAGAIN;
 				return ret;
 			}
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 5e0f3a9caedc..7d2c50fef456 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -116,7 +116,7 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
 	struct ttm_buffer_object *bo;
 
 	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
-		return -EAGAIN;
+		return -ENOSPC;
 
 	bo = list_first_entry(&man->io_reserve_lru,
 			      struct ttm_buffer_object,
@@ -143,7 +143,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 	    mem->bus.io_reserved_count++ == 0) {
 retry:
 		ret = bdev->driver->io_mem_reserve(bdev, mem);
-		if (ret == -EAGAIN) {
+		if (ret == -ENOSPC) {
 			ret = ttm_mem_io_evict(man);
 			if (ret == 0)
 				goto retry;
--
2.17.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 61355cfb7335..a48652826f67 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1505,8 +1505,6 @@  nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
 			if (ret != 1) {
 				if (WARN_ON(ret == 0))
 					return -EINVAL;
-				if (ret == -ENOSPC)
-					return -EAGAIN;
 				return ret;
 			}
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 5e0f3a9caedc..7d2c50fef456 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -116,7 +116,7 @@  static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
 	struct ttm_buffer_object *bo;
 
 	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
-		return -EAGAIN;
+		return -ENOSPC;
 
 	bo = list_first_entry(&man->io_reserve_lru,
 			      struct ttm_buffer_object,
@@ -143,7 +143,7 @@  int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 	    mem->bus.io_reserved_count++ == 0) {
 retry:
 		ret = bdev->driver->io_mem_reserve(bdev, mem);
-		if (ret == -EAGAIN) {
+		if (ret == -ENOSPC) {
 			ret = ttm_mem_io_evict(man);
 			if (ret == 0)
 				goto retry;