Re: [PATCH v5 2/6] dma: swiotlb: Track whether the pool size was explicitly set

[Aneesh Kumar K . V]

Robin Murphy <robin.murphy@xxxxxxx> writes:

> On 21/09/2026 7:36 am, Aneesh Kumar K.V (Arm) wrote:
>> swiotlb_adjust_size() must not override a pool size supplied through the
>> swiotlb= command-line parameter.
>> 
>> Currently, it determines whether a size was supplied by comparing
>> default_nslabs with IO_TLB_DEFAULT_SIZE. This fails when the user
>> explicitly requests the compiled-in default size, allowing the request
>> to be overwritten by automatic pool sizing.
>> 
>> Record whether swiotlb= supplied a numeric size and add
>> swiotlb_default_size_changed() to query that state.
>> 
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
>> ---
>>   kernel/dma/swiotlb.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>> 
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index aa19e81ae544..8f86deb25be2 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -107,6 +107,7 @@ static struct io_tlb_mem io_tlb_default_mem;
>>   
>>   static unsigned long default_nslabs = IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
>>   static unsigned long default_nareas;
>> +static bool swiotlb_size_set __initdata;
>
> Nit: there are various ways and reasons that the size is set in general; 
> could this please be named something clearer, e.g. 
> "swiotlb_cmdline_size_set"?
>
>>   /**
>>    * struct io_tlb_area - IO TLB memory area descriptor
>> @@ -269,6 +270,7 @@ setup_io_tlb_npages(char *str)
>>   		/* avoid tail segment of size < IO_TLB_SEGSIZE */
>>   		default_nslabs =
>>   			ALIGN(simple_strtoul(str, &str, 0), IO_TLB_SEGSIZE);
>> +		swiotlb_size_set = true;
>>   	}
>>   	if (*str == ',')
>>   		++str;
>> @@ -304,6 +306,11 @@ unsigned long swiotlb_size_or_default(void)
>>   	return default_nslabs << IO_TLB_SHIFT;
>>   }
>>   
>> +static bool __init swiotlb_default_size_changed(void)
>
> ...and then do we really need a trivial wrapper function with the exact 
> same scope as the variable itself? (Plus the name is similarly confusing 
> as the places this is used are themselves potentially changing the 
> default size, if you consider "default" to mean "not explicitly 
> specified"...)
>
> With the naming improved,
>

Updated and dropped the helper.

>
> Reviewed-by: Robin Murphy <robin.murphy@xxxxxxx>
>


-aneesh