Re: [PATCH 3/7] dma-buf: cleanup dma-resv shared fence debugging a bit

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 02, 2021 at 01:17:10PM +0200, Christian König wrote:
> Make that a function instead of inline.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Hm yeah pulling that out makes some sense.

> ---
>  drivers/dma-buf/dma-resv.c | 18 ++++++++++++++++++
>  include/linux/dma-resv.h   | 15 +++++++--------
>  2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 87f5d82d992a..6c6195315e9f 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -208,6 +208,24 @@ int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences)
>  }
>  EXPORT_SYMBOL(dma_resv_reserve_shared);
>  
> +#ifdef CONFIG_DEBUG_MUTEXES
> +/**
> + * dma_resv_reset_shared_max - reset shared fences for debugging
> + * @obj: the dma_resv object to reset
> + *
> + * Reset the shared_max to test if drivers do correct slot allocation.

I'd phrase this as

"Reset the number of pre-reserved shared slots to test that drivers do
correct slot allocation using dma_resv_reserve_shared(). See also
&dma_resv_list.shared_max."

That way it's a bit clearer how it all ties together.

The kerneldoc for dma_resv_list is also a bit thin, but another patch for
that. With the kerneldoc polish:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> + */
> +void dma_resv_reset_shared_max(struct dma_resv *obj)
> +{
> +	/* Test shared fence slot reservation */
> +	if (rcu_access_pointer(obj->fence)) {
> +		struct dma_resv_list *fence = dma_resv_get_list(obj);
> +
> +		fence->shared_max = fence->shared_count;
> +	}
> +}
> +#endif
> +
>  /**
>   * dma_resv_add_shared_fence - Add a fence to a shared slot
>   * @obj: the reservation object
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index d44a77e8a7e3..f32a3d176513 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -92,6 +92,12 @@ static inline struct dma_resv_list *dma_resv_get_list(struct dma_resv *obj)
>  					 dma_resv_held(obj));
>  }
>  
> +#ifdef CONFIG_DEBUG_MUTEXES
> +void dma_resv_reset_shared_max(struct dma_resv *obj);
> +#else
> +static inline void dma_resv_reset_shared_max(struct dma_resv *obj) {}
> +#endif
> +
>  /**
>   * dma_resv_lock - lock the reservation object
>   * @obj: the reservation object
> @@ -215,14 +221,7 @@ static inline struct ww_acquire_ctx *dma_resv_locking_ctx(struct dma_resv *obj)
>   */
>  static inline void dma_resv_unlock(struct dma_resv *obj)
>  {
> -#ifdef CONFIG_DEBUG_MUTEXES
> -	/* Test shared fence slot reservation */
> -	if (rcu_access_pointer(obj->fence)) {
> -		struct dma_resv_list *fence = dma_resv_get_list(obj);
> -
> -		fence->shared_max = fence->shared_count;
> -	}
> -#endif
> +	dma_resv_reset_shared_max(obj);
>  	ww_mutex_unlock(&obj->lock);
>  }
>  
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch