Re: [PATCH 1/6] New Infiniband (OFED) tapset

["David J. Wilder" <dwilder at us dot ibm dot com>]

Thanks for the review Josh.
> 
> I'm working on bz5634 to support native type casting, so hopefully
> really soon it will be possible to write such functions as:
> 
>    function ip_qp__device:long (A:long)
>    { return @cast(A, "struct ib_qp")->device }
> 
> Not yet though...

Good to know, I will look forward to getting rid of my get_member mess.

> 
> > +/**
> > + * sfunction hexdump - Print a buffer in hex.
> > + *
> > + * Given the address and the length of a buffer, print its contents in hex. 
> > + */
> > +function hexdump (buff:long, len:long) {
> > +	printf("%s\n",_hexdump(buff, len))
> > +}
> 
> I think a stap-native printf("%.*M\n", 2*len, buff) will work like this.

Thanks, I will give it a try..

> ..
> 
> 
> > +probe ib_post_send = 	ib_ehca_post_send ?,
> > +			ib_mlx4_post_send ?,
> > +			ib_mthca_post_send ?,
> > +			ib_nes_post_send ? {}
> 
> Our convention is to use dots for separating probe namespaces, so this
> should be something like "ib.post_send".  Then for the internal
> variants, prefix it with an underscore like "_ib.ehca_post_send".
I will clean this up. 

What I wanted to do was place a probe on the inline ib_post_send()  (ib_
is just part of the name). But probing inlines has problems right now. 

My intention was for each user of the inline to sets a separate probe on
it's instance of ib_post_send() giving it a unique namespace.  Something
like: 
probe ib_ipoib.ib_post_send = module(ib_ipoib).inline(ib_post_send)
What you are seeing here is the results of working around the problem.

> > +		ib_post_send_qp = $qp
> > +		ib_post_send_wr = $send_wr
> 
> Since you're already in an ib-specific probe, I think these variables
> should be named more simply, like "qp" and "wr".
> 
> 
> Josh
>