Re: [igt-dev] [PATCH i-g-t v5 11/15] lib/intel_allocator: treat default_alignment as the minimum

[Matthew Auld <matthew.auld@intel.com>]

On 24/10/2023 17:39, Zbigniew Kempczyński wrote:
> On Fri, Oct 20, 2023 at 10:37:57AM +0100, Matthew Auld wrote:
>> If something overrides the default alignment, we should only apply the
>> alignment if it is larger than the default_alignment.
>>
>> v2 (Niranjana):
>>    - Simplify slightly
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>> Cc: José Roberto de Souza <jose.souza@intel.com>
>> Cc: Pallavi Mishra <pallavi.mishra@intel.com>
>> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> ---
>>   lib/intel_allocator.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
>> index e5b9457b8..96ffe40d5 100644
>> --- a/lib/intel_allocator.c
>> +++ b/lib/intel_allocator.c
>> @@ -584,8 +584,8 @@ static int handle_request(struct alloc_req *req, struct alloc_resp *resp)
>>   			break;
>>   
>>   		case REQ_ALLOC:
>> -			if (!req->alloc.alignment)
>> -				req->alloc.alignment = ial->default_alignment;
>> +			req->alloc.alignment = max(ial->default_alignment,
>> +						   req->alloc.alignment);
> 
> It will work but change current allocator behavior regarding
> requested smaller alignment than default. I mean if allocator was
> created with (let's say) 64K alignment and we want to suballocate
> with 4K we can't do that with above code. All object regardless
> our alignment requirement will be aligned to 64K.
> 
> Of course my need to have objects aligned to 4K is still reachable
> if I create allocator with 4K as enforced alignment and pass 64K
> as alignment argument on alloc().

Yeah, that seems fine to me. If I don't ask for any specific alignment 
when creating the allocator it should just give me the "safe" default 
imo, and all further allocations should be at least that, no matter what.

If something more exotic (which doesn't seem to be the common case) is 
needed with being able to use say 4K, then manually setting that when 
creating the allocator isn't too unreasonable, with then having finer 
control with each individual allocation?

> 
> What case are you solving with above code?

For my case I basically just want to force the allocator to always use 
2M+ alignment, and I don't want random lib stuff passing in 4K/64K when 
allocating an address and somehow overriding that. But perhaps I'm 
misunderstanding how this all works?

> 
> --
> Zbigniew
> 
>>   
>>   			resp->response_type = RESP_ALLOC;
>>   			resp->alloc.offset = ial->alloc(ial,
>> -- 
>> 2.41.0
>>