Robin Murphy <[email protected]> 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
