Re: [PATCH] swiotlb: iommu/dma: Move trace_swiotlb_bounced() into swiotlb_tbl_map_single()
From: Robin Murphy <robin.murphy@arm.com>
Date: 2025-06-11 14:28:56
Also in:
linux-iommu, lkml
On 2025-06-11 3:01 pm, Steven Rostedt wrote:
From: Steven Rostedt <rostedt@goodmis.org> I'm working on code that will warn when a tracepoint is defined but not used. As the TRACE_EVENT() logic still creates all the code regardless if something calls the trace_<event>() function. It wastes around 5K per trace event (less for tracepoints). But it seems that the code in drivers/iommu/dma-iommu.c does the opposite. It calls the trace_swiotlb_bounced() tracepoint without it being defined. The tracepoint is defined in kernel/dma/swiotlb.c when CONFIG_SWIOTLB is defined, but this code exists when that config is not defined. This now fails with my work because I have all the callers reference the tracepoint that they will call. Thanks to the kernel test robot, it found this: https://lore.kernel.org/all/202506091015.7zd87kI7-lkp@intel.com/ (local) Instead of calling trace_swiotlb_bounced() from drivers/iommu/dma-iommu.c where it is useless when CONFIG_SWIOTLB is not defined, move the tracepoint into swiotlb_tbl_map_single(). This also makes it consistent with which memory is being traced (physical as supposed to dma address).
Consistent with what? Certainly not the other callers which invoke the tracepoint with phys_to_dma(orig_addr), but will now do so twice with potentially different values. Arguably iommu-dma is already consistent as things stand, since it does not support static address offsets underneath IOMMU translation, and therefore orig_addr will always be equal to phys_to_dma(orig_addr) anyway. The point of interest really is in those other cases, where if there *were* a static DMA offset then phys_to_dma(orig_addr) would actually be a pretty meaningless value which is not used anywhere else and therefore not overly helpful to trace - neither the recognisable PA of the underlying memory itself, nor the actual DMA address which will be used subsequently, since at this point that's yet to be allocated.
Fixes: ed18a46262be4 ("iommu/dma: Factor out a iommu_dma_map_swiotlb helper")
It's a fair bit older than that, try a63c357b9fd5 ("iommu/dma: Trace
bounce buffer usage when mapping buffers") - looks like it's always just
been hopeful of being DCE'd.
Thanks,
Robin.
quoted hunk ↗ jump to hunk
Closes: https://lore.kernel.org/all/20250610202457.5a599336@gandalf.local.home/ (local) Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> --- drivers/iommu/dma-iommu.c | 2 -- kernel/dma/swiotlb.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-)diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index ea2ef53bd4fe..1935b360cc94 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c@@ -1153,8 +1153,6 @@ static phys_addr_t iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys, return (phys_addr_t)DMA_MAPPING_ERROR; } - trace_swiotlb_bounced(dev, phys, size); - phys = swiotlb_tbl_map_single(dev, phys, size, iova_mask(iovad), dir, attrs);diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index abcf3fa63a56..c112f1d98861 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c@@ -1379,6 +1379,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, phys_addr_t tlb_addr; unsigned short pad_slots; + trace_swiotlb_bounced(dev, orig_addr, mapping_size); + if (!mem || !mem->nslabs) { dev_warn_ratelimited(dev, "Can not allocate SWIOTLB buffer earlier and can't now provide you with the DMA bounce buffer");