On Wed, 11 Jun 2025 15:28:52 +0100 Robin Murphy <[email protected]> wrote:
> On 2025-06-11 3:01 pm, Steven Rostedt wrote: > > From: Steven Rostedt <[email protected]> > > > > 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/[email protected]/ > > > > 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. > > > 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
