linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Catalin Marinas <catalin.marinas@arm.com>,
	Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Cc: vbabka@kernel.org, harry.yoo@oracle.com,
	akpm@linux-foundation.org, hao.li@linux.dev, cl@gentwo.org,
	rientjes@google.com, roman.gushchin@linux.dev,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, stern@rowland.harvard.edu,
	linux@roeck-us.net, andy.shevchenko@gmail.com, hch@lst.de,
	Jeff.kirsher@gmail.com, Robin Murphy <robin.murphy@arm.com>
Subject: Re: [PATCH] mm/slab: align kmalloc to cacheline when DMA API debugging is active
Date: Fri, 27 Mar 2026 15:09:12 +0100	[thread overview]
Message-ID: <d2ed7315-72ff-43f3-bfaa-995025cb9419@samsung.com> (raw)
In-Reply-To: <acZ3ZUXhFHpSXzYS@arm.com>

Hi

On 27.03.2026 13:26, Catalin Marinas wrote:
> + Marek, Robin

Thanks for adding me to the loop.

> On Fri, Mar 27, 2026 at 10:58:46AM +0500, Mikhail Gavrilov wrote:
>> When CONFIG_DMA_API_DEBUG is enabled, the DMA debug infrastructure
>> tracks active mappings per cacheline and warns if two different DMA
>> mappings share the same cacheline ("cacheline tracking EEXIST,
>> overlapping mappings aren't supported").
>>
>> On x86_64, ARCH_KMALLOC_MINALIGN defaults to 8, so small kmalloc
>> allocations (e.g. the 8-byte hub->buffer and hub->status in the USB
>> hub driver) frequently land in the same 64-byte cacheline.  When both
>> are DMA-mapped, this triggers a false positive warning.
>>
>> This has been reported repeatedly since v5.14 (when the EEXIST check
>> was added) across various USB host controllers and devices including
>> xhci_hcd with USB hubs, USB audio devices, and USB ethernet adapters.
> This indeed has come up regularly in the past years.
>
>> +/*
>> + * Align memory allocations to cache lines if DMA API debugging is active
>> + * to avoid false positive DMA overlapping error messages.
>> + */
>> +#ifdef CONFIG_DMA_API_DEBUG
>> +#ifndef ARCH_KMALLOC_MINALIGN
>> +#define ARCH_KMALLOC_MINALIGN  L1_CACHE_BYTES
>> +#elif ARCH_KMALLOC_MINALIGN < L1_CACHE_BYTES
>> +#undef ARCH_KMALLOC_MINALIGN
>> +#define ARCH_KMALLOC_MINALIGN  L1_CACHE_BYTES
>> +#endif
>> +#endif
> TL;DR: I think this is fine:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> I'm not sure that's the best way to hide the warning but there
> are no great solutions either. On one hand, we want the DMA debug to
> capture potential problems on architectures it's not running on. OTOH,
> we also want to avoid false positives on coherent architectures/devices.
> I don't think reconciling the two requirements is easy.
>
> When DMA_API_DEBUG is enabled, the above will change the x86 behaviour
> that could have implications beyond DMA (e.g. may not catch some buffer
> overflow because it's within L1_CACHE_BYTES). Similarly for non-coherent
> architectures that select DMA_BOUNCE_UNALIGNED_KMALLOC (arm64 and riscv
> currently). arm64 defines ARCH_DMA_MINALIGN to 128 but
> ARCH_KMALLOC_MINALIGN to 8 (why 128 is larger than L1_CACHE_BYTES is
> another matter but let's ignore it for now).

IMHO enabling DMA_API_DEBUG should not change the kernel behavior, so I 
would prefer fixing this in DMA-debug code somehow.

> More of a thinking out loud, we have:
>
> 1. Coherent architectures - alignment doesn't matter
>
> 2. Non-coherent architectures with:
>     a) Sufficiently large ARCH_KMALLOC_MINALIGN
>     b) Small ARCH_KMALLOC_MINALIGN but DMA_BOUNCE_UNALIGNED_KMALLOC
>     c) Broken config - forgot to set ARCH_DMA_MINALIGN or bouncing
>
> We can ignore (2.c), the aim of the DMA debug is to catch wrong uses in
> drivers. If drivers is the only goal, the above change will do when
> running on (1) or (2.a) hardware - it will catch sub-L1_CACHE_BYTES
> buffers from drivers while assuming kmalloc() machinery is safe.
> However, if running on (2.b) it won't catch anything that may be
> problematic on (2.a) since the DMA debug ignores the overlap.
>
> We could make DMA_BOUNCE_UNALIGNED_KMALLOC dependent on !DMA_API_DEBUG
> but it would be nice to be able to sanity-check the bouncing logic.
> Well, it wasn't checking it before and with commit 03521c892bb8
> ("dma-debug: don't report false positives with
> DMA_BOUNCE_UNALIGNED_KMALLOC"), we made this clear that overlapping will
> be ignored.
>
> Irrespective of whether we disable bouncing with DMA_API_DEBUG, maybe we
> could replace the above commit with:
>
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 3928a509c44c..488045ef6245 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -175,7 +175,7 @@ dma_addr_t dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
>   	if (!is_mmio)
>   		kmsan_handle_dma(phys, size, dir);
>   	trace_dma_map_phys(dev, phys, addr, size, dir, attrs);
> -	debug_dma_map_phys(dev, phys, size, dir, addr, attrs);
> +	debug_dma_map_phys(dev, dma_to_phys(addr), size, dir, addr, attrs);
>
>   	return addr;
>   }
>
> Anyway, this I think is unrelated to the proposed change affecting x86,
> more of a how to make the DMA API debugging more useful when running on
> arm64 or riscv.

This is not enough, there is also a dma_map_sg_attrs() path.

I've reverted 03521c892bb8 and added the following change:

diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c index 
55e7ca8ceb86..bbada41143ea 100644 --- a/kernel/dma/debug.c +++ 
b/kernel/dma/debug.c @@ -18,6 +18,7 @@ #include <linux/uaccess.h> 
#include <linux/export.h> #include <linux/device.h> +#include 
<linux/dma-direct.h> #include <linux/types.h> #include <linux/sched.h> 
#include <linux/ctype.h> @@ -1241,7 +1242,8 @@ void 
debug_dma_map_phys(struct device *dev, phys_addr_t phys, size_t size, 
entry->dev = dev; entry->type = dma_debug_phy; - entry->paddr = phys; + 
entry->paddr = IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) ? + 
dma_to_phys(dev, dma_addr) : phys; entry->dev_addr = dma_addr; 
entry->size = size; entry->direction = direction; @@ -1335,7 +1337,9 @@ 
void debug_dma_map_sg(struct device *dev, struct scatterlist *sg, 
entry->type = dma_debug_sg; entry->dev = dev; - entry->paddr = 
sg_phys(s); + entry->paddr = + 
IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) ? + dma_to_phys(dev, 
sg_dma_address(s)) : sg_phys(s); entry->size = sg_dma_len(s); 
entry->dev_addr = sg_dma_address(s); entry->direction = direction;

thenran my tests on ARM64 and RV64 boards. Only one new warning has been 
reported (I didn't analyze it yet), so this might be indeed a better 
solution than skipping overlapping cache lines warnings when 
DMA_BOUNCE_UNALIGNED_KMALLOC is set.

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



  parent reply	other threads:[~2026-03-27 14:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-27  5:58 Mikhail Gavrilov
2026-03-27  6:37 ` Harry Yoo (Oracle)
2026-03-27  6:50   ` Mikhail Gavrilov
2026-03-27  8:00     ` Harry Yoo (Oracle)
2026-03-27  8:07       ` Mikhail Gavrilov
2026-03-27  8:43         ` Harry Yoo (Oracle)
2026-03-27 10:25           ` Mikhail Gavrilov
2026-03-27 10:39             ` Harry Yoo (Oracle)
2026-03-27  6:41 ` Guenter Roeck
2026-03-27 12:26 ` Catalin Marinas
2026-03-27 12:34   ` Andy Shevchenko
2026-03-27 14:09   ` Marek Szyprowski [this message]
2026-03-27 14:30     ` Vlastimil Babka (SUSE)
2026-03-27 14:37       ` Mikhail Gavrilov
2026-03-27 14:41         ` Marek Szyprowski
2026-03-27 14:55     ` Marek Szyprowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2ed7315-72ff-43f3-bfaa-995025cb9419@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=Jeff.kirsher@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=cl@gentwo.org \
    --cc=hao.li@linux.dev \
    --cc=harry.yoo@oracle.com \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mikhail.v.gavrilov@gmail.com \
    --cc=rientjes@google.com \
    --cc=robin.murphy@arm.com \
    --cc=roman.gushchin@linux.dev \
    --cc=stern@rowland.harvard.edu \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox