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

[Tvrtko Ursulin <tursulin@ursulin.net>]

On 25/02/2026 15:27, Tvrtko Ursulin wrote:
> 
> On 25/02/2026 12:10, Natalie Vock wrote:
>> Move all code for attempting allocation for a specific place to
>> ttm_bo_alloc_place. With subsequent patches, this logic is going to get
>> more complicated, so it helps readability to have this separate.
>>
>> ttm_bo_alloc_at_place takes a pointer to a struct ttm_bo_alloc_state.
>> This struct holds various state produced by the allocation (e.g. cgroup
>> resource associated with the allocation) that the caller needs to keep
>> track of (and potentially dispose of). This is just the limiting cgroup
>> pool for now, but future patches will add more state needing to be 
>> tracked.
>>
>> ttm_bo_alloc_at_place also communicates via return codes if eviction
>> using ttm_bo_evict_alloc should be attempted. This is preparation for
>> attempting eviction in more cases than just force_space being set.
>>
>> No functional change intended.
>>
>> Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c | 109 ++++++++++++++++++++++++++++++++ 
>> +----------
>>   1 file changed, 84 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index acb9197db8798..48dbaaa46824c 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -489,6 +489,11 @@ int ttm_bo_evict_first(struct ttm_device *bdev, 
>> struct ttm_resource_manager *man
>>       return ret;
>>   }
>> +struct ttm_bo_alloc_state {
>> +    /** @limit_pool: Which pool limit we should test against */
>> +    struct dmem_cgroup_pool_state *limit_pool;
>> +};
>> +
>>   /**
>>    * struct ttm_bo_evict_walk - Parameters for the evict walk.
>>    */
>> @@ -504,12 +509,13 @@ struct ttm_bo_evict_walk {
>>       /** @evicted: Number of successful evictions. */
>>       unsigned long evicted;
>> -    /** @limit_pool: Which pool limit we should test against */
>> -    struct dmem_cgroup_pool_state *limit_pool;
>>       /** @try_low: Whether we should attempt to evict BO's with low 
>> watermark threshold */
>>       bool try_low;
>>       /** @hit_low: If we cannot evict a bo when @try_low is false 
>> (first pass) */
>>       bool hit_low;
>> +
>> +    /** @alloc_state: State associated with the allocation attempt. */
>> +    struct ttm_bo_alloc_state *alloc_state;
>>   };
>>   static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct 
>> ttm_buffer_object *bo)
>> @@ -518,8 +524,9 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk 
>> *walk, struct ttm_buffer_object *
>>           container_of(walk, typeof(*evict_walk), walk);
>>       s64 lret;
>> -    if (!dmem_cgroup_state_evict_valuable(evict_walk->limit_pool, bo- 
>> >resource->css,
>> -                          evict_walk->try_low, &evict_walk->hit_low))
>> +    if (!dmem_cgroup_state_evict_valuable(evict_walk->alloc_state- 
>> >limit_pool,
>> +                          bo->resource->css, evict_walk->try_low,
>> +                          &evict_walk->hit_low))
>>           return 0;
>>       if (bo->pin_count || !bo->bdev->funcs->eviction_valuable(bo, 
>> evict_walk->place))
>> @@ -561,7 +568,7 @@ static int ttm_bo_evict_alloc(struct ttm_device 
>> *bdev,
>>                     struct ttm_operation_ctx *ctx,
>>                     struct ww_acquire_ctx *ticket,
>>                     struct ttm_resource **res,
>> -                  struct dmem_cgroup_pool_state *limit_pool)
>> +                  struct ttm_bo_alloc_state *state)
>>   {
>>       struct ttm_bo_evict_walk evict_walk = {
>>           .walk = {
>> @@ -574,7 +581,7 @@ static int ttm_bo_evict_alloc(struct ttm_device 
>> *bdev,
>>           .place = place,
>>           .evictor = evictor,
>>           .res = res,
>> -        .limit_pool = limit_pool,
>> +        .alloc_state = state,
>>       };
>>       s64 lret;
>> @@ -689,6 +696,58 @@ static int 
>> ttm_bo_add_pipelined_eviction_fences(struct ttm_buffer_object *bo,
>>       return dma_resv_reserve_fences(bo->base.resv, 1);
>>   }
>> +
>> +/**
>> + * ttm_bo_alloc_at_place - Attempt allocating a BO's backing store in 
>> a place
>> + *
>> + * @bo: The buffer to allocate the backing store of
>> + * @place: The place to attempt allocation in
>> + * @ctx: ttm_operation_ctx associated with this allocation
>> + * @force_space: If we should evict buffers to force space
>> + * @res: On allocation success, the resulting struct ttm_resource.
>> + * @alloc_state: Object holding allocation state such as charged 
>> cgroups.
>> + *
>> + * Returns:
>> + * -EBUSY: No space available, but allocation should be retried with 
>> ttm_bo_evict_alloc.
>> + * -ENOSPC: No space available, allocation should not be retried.
>> + * -ERESTARTSYS: An interruptible sleep was interrupted by a signal.
>> + *
>> + */
>> +static int ttm_bo_alloc_at_place(struct ttm_buffer_object *bo,
>> +                 const struct ttm_place *place,
>> +                 struct ttm_operation_ctx *ctx,
>> +                 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) {
>> +        /*
>> +         * -EAGAIN means the charge failed, which we treat like an
>> +         * allocation failure. Therefore, return an error code 
>> indicating
>> +         * the allocation failed - either -EBUSY if the allocation 
>> should
>> +         * be retried with eviction, or -ENOSPC if there should be no 
>> second
>> +         * attempt.
>> +         */
> 
> Ah having started reading 4/6 I see this comment actually is one patch 
> premature. So please fix that and keep my r-b.

Or perhaps you are talking about charge here, not because in a later 
patch the call to try charge is put right here, but because even now it 
is happening inside the ttm_resource_alloc? I guess that's passable 
although not immediately obvious from just the context of this function. 
Okay, I think the comment can stay as is since in the next patch it 
becomes immediately obvious, sorry for the noise.

Regards,

Tvrtko

>> +        if (ret == -EAGAIN)
>> +            return may_evict ? -EBUSY : -ENOSPC;
>> +
>> +        if (ret == -ENOSPC && may_evict)
>> +            return -EBUSY;
>> +
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   /**
>>    * ttm_bo_alloc_resource - Allocate backing store for a BO
>>    *
>> @@ -714,7 +773,9 @@ static int ttm_bo_alloc_resource(struct 
>> ttm_buffer_object *bo,
>>                    bool force_space,
>>                    struct ttm_resource **res)
>>   {
>> +    struct ttm_bo_alloc_state alloc_state = {0};
>>       struct ttm_device *bdev = bo->bdev;
>> +    struct ttm_resource_manager *man;
>>       struct ww_acquire_ctx *ticket;
>>       int i, ret;
>> @@ -725,9 +786,6 @@ static int ttm_bo_alloc_resource(struct 
>> ttm_buffer_object *bo,
>>       for (i = 0; i < placement->num_placement; ++i) {
>>           const struct ttm_place *place = &placement->placement[i];
>> -        struct dmem_cgroup_pool_state *limit_pool = NULL;
>> -        struct ttm_resource_manager *man;
>> -        bool may_evict;
>>           man = ttm_manager_type(bdev, place->mem_type);
>>           if (!man || !ttm_resource_manager_used(man))
>> @@ -737,25 +795,26 @@ static int ttm_bo_alloc_resource(struct 
>> ttm_buffer_object *bo,
>>                       TTM_PL_FLAG_FALLBACK))
>>               continue;
>> -        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 && ret != -EAGAIN) {
>> -                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, ctx, 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);
>> -            if (ret == -EBUSY)
>> +                         ticket, res, &alloc_state);
>> +
>> +            dmem_cgroup_pool_state_put(alloc_state.limit_pool);
>> +
>> +            if (ret) {
>> +                if (ret != -EBUSY)
>> +                    return ret;
>>                   continue;
>> -            if (ret)
>> -                return ret;
>> +            }
>> +        } else if (ret) {
>> +            dmem_cgroup_pool_state_put(alloc_state.limit_pool);
>> +            return ret;
>>           }
>>           ret = ttm_bo_add_pipelined_eviction_fences(bo, man, ctx- 
>> >no_wait_gpu);
>>
>