Re: [PATCH v4] IB/core: Trace points for diagnosing completion queue issues

[Chuck Lever <chuck.lever@oracle.com>]

> On Nov 6, 2019, at 3:17 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> On Sat, Oct 12, 2019 at 03:42:56PM -0400, Chuck Lever wrote:
>> +static int __ib_poll_cq(struct ib_cq *cq, int num_entries, struct ib_wc *wc)
>> +{
>> +	int rc;
>> +
>> +	rc = ib_poll_cq(cq, num_entries, wc);
>> +	trace_cq_poll(cq, num_entries, rc);
>> +	return rc;
>> +}
> 
> Why not put the trace point in ib_poll_cq directly?

in rdma/ib_verbs.h:

3876 static inline int ib_poll_cq(struct ib_cq *cq, int num_entries,
3877                              struct ib_wc *wc)
3878 {
3879         return cq->device->ops.poll_cq(cq, num_entries, wc);
3880 }

The trace point would have to go in every driver's ->poll_cq.

Putting #include <trace/events/rdma_core.h> in a header could
be a problem. I've found it almost never works well, due to the
extra stuff that is pulled into every source file that would
include rdma/ib_verbs.h.

Note that the use of an indirect call here is also challenging
for adding a kprobe in this path (ie, for eBPF). Maybe a better
approach would be to move ib_poll_cq to drivers/infiniband/core/cq.c
and then add the trace point there?


> What is the overhead of these things if you don't use them?

IIUC, one conditional branch.


--
Chuck Lever