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

From: daniel@ffwll.ch
Cc: Madhav.Chauhan@amd.com, michael.j.ruhl@intel.com,
	tzimmermann@suse.de, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau
Date: Tue, 21 Jul 2020 10:50:32 +0200	[thread overview]
Message-ID: <20200721085032.GA6419@phenom.ffwll.local> (raw)
In-Reply-To: <20200721073245.2484-2-christian.koenig@amd.com>

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel