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

Robin Murphy [off-list ref] writes:

On 21/09/2026 7:36 am, Aneesh Kumar K.V (Arm) wrote:

quoted

A default SWIOTLB pool used only for unaligned kmalloc bouncing can be

 [ ... 54 lines skipped ... ] 

quoted

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.

quoted

  	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