On 07.10.25 15:57, Tvrtko Ursulin wrote:
>>> -#define TTM_POOL_USE_DMA_ALLOC BIT(0) /* Use coherent DMA
>>> allocations. */
>>> -#define TTM_POOL_USE_DMA32 BIT(1) /* Use GFP_DMA32 allocations. */
>>> +#define TTM_POOL_BENEFICIAL_ORDER(n) ((n) & 0xff) /* Max order which
>>> caller can benefit from */
>>
>> Looks good in general, but I'm not 100% convinced that we want to mix this
>> value into the flags.
>>
>> On the one hand it makes your live easier because you don't need to change
>> all drivers using it, on the other hand changing all drivers using it would
>> potentially be cleaner and document the value better.
>
> I was not 100% convinced either but it looked a reasonable compromise.
>
> My thinking was to not simply add an int after the existing two booleans but
> to try and clean it up at the same time. Once I replaced them with flags then
> the option were to either add a new int argument or add some flags like
> TTM_POOL_BENEFICIAL_SIZE_2M, TTM_POOL_BENEFICIAL_SIZE_64K, with the thinking
> there probably isn't a full range of page sizes. But then I thought why not
> just put the order in some bits. Advantages being it adds names to anonymous
> booleans and is extensible with no further churn.
>
> But I don't know, I am happy to change it to something else if you are sure
> this isn't the way.
>
> If we add a new int then it has to have some "stick with default" semantics.
> Like -1 or whatnot. With is also meh. I wouldn't do a zero because it feels
> conflated.
Ideally drivers would use MAX_PAGE_ORDER, but yeah that is quite some work to
do.
Feel free to go ahead with the flags approach, it should solve the problem and
is not much work for now.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
>
>>> +#define TTM_POOL_USE_DMA_ALLOC BIT(8) /* Use coherent DMA
>>> allocations. */
>>> +#define TTM_POOL_USE_DMA32 BIT(9) /* Use GFP_DMA32 allocations. */
>>> /**
>>> * struct ttm_pool - Pool for all caching and orders
>>> @@ -111,4 +112,9 @@ static inline bool ttm_pool_uses_dma32(struct ttm_pool
>>> *pool)
>>> return pool->flags & TTM_POOL_USE_DMA32;
>>> }
>>> +static inline bool ttm_pool_beneficial_order(struct ttm_pool *pool)
>>> +{
>>> + return pool->flags & 0xff;
>>> +}
>>> +
>>> #endif
>>
>
