On Wed, 11 Jun 2025 15:28:52 +0100
Robin Murphy [off-list ref] wrote:

On 2025-06-11 3:01 pm, Steven Rostedt wrote:

quoted

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.

I only added that line because of what Christoph said. I'll let you
folks hammer that point.

quoted

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.

Ah, yeah, that's the real culprit. Thanks!

Well, it may be some time before I push my code that will cause this to
fail to build, as I want to clean up the unused events before I add it
(otherwise there's going to be lots of warnings whenever it's enabled).

-- Steve