Re: [PATCH v5 3/6] dma: swiotlb: Centralize minimal pool sizing

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Robin Murphy <robin.murphy@arm.com> writes:

> On 21/09/2026 7:36 am, Aneesh Kumar K.V (Arm) wrote:
>> A default SWIOTLB pool used only for unaligned kmalloc bouncing can be

 [ ... 54 lines skipped ... ] 

>>   
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index 8f86deb25be2..f368a73f4ed0 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -483,9 +483,18 @@ static bool __init swiotlb_kmalloc_needs_bounce(void)
>>   static void __init
>>   swiotlb_adjust_pool_size(enum swiotlb_pool_policy policy)
>>   {
>> +	if (swiotlb_default_size_changed())
>> +		return;
>> +
>
> This appears to be entirely redundant, as ultimately the point of this 
> function is to call swiotlb_adjust_size() (if it does anything at all), 
> and the first thing that does is this same exact check. We hardly need 
> to micro-optimise short-circuiting a handful of arithmetic in a one-off 
> setup path, and it's convoluted enough as it is, so please try to avoid 
> redundancy that makes it even more confusing to follow.
>

OK, I'll drop this.

>
>>   	switch (policy) {
>> -	case SWIOTLB_POOL_MINIMAL:
>> +	case SWIOTLB_POOL_MINIMAL: {
>> +		unsigned long size;
>> +
>> +		/* Use 1MB per 1GB of RAM for kmalloc() bouncing. */
>> +		size = DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
>> +		swiotlb_adjust_size(min(swiotlb_size_or_default(), size));
>>   		break;
>
> Similarly I think it would be clearer if we had a common 
> swiotlb_adjust_size() call at the end of the function, and then either 
> calculate a size or return early in each switch case as appropriate.
>
> Furthermore, swiotlb_size_or_default() is awful IMO - and in fact after 
> this series we could perhaps clean it up entirely by making the size 
> implicit in swiotlb_init_late() - not to mention misleadingly redundant. 
> I'd say just open-code "default_nslabs << IO_TLB_SHIFT" like elsewhere 
> in the file, but in fact it may as well just be IO_TLB_DEFAULT_SIZE 
> (think about it...)
>

How about we rename swiotlb_size_or_default to

unsigned long swiotlb_default_pool_size(void)
{
	return default_nslabs << IO_TLB_SHIFT;
}

We still need a helper because arch/arm/xen/mm.c also uses it. I also
updated swiotlb_adjust_pool_size() as suggested.

static void __init
swiotlb_adjust_pool_size(enum swiotlb_pool_policy policy)
{
	unsigned long size;

	switch (policy) {
	case SWIOTLB_POOL_MINIMAL:
		/* Use 1MB per 1GB of RAM for kmalloc() bouncing. */
		size = DIV_ROUND_UP(memblock_phys_mem_size(), 1024);
		size = min(swiotlb_size_or_default(), size);
		break;
	case SWIOTLB_POOL_CC_GUEST:
		size = swiotlb_adjusted_size();
		break;
	case SWIOTLB_POOL_NONE:
		WARN(true, "Cannot adjust SWIOTLB size without a pool\n");
		return;
	case SWIOTLB_POOL_DEFAULT:
	default:
		return;
	}

	swiotlb_adjust_size(size);
}


-aneesh