Re: [PATCH 04/28] drm/qxl: use iterator instead of dma_resv_shared_list

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Nov 29, 2021 at 01:06:35PM +0100, Christian König wrote:
> I'm not sure why it is useful to know the number of fences
> in the reservation object, but we try to avoid exposing the
> dma_resv_shared_list() function.
> 
> So use the iterator instead. If more information is desired
> we could use dma_resv_describe() as well.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/qxl/qxl_debugfs.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c
> index 1f9a59601bb1..6a36b0fd845c 100644
> --- a/drivers/gpu/drm/qxl/qxl_debugfs.c
> +++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
> @@ -57,13 +57,16 @@ qxl_debugfs_buffers_info(struct seq_file *m, void *data)
>  	struct qxl_bo *bo;
>  
>  	list_for_each_entry(bo, &qdev->gem.objects, list) {
> -		struct dma_resv_list *fobj;
> -		int rel;
> -
> -		rcu_read_lock();
> -		fobj = dma_resv_shared_list(bo->tbo.base.resv);
> -		rel = fobj ? fobj->shared_count : 0;
> -		rcu_read_unlock();
> +		struct dma_resv_iter cursor;
> +		struct dma_fence *fence;
> +		int rel = 0;
> +
> +		dma_resv_iter_begin(&cursor, bo->tbo.base.resv, true);
> +		dma_resv_for_each_fence_unlocked(&cursor, fence) {
> +			if (dma_resv_iter_is_restarted(&cursor))

I think a kerneldoc patch for struct dma_resv_iter to highlight that
iterators can be restarted and users need to take that into account if
they accumulate statistics with dma_resv_iter_is_restarted() would be
good. I guess I'll go type that.

On this patch:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +				rel = 0;
> +			++rel;
> +		}
>  
>  		seq_printf(m, "size %ld, pc %d, num releases %d\n",
>  			   (unsigned long)bo->tbo.base.size,
> -- 
> 2.25.1
> 

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