On 11/29/2017 09:32 AM, Alex Williamson wrote:

On Thu, 23 Nov 2017 15:13:37 +1100
Alexey Kardashevskiy [off-list ref] wrote:

quoted

On 17/11/17 17:58, Alexey Kardashevskiy wrote:

quoted

On 17/11/17 11:13, Alex Williamson wrote:  

quoted

On Tue, 14 Nov 2017 10:47:12 +1100
Alexey Kardashevskiy [off-list ref] wrote:
 

quoted

On 27/10/17 14:00, Alexey Kardashevskiy wrote:  

quoted

This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already
uses these via the IOMMU API (iommu_map/__iommu_unmap).

Signed-off-by: Alexey Kardashevskiy <redacted>    

Is this really legitimate to include tracepoints from a different  
subsystem?>  The vfio type1 backend gets these trace points by virtue of  
it actually using the IOMMU API, it doesn't call them itself.  I'm kind
of surprised these are actually available to be called from a module.  

They are explicitly exported:

EXPORT_TRACEPOINT_SYMBOL_GPL(map);
EXPORT_TRACEPOINT_SYMBOL_GPL(unmap);

Tracepoints need to be exported as per the tracepoint sub-system
requirements.

quoted

quoted

I would think this is for things like drivers/vfio/vfio_iommu_spapr_tce.c ,
why else?...

  

quoted

I suspect the way to do this is probably to define our own tracepoints
in the vfio/spapr backend or insert tracepoints into the IOMMU layers
that that code calls into rather than masquerading as tracepoints from
a different subsystem.  Right?  

This makes sense too. But it is going to be just cut-n-paste and some
confusion -
/sys/kernel/debug/tracing/events/iommu/map will still be present but
won't work and
/sys/kernel/debug/tracing/events/vfio/vfio_iommu_spapr_tce/map will.

But iommu/map does work, it's just not called by the vfio spapr tce
backend, which is absolutely correct.  In fact, that's part of my
reservation about this approach is that it could be interpreted as
implying a call path that doesn't exist on this arch.

These tracepoints are intended to be called after a iommo/map/unmap
calls from IOMMU API. It doesn't make any sense to call them from
random places. It is totally useless to do so and makes it difficult
to debug problems.

As an author of these tracepoints - that is my opinion.

thanks,
-- Shuah