[RFC] dma/mapping.c: WARN_ONCE on dma_addressing_limited() being true

[RFC] dma/mapping.c: WARN_ONCE on dma_addressing_limited() being true

[Balbir Singh]

In the debug and resolution of an issue involving forced use of bounce
buffers, 7170130e4c72 ("x86/mm/init: Handle the special case of device
private pages in add_pages(), to not increase max_pfn and trigger
dma_addressing_limited() bounce buffers"). It would have been easier
to debug the issue if dma_addressing_limited() had a warning about a
device not being able to address all of memory and thus forcing all
accesses through a bounce buffer. Please see[2].

A warning would have let the user of the system know that in their
particular case, use_dma32 is set due to the addressing limitation
and this would impact performance of the driver in use.

Implement a WARN_ONCE() to point to the potential use of bounce buffers
when we hit the condition. When swiotlb is used,
dma_addressing_limited() is used to determine the size of maximum dma
buffer size in dma_direct_max_mapping_size(). The warning could be
triggered in that check as well.

Link: https://lore.kernel.org/lkml/20250401000752.249348-1-balbirs@nvidia.com/ [1]
Link: https://lore.kernel.org/lkml/20250310112206.4168-1-spasswolf@web.de/ [2]

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Kees Cook <kees@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Bert Karwatzki <spasswolf@web.de>

Signed-off-by: Balbir Singh <balbirs@nvidia.com>
---
Testing: Tested lightly with on a virtual machine, I do not have access
to a device where dma_addressing_limited() is true


 kernel/dma/mapping.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index cda127027e48..0f0455fa5bc8 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -918,7 +918,7 @@ EXPORT_SYMBOL(dma_set_coherent_mask);
  * the system, else %false.  Lack of addressing bits is the prime reason for
  * bounce buffering, but might not be the only one.
  */
-bool dma_addressing_limited(struct device *dev)
+static bool __dma_addressing_limited(struct device *dev)
 {
 	const struct dma_map_ops *ops = get_dma_ops(dev);
 
@@ -930,6 +930,17 @@ bool dma_addressing_limited(struct device *dev)
 		return false;
 	return !dma_direct_all_ram_mapped(dev);
 }
+
+bool dma_addressing_limited(struct device *dev)
+{
+	bool ret = __dma_addressing_limited(dev);
+
+	WARN_ONCE((ret == true),
+		"%s might have lower performance due to bounce buffering",
+		dev_name(dev));
+
+	return ret;
+}
 EXPORT_SYMBOL_GPL(dma_addressing_limited);
 
 size_t dma_max_mapping_size(struct device *dev)
-- 
2.49.0

Re: [RFC] dma/mapping.c: WARN_ONCE on dma_addressing_limited() being true

[Christoph Hellwig]

On Sat, Apr 12, 2025 at 07:41:10PM +1000, Balbir Singh wrote:
> In the debug and resolution of an issue involving forced use of bounce
> buffers, 7170130e4c72 ("x86/mm/init: Handle the special case of device
> private pages in add_pages(), to not increase max_pfn and trigger
> dma_addressing_limited() bounce buffers"). It would have been easier
> to debug the issue if dma_addressing_limited() had a warning about a
> device not being able to address all of memory and thus forcing all
> accesses through a bounce buffer. Please see[2].
> 
> A warning would have let the user of the system know that in their
> particular case, use_dma32 is set due to the addressing limitation
> and this would impact performance of the driver in use.
> 
> Implement a WARN_ONCE() to point to the potential use of bounce buffers
> when we hit the condition. When swiotlb is used,
> dma_addressing_limited() is used to determine the size of maximum dma
> buffer size in dma_direct_max_mapping_size(). The warning could be
> triggered in that check as well.

dma_addressing_limited is a perfectly expected condition, and returns
true for many devices and still plenty system configuation.  A kernel
warning with a stacktrace is not acceptable for that.  A simple one-line
dev_info might be ok, but could still be rather spammy on many systems.

Re: [RFC] dma/mapping.c: WARN_ONCE on dma_addressing_limited() being true

[Balbir Singh]

On 4/14/25 15:54, Christoph Hellwig wrote:
> On Sat, Apr 12, 2025 at 07:41:10PM +1000, Balbir Singh wrote:
>> In the debug and resolution of an issue involving forced use of bounce
>> buffers, 7170130e4c72 ("x86/mm/init: Handle the special case of device
>> private pages in add_pages(), to not increase max_pfn and trigger
>> dma_addressing_limited() bounce buffers"). It would have been easier
>> to debug the issue if dma_addressing_limited() had a warning about a
>> device not being able to address all of memory and thus forcing all
>> accesses through a bounce buffer. Please see[2].
>>
>> A warning would have let the user of the system know that in their
>> particular case, use_dma32 is set due to the addressing limitation
>> and this would impact performance of the driver in use.
>>
>> Implement a WARN_ONCE() to point to the potential use of bounce buffers
>> when we hit the condition. When swiotlb is used,
>> dma_addressing_limited() is used to determine the size of maximum dma
>> buffer size in dma_direct_max_mapping_size(). The warning could be
>> triggered in that check as well.
> 
> dma_addressing_limited is a perfectly expected condition, and returns
> true for many devices and still plenty system configuation.  A kernel
> warning with a stacktrace is not acceptable for that.  A simple one-line
> dev_info might be ok, but could still be rather spammy on many systems.
> 

Thanks for the review!

I'll convert it to a dev_info(). We can remove it, if it causes confusion
or users complain about it?

Balbir

Re: [RFC] dma/mapping.c: WARN_ONCE on dma_addressing_limited() being true

[Christian König]

Am 14.04.25 um 10:25 schrieb Balbir Singh:
> On 4/14/25 15:54, Christoph Hellwig wrote:
>> On Sat, Apr 12, 2025 at 07:41:10PM +1000, Balbir Singh wrote:
>>> In the debug and resolution of an issue involving forced use of bounce
>>> buffers, 7170130e4c72 ("x86/mm/init: Handle the special case of device
>>> private pages in add_pages(), to not increase max_pfn and trigger
>>> dma_addressing_limited() bounce buffers"). It would have been easier
>>> to debug the issue if dma_addressing_limited() had a warning about a
>>> device not being able to address all of memory and thus forcing all
>>> accesses through a bounce buffer. Please see[2].
>>>
>>> A warning would have let the user of the system know that in their
>>> particular case, use_dma32 is set due to the addressing limitation
>>> and this would impact performance of the driver in use.
>>>
>>> Implement a WARN_ONCE() to point to the potential use of bounce buffers
>>> when we hit the condition. When swiotlb is used,
>>> dma_addressing_limited() is used to determine the size of maximum dma
>>> buffer size in dma_direct_max_mapping_size(). The warning could be
>>> triggered in that check as well.
>> dma_addressing_limited is a perfectly expected condition, and returns
>> true for many devices and still plenty system configuation.  A kernel
>> warning with a stacktrace is not acceptable for that.  A simple one-line
>> dev_info might be ok, but could still be rather spammy on many systems.
>>
> Thanks for the review!
>
> I'll convert it to a dev_info(). We can remove it, if it causes confusion
> or users complain about it?

I would even say that this should be only debugging level.

As Christoph explained it is perfectly normal for device to not be able to address everything in the system. So even an info print sounds like to much.

But I totally agree that it is interesting for debugging, that issue was really hard to nail down.

Regards,
Christian.

>
> Balbir

Re: [RFC] dma/mapping.c: WARN_ONCE on dma_addressing_limited() being true

[Marek Szyprowski]

On 14.04.2025 11:45, Christian König wrote:
> Am 14.04.25 um 10:25 schrieb Balbir Singh:
>> On 4/14/25 15:54, Christoph Hellwig wrote:
>>> On Sat, Apr 12, 2025 at 07:41:10PM +1000, Balbir Singh wrote:
>>>> In the debug and resolution of an issue involving forced use of bounce
>>>> buffers, 7170130e4c72 ("x86/mm/init: Handle the special case of device
>>>> private pages in add_pages(), to not increase max_pfn and trigger
>>>> dma_addressing_limited() bounce buffers"). It would have been easier
>>>> to debug the issue if dma_addressing_limited() had a warning about a
>>>> device not being able to address all of memory and thus forcing all
>>>> accesses through a bounce buffer. Please see[2].
>>>>
>>>> A warning would have let the user of the system know that in their
>>>> particular case, use_dma32 is set due to the addressing limitation
>>>> and this would impact performance of the driver in use.
>>>>
>>>> Implement a WARN_ONCE() to point to the potential use of bounce buffers
>>>> when we hit the condition. When swiotlb is used,
>>>> dma_addressing_limited() is used to determine the size of maximum dma
>>>> buffer size in dma_direct_max_mapping_size(). The warning could be
>>>> triggered in that check as well.
>>> dma_addressing_limited is a perfectly expected condition, and returns
>>> true for many devices and still plenty system configuation.  A kernel
>>> warning with a stacktrace is not acceptable for that.  A simple one-line
>>> dev_info might be ok, but could still be rather spammy on many systems.
>>>
>> Thanks for the review!
>>
>> I'll convert it to a dev_info(). We can remove it, if it causes confusion
>> or users complain about it?
> I would even say that this should be only debugging level.
>
> As Christoph explained it is perfectly normal for device to not be able to address everything in the system. So even an info print sounds like to much.
>
> But I totally agree that it is interesting for debugging, that issue was really hard to nail down.

Right, please use dev_debug().

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland