<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<b style="font-style: inherit; font-variant-ligatures: inherit; font-variant-caps: inherit; font-size: 11pt; font-family: Calibri, sans-serif;">From:</b><span style="color: rgb(0, 0, 0); font-size: 11pt; font-family: Calibri, sans-serif;"> dtrace-devel-bounces@oss.oracle.com
<dtrace-devel-bounces@oss.oracle.com> on behalf of Kris Van Hees <kris.van.hees@oracle.com></span><br>
</div>
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>Sent:</b> Saturday, January 16, 2021 12:21 AM<br>
<b>To:</b> dtrace-devel@oss.oracle.com <dtrace-devel@oss.oracle.com><br>
<b>Subject:</b> [DTrace-devel] [PATCH] Record clause id in mstate</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">
<blockquote style="margin-top:0;margin-bottom:0">Support for the ERROR probe requires a per-probe unique id to identify<br>
the clause the ERROR probe was triggered from. Add a clid member to<br>
<div>the machine state, and initialize it in the prologue.<br>
</div>
<div><br>
</div>
<div>diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h</div>
@@ -20,7 +20,8 @@ extern "C" {<br>
<br>
#define DT_CONST_EPID 1<br>
#define DT_CONST_ARGC 2<br>
-#define DT_CONST_PRID 3<br>
+#define DT_CONST_CLID 3<br>
+#define DT_CONST_PRID 4<br>
<div><br>
</div>
</blockquote>
<div>I'm curious what dictates the ordering here? Why insert rather than append? Why not a more consistent ordering of these things -- e.g., here, dt_dlibs.c, in MST, etc. Even the DT_IDENT_SCALAR case in dt_link_construct(). Not a big deal, but uniformity
would seem nice.</div>
<blockquote style="margin-top:0;margin-bottom:0">
<div> <br>
</div>
<div>diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c</div>
@@ -2324,7 +2324,8 @@ dt_link_layout(dtrace_hdl_t *dtp, const dtrace_difo_t *dp, uint_t *pcp,<br>
static int<br>
dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,<br>
const dtrace_difo_t *sdp, dt_strtab_t *stab,<br>
- uint_t *pcp, uint_t *rcp, uint_t *vcp, dtrace_epid_t epid)<br>
+ uint_t *pcp, uint_t *rcp, uint_t *vcp, dtrace_epid_t epid,<br>
+ uint_t clid)<br>
{<br>
uint_t pc = *pcp;<br>
uint_t rc = *rcp;<br>
@@ -2412,6 +2413,9 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,<br>
case DT_CONST_PRID:<br>
nrp->dofr_data = prp->desc->id;<br>
break;<br>
+ case DT_CONST_CLID:<br>
+ nrp->dofr_data = clid;<br>
+ break;<br>
case DT_CONST_ARGC:<br>
nrp->dofr_data = 0; /* FIXME */<br>
break;<br>
@@ -2427,13 +2431,14 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,<br>
rdp = dt_dlib_get_func_difo(dtp, idp);<br>
if (rdp == NULL)<br>
return -1;<br>
- if (rdp->dtdo_ddesc != NULL)<br>
+ if (rdp->dtdo_ddesc != NULL) {<br>
nepid = dt_epid_add(dtp, rdp->dtdo_ddesc,<br>
prp->desc->id);<br>
- else<br>
+ clid++;<br>
+ } else<br>
nepid = 0;<br>
ipc = dt_link_construct(dtp, prp, dp, rdp, stab,<br>
- pcp, rcp, vcp, nepid);<br>
+ pcp, rcp, vcp, nepid, clid);<br>
if (ipc == -1)<br>
return -1;<br>
<br>
@@ -2543,7 +2548,8 @@ dt_link(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp)<br>
if (stab == NULL)<br>
goto nomem;<br>
<br>
- rc = dt_link_construct(dtp, prp, fdp, dp, stab, &insc, &relc, &varc, 0);<br>
+ rc = dt_link_construct(dtp, prp, fdp, dp, stab, &insc, &relc, &varc,<br>
+ 0, 0);<br>
dt_dlib_reset(dtp, B_FALSE);<br>
if (rc == -1)<br>
goto fail;<br>
<div>diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c<br>
</div>
<br>
@@ -263,6 +263,7 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)<br>
dt_irlist_t *dlp = &pcb->pcb_ir;<br>
dt_ident_t *epid = dt_dlib_get_var(pcb->pcb_hdl, "EPID");<br>
dt_ident_t *prid = dt_dlib_get_var(pcb->pcb_hdl, "PRID");<br>
+ dt_ident_t *clid = dt_dlib_get_var(pcb->pcb_hdl, "CLID");<br>
<br>
assert(epid != NULL);<br>
<div> assert(prid != NULL);<br>
</div>
</blockquote>
<div><br>
</div>
<div>I cannot tell: should there now be a "+ assert(clid)"?</div>
<div><br>
</div>
<blockquote style="margin-top:0;margin-bottom:0">@@ -292,6 +293,7 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)<br>
* dctx->mst->tstamp = 0; // stdw [%r0 + DMST_TSTAMP], 0<br>
* dctx->mst->epid = EPID; // stw [%r0 + DMST_EPID], EPID<br>
* dctx->mst->prid = PRID; // stw [%r0 + DMST_PRID], PRID<br>
<div>+ * dctx->mst->prid = CLID; // stw [%r0 + DMST_CLID], CLID<br>
</div>
</blockquote>
<div><br>
</div>
<div>dctx->mst->clid? Or, just skip these comments altogether since they are no more readable than the actual code (and clearly the comments and code get out of sync).</div>
<div><br>
</div>
<blockquote style="margin-top:0;margin-bottom:0">
<div> * *((uint32_t *)&buf[0]) = EPID;<br>
</div>
* // stw [%r9 + 0], EPID<br>
*/<br>
@@ -300,6 +302,7 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)<br>
emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_0, DMST_TSTAMP, 0));<br>
emite(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_0, DMST_EPID, -1), epid);<br>
emite(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_0, DMST_PRID, -1), prid);<br>
+ emite(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_0, DMST_CLID, -1), clid);<br>
emite(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_9, 0, -1), epid);<br>
<br>
/*<br>
<div>diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h<br>
</div>
<div>@@ -20,6 +20,7 @@<br>
</div>
typedef struct dt_mstate {<br>
uint32_t epid; /* Enabled probe ID */<br>
uint32_t prid; /* Probe ID */<br>
+ uint32_t clid; /* Clause ID (unique per probe) */<br>
uint32_t tag; /* Tag (for future use) */<br>
uint64_t fault; /* DTrace fault flags */<br>
uint64_t tstamp; /* cached timestamp value */<br>
@@ -55,6 +56,7 @@ typedef struct dt_dctx {<br>
<br>
#define DMST_EPID offsetof(dt_mstate_t, epid)<br>
#define DMST_PRID offsetof(dt_mstate_t, prid)<br>
+#define DMST_CLID offsetof(dt_mstate_t, clid)<br>
#define DMST_TAG offsetof(dt_mstate_t, tag)<br>
#define DMST_FAULT offsetof(dt_mstate_t, fault)<br>
#define DMST_TSTAMP offsetof(dt_mstate_t, tstamp)<br>
<div>diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c<br>
</div>
<div>@@ -74,6 +74,7 @@ static const dt_ident_t dt_bpf_symbols[] = {<br>
</div>
/* BPF internal identifiers */<br>
DT_BPF_SYMBOL_ID(EPID, DT_IDENT_SCALAR, DT_CONST_EPID),<br>
DT_BPF_SYMBOL_ID(PRID, DT_IDENT_SCALAR, DT_CONST_PRID),<br>
+ DT_BPF_SYMBOL_ID(CLID, DT_IDENT_SCALAR, DT_CONST_CLID),<br>
DT_BPF_SYMBOL_ID(ARGC, DT_IDENT_SCALAR, DT_CONST_ARGC),<br>
/* End-of-list marker */<br>
<div> { NULL, }<br>
</div>
</blockquote>
<div><br>
</div>
<div>I don't really follow the code but:</div>
<div>Reviewed-by: Eugene Loh <eugene.loh@oracle.com></div>
</div>
</span></font></div>
</body>
</html>