Re: [PATCH 1/4] drm/xe/client: fix deadlock in show_meminfo()

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Sep 10, 2024 at 02:11:46PM +0100, Matthew Auld wrote:
> There is a real deadlock as well as sleeping in atomic() bug in here, if
> the bo put happens to be the last ref, since bo destruction wants to
> grab the same spinlock and sleeping locks.  Fix that by dropping the ref
> using xe_bo_put_deferred(), and moving the final commit outside of the
> lock. Dropping the lock around the put is tricky since the bo can go
> out of scope and delete itself from the list, making it difficult to
> navigate to the next list entry.
> 
> Fixes: 0845233388f8 ("drm/xe: Implement fdinfo memory stats printing")
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2727
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v6.8+
> ---
>  drivers/gpu/drm/xe/xe_drm_client.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
> index e64f4b645e2e..badfa045ead8 100644
> --- a/drivers/gpu/drm/xe/xe_drm_client.c
> +++ b/drivers/gpu/drm/xe/xe_drm_client.c
> @@ -196,6 +196,7 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
>  	struct xe_drm_client *client;
>  	struct drm_gem_object *obj;
>  	struct xe_bo *bo;
> +	LLIST_HEAD(deferred);
>  	unsigned int id;
>  	u32 mem_type;
>  
> @@ -215,11 +216,14 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
>  	list_for_each_entry(bo, &client->bos_list, client_link) {
>  		if (!kref_get_unless_zero(&bo->ttm.base.refcount))
>  			continue;
> +
>  		bo_meminfo(bo, stats);
> -		xe_bo_put(bo);
> +		xe_bo_put_deferred(bo, &deferred);
>  	}
>  	spin_unlock(&client->bos_lock);
>  
> +	xe_bo_put_commit(&deferred);
> +
>  	for (mem_type = XE_PL_SYSTEM; mem_type < TTM_NUM_MEM_TYPES; ++mem_type) {
>  		if (!xe_mem_type_to_name[mem_type])
>  			continue;
> -- 
> 2.46.0
>