On 12/20/2018 12:02 AM, Ira Weiny wrote: > On Wed, Dec 19, 2018 at 09:59:56PM -0500, Hal Rosenstock wrote: >> Hi Ira, >> >> On 12/17/2018 2:15 PM, ira.weiny@xxxxxxxxx wrote: >>> From: Ira Weiny <ira.weiny@xxxxxxxxx> >>> >>> A while ago I wrote these patches for MAD stack tracing. At the time I was >>> proposing to remove the snoop interface.[1] >>> >>> For this submission I'd like to propose adding the tracing and leave the snoop >>> interface in. While I still don't see a need for the snoop interface, I'm no >>> longer advocating getting rid of it as the functionality of these patches is >>> different. >> >> I have a few questions on this patch series: >> >> Should the agent, umad, and SMP trace points also be checked for being >> enabled as the send and receive ones are ? > > The enable is only used in the send/recv because they need to call > trace_create_mad_addr to convert the address based on AH_TYPE. The other trace > points can use the "built in" trace enable/disable. For example the umad trace > points show up with the following enable files. > > /sys/kernel/debug/tracing/events/ib_umad/ib_umad_write/enable > /sys/kernel/debug/tracing/events/ib_umad/ib_umad_read/enable > /sys/kernel/debug/tracing/events/ib_umad/enable > > The top level enable above will enable both write and read in this case. > >> >> What is the overhead of the trace enabled check when the trace points >> are off ? > > Trace points are practically a no-op when disabled. We have used them in the > HFI1 driver fast path without detriment. > >> Are there MAD performance numbers with tracing off before and >> after these patches ? > > I don't have numbers for this. AFAIK there are no common benchmarks for MAD > stack performance. I have written some in the past but I don't know where that > code is at the moment. > >> >> Does this work with RMPP ? Was this tested with RMPP responses such as >> SA GetTableResp ? > > Define "work"? ;-) What I meant was how RMPP looks when trace is on. In addition to not displaying the RMPP fields, I guess impact is that TID would be replicated in the MAD RMPP sequence. > RMPP is not affected but is also not reported. And the traces at the lower > levels are showing the individual packet "MADs" where as at the umad level they > are showing the RMPP "MAD", if RMPP is done within the kernel. > > Additional RMPP features should probably be added at some point (like decoding > the rmpp header if present) but I just never got around to it because it > requires more processing of the packets (something I wanted to avoid). > > This does not break anything it just may not be as much information as we would > like WRT RMPP. But having this is better than nothing IMO. > >> >> Should GSI MAD trace points include the GRH if it's present ? > > That could be added for sure. As OPA does not use GRH a lot it was not high on > my list of priorities. > > We will probably want to add lots of things in the future, especially to other > areas of the core; SA, rdmacm, or verbs? > > This is one of the reasons I've backed away from removing the snoop feature. > Lets leave snoop in for now and get these in as a start. Sound ok? This seems reasonable to me. -- Hal > Ira > >> >> Thanks. >> >> -- Hal >> >>> In addition I wrote a sample eBPF which shows how one can further filter at the >>> tracepoints to distill the information being traced. I don't know if this >>> should be submitted through another tree or if it is ok to take though >>> linux-rdma? >>> >>> Ira >>> >>> [1] https://www.spinics.net/lists/linux-rdma/msg29109.html >>> >>> >>> Ira Weiny (6): >>> IB/MAD: Add send path trace points >>> IB/MAD: Add recv path trace point >>> IB/MAD: Add agent trace points >>> IB/UMAD: Add umad trace points >>> IB/MAD: Add SMP details to MAD tracing >>> BPF: Add sample code for new ib_umad tracepoint >>> >>> drivers/infiniband/core/mad.c | 95 ++++++++- >>> drivers/infiniband/core/user_mad.c | 7 + >>> include/trace/events/ib_mad.h | 391 +++++++++++++++++++++++++++++++++++++ >>> include/trace/events/ib_umad.h | 140 +++++++++++++ >>> samples/bpf/Makefile | 3 + >>> samples/bpf/ibumad_kern.c | 123 ++++++++++++ >>> samples/bpf/ibumad_user.c | 120 ++++++++++++ >>> 7 files changed, 878 insertions(+), 1 deletion(-) >>> create mode 100644 include/trace/events/ib_mad.h >>> create mode 100644 include/trace/events/ib_umad.h >>> create mode 100644 samples/bpf/ibumad_kern.c >>> create mode 100644 samples/bpf/ibumad_user.c >>> >
