Nouveau - Sep 2020 - [bug report] drm/nouveau: move io_reserve_lru handling into the driver v5

Hello Christian K?nig,

The patch 141b15e59175: "drm/nouveau: move io_reserve_lru handling
into the driver v5" from Aug 21, 2020, leads to the following static
checker warning:

	drivers/gpu/drm/nouveau/nouveau_ttm.c:148 nouveau_ttm_fault()
	warn: inconsistent returns '*bo->base.resv'.

drivers/gpu/drm/nouveau/nouveau_ttm.c
   126  static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
   127  {
   128          struct vm_area_struct *vma = vmf->vma;
   129          struct ttm_buffer_object *bo = vma->vm_private_data;
   130          pgprot_t prot;
   131          vm_fault_t ret;
   132  
   133          ret = ttm_bo_vm_reserve(bo, vmf);
   134          if (ret)
   135                  return ret;
   136  
   137          nouveau_bo_del_io_reserve_lru(bo);
   138  
   139          prot = vm_get_page_prot(vma->vm_flags);
   140          ret = ttm_bo_vm_fault_reserved(vmf, prot,
TTM_BO_VM_NUM_PREFAULT, 1);
   141          if (ret == VM_FAULT_RETRY && !(vmf->flags &
FAULT_FLAG_RETRY_NOWAIT))
   142                  return ret;
                        ^^^^^^^^^^
Call dma_resv_unlock() before returning?

   143  
   144          nouveau_bo_add_io_reserve_lru(bo);
   145  
   146          dma_resv_unlock(bo->base.resv);
   147  
   148          return ret;
   149  }

regards,
dan carpenter

Hi Dan,

Am 09.09.20 um 13:49 schrieb Dan Carpenter:
> Hello Christian K?nig,
>
> The patch 141b15e59175: "drm/nouveau: move io_reserve_lru handling
> into the driver v5" from Aug 21, 2020, leads to the following static
> checker warning:
>
> 	drivers/gpu/drm/nouveau/nouveau_ttm.c:148 nouveau_ttm_fault()
> 	warn: inconsistent returns '*bo->base.resv'.
>
> drivers/gpu/drm/nouveau/nouveau_ttm.c
>     126  static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
>     127  {
>     128          struct vm_area_struct *vma = vmf->vma;
>     129          struct ttm_buffer_object *bo = vma->vm_private_data;
>     130          pgprot_t prot;
>     131          vm_fault_t ret;
>     132
>     133          ret = ttm_bo_vm_reserve(bo, vmf);
>     134          if (ret)
>     135                  return ret;
>     136
>     137          nouveau_bo_del_io_reserve_lru(bo);
>     138
>     139          prot = vm_get_page_prot(vma->vm_flags);
>     140          ret = ttm_bo_vm_fault_reserved(vmf, prot,
TTM_BO_VM_NUM_PREFAULT, 1);
>     141          if (ret == VM_FAULT_RETRY && !(vmf->flags &
FAULT_FLAG_RETRY_NOWAIT))
>     142                  return ret;
>                          ^^^^^^^^^^
> Call dma_resv_unlock() before returning?
No, this is correct. In the case of returning VM_FAULT_RETRY the 
function ttm_bo_vm_fault_reserved() has already dropped the lock for us.

Regards,
Christian.
>
>     143
>     144          nouveau_bo_add_io_reserve_lru(bo);
>     145
>     146          dma_resv_unlock(bo->base.resv);
>     147
>     148          return ret;
>     149  }
>
> regards,
> dan carpenter