This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH 1/6] New Infiniband (OFED) tapset
- From: Josh Stone <jistone at redhat dot com>
- To: "David J. Wilder" <dwilder at us dot ibm dot com>
- Cc: systemtap at sourceware dot org, xma at us dot ibm dot com, pradeep at us dot ibm dot com, prasad at linux dot vnet dot ibm dot com
- Date: Thu, 05 Feb 2009 19:41:45 -0800
- Subject: Re: [PATCH 1/6] New Infiniband (OFED) tapset
- References: <1233880153.23376.35.camel@wilder.ibm.com>
Hi -
David J. Wilder wrote:
This tapset is the first of several tapsets used to probe the ofed
infiniband stack. Ofed.stp includes probes to several of the ib
"kernel" verbs. Also included are core functions that users of the ofed
tapescripts may find helpful
I know little about infiniband, so I'm mostly peeking through here for
safety issues and adherence to conventions...
+%{
+
+#define get_memberfault(STRUCT, MEMBER) \
+ goto end; \
+deref_fault: \
+ printk("deref fault in ofed.stp, base pointer = %p\n", \
+ (struct STRUCT *)THIS->A); \
+ THIS->__retvalue = -1; \
+end: ;
The kread will prepare an error string for the normal stap error
channels (c->last_error), so you don't need another message in printk.
You can also use CATCH_DEREF_FAULT() to provide the deref_fault label,
unless you really want the retvalue set to -1.
+#define __get_member(STRUCT,MEMBER) kread(&((struct STRUCT *)THIS->A)->MEMBER);
I think you need to cast with (struct STRUCT *)(long) for 32-bit platforms.
+#define _get_member(STRUCT,MEMBER) __get_member(STRUCT,MEMBER); get_memberfault(STRUCT,MEMBER);
+#define get_member(STRUCT,MEMBER,TYPE) (long)(TYPE)_get_member(STRUCT, MEMBER);
+%}
The kread will cast the value to typeof(STRUCT->MEMBER), so do you
really need the manual TYPE cast?
+function ib_qp__device:long (A:long)
+ %{ THIS->__retvalue = get_member(ib_qp, device, void *)%}
Side-effect-free embedded-c like this should have a comment "/* pure */"
so our optimizer knows that it may be possible to remove it.
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...
+/**
+ * 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...
+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".
+ 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