2024-01-08 14:00:38

by zhangpeng (AS)

Subject: [PATCH] swiotlb: check alloc_size before the allocation of a new memory pool

From: ZhangPeng <[email protected]>

The allocation request for swiotlb contiguous memory greater than
128*2KB cannot be fulfilled because it exceeds the maximum contiguous
memory limit. If the swiotlb memory we allocate is larger than 128*2KB,
swiotlb_find_slots() will still schedule the allocation of a new memory
pool, which will increase memory overhead.

Fix it by adding a check with alloc_size no more than 128*2KB before
scheduling the allocation of a new memory pool in swiotlb_find_slots().

Signed-off-by: ZhangPeng <[email protected]>
---
kernel/dma/swiotlb.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 33d942615be5..cc92cff02c60 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -1126,6 +1126,9 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
u64 phys_limit;
int index;

+ if (alloc_size > IO_TLB_SEGSIZE * IO_TLB_SIZE)
+ return -1;
+
rcu_read_lock();
list_for_each_entry_rcu(pool, &mem->pools, node) {
index = swiotlb_pool_find_slots(dev, pool, orig_addr,
--
2.25.1

2024-01-08 15:50:19

by Petr Tesarik

Subject: Re: [PATCH] swiotlb: check alloc_size before the allocation of a new memory pool

On 1/8/2024 3:00 PM, Peng Zhang wrote:
> From: ZhangPeng <[email protected]>
>
> The allocation request for swiotlb contiguous memory greater than
> 128*2KB cannot be fulfilled because it exceeds the maximum contiguous
> memory limit. If the swiotlb memory we allocate is larger than 128*2KB,
> swiotlb_find_slots() will still schedule the allocation of a new memory
> pool, which will increase memory overhead.
>
> Fix it by adding a check with alloc_size no more than 128*2KB before
> scheduling the allocation of a new memory pool in swiotlb_find_slots().
>
> Signed-off-by: ZhangPeng <[email protected]>
> ---
> kernel/dma/swiotlb.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 33d942615be5..cc92cff02c60 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -1126,6 +1126,9 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
> u64 phys_limit;
> int index;
>
> + if (alloc_size > IO_TLB_SEGSIZE * IO_TLB_SIZE)
> + return -1;
> +
> rcu_read_lock();
> list_for_each_entry_rcu(pool, &mem->pools, node) {
> index = swiotlb_pool_find_slots(dev, pool, orig_addr,

IIUC this such big allocations are not normally required by drivers, but
I have already run into a similar issue with a Raspberry Pi 4 dma-buf
object, so they can be triggered at will by user space. I also believe
this sanity check is a good idea in general, not only when dynamic
SWIOTLB is enabled.

Reviewed-by: Petr Tesarik <[email protected]>

Petr T

2024-01-08 17:07:06

by Christoph Hellwig

Subject: Re: [PATCH] swiotlb: check alloc_size before the allocation of a new memory pool

This looks good, but the surrounding code changed quite a bit in
the dma-mapping code. Can you redo it against dma-mapping for-next
(which should get merged into Linus' tree ASAP as I already sent the
pull request)

2024-01-09 02:34:32

by zhangpeng (AS)

Subject: Re: [PATCH] swiotlb: check alloc_size before the allocation of a new memory pool

On 2024/1/8 23:46, Petr Tesarik wrote:

> On 1/8/2024 3:00 PM, Peng Zhang wrote:
>> From: ZhangPeng <[email protected]>
>>
>> The allocation request for swiotlb contiguous memory greater than
>> 128*2KB cannot be fulfilled because it exceeds the maximum contiguous
>> memory limit. If the swiotlb memory we allocate is larger than 128*2KB,
>> swiotlb_find_slots() will still schedule the allocation of a new memory
>> pool, which will increase memory overhead.
>>
>> Fix it by adding a check with alloc_size no more than 128*2KB before
>> scheduling the allocation of a new memory pool in swiotlb_find_slots().
>>
>> Signed-off-by: ZhangPeng <[email protected]>
>> ---
>> kernel/dma/swiotlb.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index 33d942615be5..cc92cff02c60 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -1126,6 +1126,9 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
>> u64 phys_limit;
>> int index;
>>
>> + if (alloc_size > IO_TLB_SEGSIZE * IO_TLB_SIZE)
>> + return -1;
>> +
>> rcu_read_lock();
>> list_for_each_entry_rcu(pool, &mem->pools, node) {
>> index = swiotlb_pool_find_slots(dev, pool, orig_addr,
> IIUC this such big allocations are not normally required by drivers, but
> I have already run into a similar issue with a Raspberry Pi 4 dma-buf
> object, so they can be triggered at will by user space. I also believe
> this sanity check is a good idea in general, not only when dynamic
> SWIOTLB is enabled.
>
> Reviewed-by: Petr Tesarik <[email protected]>
>
> Petr T

Thanks for your review!

--
Best Regards,
Peng

2024-01-09 02:34:47

by zhangpeng (AS)

Subject: Re: [PATCH] swiotlb: check alloc_size before the allocation of a new memory pool

On 2024/1/9 1:06, Christoph Hellwig wrote:

> This looks good, but the surrounding code changed quite a bit in
> the dma-mapping code. Can you redo it against dma-mapping for-next
> (which should get merged into Linus' tree ASAP as I already sent the
> pull request)

Of course, I'll rebase to the for-next branch in the next version. Thanks!

--
Best Regards,
Peng