Re: [PATCH v8 3/6] drm/ttm: Extract code for attempting allocation in a place

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The synthetic `-EBUSY` signal collides with the native `-EBUSY` error code, causing unintended evictions and silent API contract changes.
--

--- Patch [3]: [PATCH v8 3/6] drm/ttm: Extract code for attempting allocation in a place ---
commit 8416fd662f16adcc69fd20180c1e3da4cb94ac03
Author: Natalie Vock <[email protected]>

drm/ttm: Extract code for attempting allocation in a place

This commit moves the logic for attempting buffer allocation for a specific
place into a separate ttm_bo_alloc_at_place() function. It uses a synthetic
-EBUSY return code to signal that eviction should be attempted.

> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3980f376e3ba4..1cdb2172d5924 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c

[ ... ]

> @@ -488,6 +488,51 @@ int ttm_bo_evict_first(struct ttm_device *bdev, struct ttm_resource_manager *man
> +static int ttm_bo_alloc_at_place(struct ttm_buffer_object *bo,
> +				 const struct ttm_place *place,
> +				 bool force_space,
> +				 struct ttm_resource **res,
> +				 struct ttm_bo_alloc_state *alloc_state)
> +{
> +	bool may_evict;
> +	int ret;
> +
> +	may_evict = force_space && place->mem_type != TTM_PL_SYSTEM;
> +
> +	ret = ttm_resource_alloc(bo, place, res,
> +				 force_space ? &alloc_state->limit_pool : NULL);
> +
> +	if (ret) {
> +		if (ret == -ENOSPC && may_evict)
> +			return -EBUSY;
> +
> +		return ret;

[Severity: Medium]
If ttm_resource_alloc() organically returns -EBUSY, will this native error be
passed up unmodified and collide with the synthetic -EBUSY eviction signal?

> +	}
> +
> +	return 0;
> +}

[ ... ]

> @@ -736,25 +782,25 @@ static int ttm_bo_alloc_resource(struct ttm_buffer_object *bo,
> -		may_evict = (force_space && place->mem_type != TTM_PL_SYSTEM);
> -		ret = ttm_resource_alloc(bo, place, res, force_space ? &limit_pool : NULL);
> -		if (ret) {
> -			if (ret != -ENOSPC) {
> -				dmem_cgroup_pool_state_put(limit_pool);
> -				return ret;
> -			}
> -			if (!may_evict) {
> -				dmem_cgroup_pool_state_put(limit_pool);
> -				continue;
> -			}
> +		ret = ttm_bo_alloc_at_place(bo, place, force_space, res,
> +					    &alloc_state);
>  
> +		if (ret == -ENOSPC) {
> +			dmem_cgroup_pool_state_put(alloc_state.limit_pool);
> +			continue;
> +		} else if (ret == -EBUSY) {
>  			ret = ttm_bo_evict_alloc(bdev, man, place, bo, ctx,
> -						 ticket, res, limit_pool);
> -			dmem_cgroup_pool_state_put(limit_pool);
> +						 ticket, res, &alloc_state);

[Severity: Medium]
Before this patch, if ttm_resource_alloc() returned a native -EBUSY, it was
directly returned to the caller since it was not -ENOSPC.

By treating any -EBUSY returned from ttm_bo_alloc_at_place() as a signal to evict,
does this inadvertently cause an organic -EBUSY to trigger an unnecessary eviction
walk rather than propagating the error?

> +
> +			dmem_cgroup_pool_state_put(alloc_state.limit_pool);
> +
>  			if (ret == -EBUSY)
>  				continue;
> -			if (ret)
> +			else if (ret)
>  				return ret;
> +		} else if (ret) {
> +			dmem_cgroup_pool_state_put(alloc_state.limit_pool);
> +			return ret;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3