Re: refcount btrace
On Fri, Apr 22, 2022 at 12:07:44AM +0200, Alexander Bluhm wrote:
> I still think it is worth to have refcount debugging in generic
> kernel dt(4). Having tools is easier than first adding printf to
> hunt a bug. I see no downside.
I have talked with mpi@. We both think it is useful to have refcount
debugging tools available in GENERIC kernel. If there are no hard
objections, I will put this in tomorrow.
bluhm
> Index: dev/dt/dt_prov_static.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 dt_prov_static.c
> --- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
> +++ dev/dt/dt_prov_static.c 21 Apr 2022 21:06:03 -
> @@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
> DT_STATIC_PROBE0(smr, wakeup);
> DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
>
> +/*
> + * reference counting
> + */
> +DT_STATIC_PROBE0(refcnt, none);
> +DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
> +DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
>
> /*
> * List of all static probes
> @@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
> &_DT_STATIC_P(smr, barrier_exit),
> &_DT_STATIC_P(smr, wakeup),
> &_DT_STATIC_P(smr, thread),
> + /* refcnt */
> + &_DT_STATIC_P(refcnt, none),
> + &_DT_STATIC_P(refcnt, inpcb),
> + &_DT_STATIC_P(refcnt, tdb),
> };
>
> +struct dt_probe *const *dtps_index_refcnt;
> +
> int
> dt_prov_static_init(void)
> {
> int i;
>
> - for (i = 0; i < nitems(dtps_static); i++)
> + for (i = 0; i < nitems(dtps_static); i++) {
> + if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
> + dtps_index_refcnt = &dtps_static[i];
> dt_dev_register_probe(dtps_static[i]);
> + }
>
> return i;
> }
> Index: dev/dt/dtvar.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 dtvar.h
> --- dev/dt/dtvar.h27 Feb 2022 10:14:01 - 1.13
> +++ dev/dt/dtvar.h21 Apr 2022 21:06:03 -
> @@ -313,11 +313,30 @@ extern volatile uint32_tdt_tracing; /*
> #define DT_STATIC_ENTER(func, name, args...) do {
> \
> extern struct dt_probe _DT_STATIC_P(func, name);\
> struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
> - struct dt_provider *dtpv = dtp->dtp_prov; \
> \
> if (__predict_false(dt_tracing) && \
> __predict_false(dtp->dtp_recording)) { \
> + struct dt_provider *dtpv = dtp->dtp_prov; \
> + \
> dtpv->dtpv_enter(dtpv, dtp, args); \
> + } \
> +} while (0)
> +
> +#define _DT_INDEX_P(func)(dtps_index_##func)
> +
> +#define DT_INDEX_ENTER(func, index, args...) do {\
> + extern struct dt_probe **_DT_INDEX_P(func); \
> + \
> + if (__predict_false(dt_tracing) && \
> + __predict_false(index > 0) && \
> + __predict_true(_DT_INDEX_P(func) != NULL)) {\
> + struct dt_probe *dtp = _DT_INDEX_P(func)[index];\
> + \
> + if(__predict_false(dtp->dtp_recording)) { \
> + struct dt_provider *dtpv = dtp->dtp_prov; \
> + \
> + dtpv->dtpv_enter(dtpv, dtp, args); \
> + } \
> } \
> } while (0)
>
> Index: kern/kern_synch.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 kern_synch.c
> --- kern/kern_synch.c 18 Mar 2022 15:32:06 - 1.185
> +++ kern/kern_synch.c 21 Apr 2022 21:06:03 -
> @@ -804,7 +804,15 @@ sys___thrwakeup(struct proc *p, void *v,
> void
> refcnt_init(struct refcnt *r)
> {
> + refcnt_init_trace(r, 0);
> +}
> +
> +void
> +refcnt_init_trace(struct refcnt *r, int idx)
> +{
> + r->r_traceidx = idx;
> atomic_store_int(&r->r_refs, 1);
> + TRACEINDEX(refcnt, r->r_traceidx, r, 0, +1);
> }
>
> void
> @@ -814,6 +822,7 @@ ref
Re: refcount btrace
I still think it is worth to have refcount debugging in generic
kernel dt(4). Having tools is easier than first adding printf to
hunt a bug. I see no downside.
ok?
bluhm
Index: dev/dt/dt_prov_static.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
retrieving revision 1.13
diff -u -p -r1.13 dt_prov_static.c
--- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
+++ dev/dt/dt_prov_static.c 21 Apr 2022 21:06:03 -
@@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
DT_STATIC_PROBE0(smr, wakeup);
DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
+/*
+ * reference counting
+ */
+DT_STATIC_PROBE0(refcnt, none);
+DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
+DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
/*
* List of all static probes
@@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
&_DT_STATIC_P(smr, barrier_exit),
&_DT_STATIC_P(smr, wakeup),
&_DT_STATIC_P(smr, thread),
+ /* refcnt */
+ &_DT_STATIC_P(refcnt, none),
+ &_DT_STATIC_P(refcnt, inpcb),
+ &_DT_STATIC_P(refcnt, tdb),
};
+struct dt_probe *const *dtps_index_refcnt;
+
int
dt_prov_static_init(void)
{
int i;
- for (i = 0; i < nitems(dtps_static); i++)
+ for (i = 0; i < nitems(dtps_static); i++) {
+ if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
+ dtps_index_refcnt = &dtps_static[i];
dt_dev_register_probe(dtps_static[i]);
+ }
return i;
}
Index: dev/dt/dtvar.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
retrieving revision 1.13
diff -u -p -r1.13 dtvar.h
--- dev/dt/dtvar.h 27 Feb 2022 10:14:01 - 1.13
+++ dev/dt/dtvar.h 21 Apr 2022 21:06:03 -
@@ -313,11 +313,30 @@ extern volatile uint32_t dt_tracing; /*
#defineDT_STATIC_ENTER(func, name, args...) do {
\
extern struct dt_probe _DT_STATIC_P(func, name);\
struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
- struct dt_provider *dtpv = dtp->dtp_prov; \
\
if (__predict_false(dt_tracing) && \
__predict_false(dtp->dtp_recording)) { \
+ struct dt_provider *dtpv = dtp->dtp_prov; \
+ \
dtpv->dtpv_enter(dtpv, dtp, args); \
+ } \
+} while (0)
+
+#define _DT_INDEX_P(func) (dtps_index_##func)
+
+#define DT_INDEX_ENTER(func, index, args...) do { \
+ extern struct dt_probe **_DT_INDEX_P(func); \
+ \
+ if (__predict_false(dt_tracing) && \
+ __predict_false(index > 0) && \
+ __predict_true(_DT_INDEX_P(func) != NULL)) {\
+ struct dt_probe *dtp = _DT_INDEX_P(func)[index];\
+ \
+ if(__predict_false(dtp->dtp_recording)) { \
+ struct dt_provider *dtpv = dtp->dtp_prov; \
+ \
+ dtpv->dtpv_enter(dtpv, dtp, args); \
+ } \
} \
} while (0)
Index: kern/kern_synch.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.185
diff -u -p -r1.185 kern_synch.c
--- kern/kern_synch.c 18 Mar 2022 15:32:06 - 1.185
+++ kern/kern_synch.c 21 Apr 2022 21:06:03 -
@@ -804,7 +804,15 @@ sys___thrwakeup(struct proc *p, void *v,
void
refcnt_init(struct refcnt *r)
{
+ refcnt_init_trace(r, 0);
+}
+
+void
+refcnt_init_trace(struct refcnt *r, int idx)
+{
+ r->r_traceidx = idx;
atomic_store_int(&r->r_refs, 1);
+ TRACEINDEX(refcnt, r->r_traceidx, r, 0, +1);
}
void
@@ -814,6 +822,7 @@ refcnt_take(struct refcnt *r)
refs = atomic_inc_int_nv(&r->r_refs);
KASSERT(refs != 0);
+ TRACEINDEX(refcnt, r->r_traceidx, r, refs - 1, +1);
(void)refs;
}
@@ -824,6 +833,7 @@ refcnt_rele(struct refcnt *r)
refs = atomic_dec_int_nv(&r->r_refs);
KASSERT(refs != ~0);
+ TRACEINDEX(refcnt, r->r_traceidx, r, r
Re: refcount btrace
On Mon, Apr 11, 2022 at 07:19:00PM +0200, Martin Pieuchot wrote:
> On 08/04/22(Fri) 12:16, Alexander Bluhm wrote:
> > On Fri, Apr 08, 2022 at 02:39:34AM +, Visa Hankala wrote:
> > > On Thu, Apr 07, 2022 at 07:55:11PM +0200, Alexander Bluhm wrote:
> > > > On Wed, Mar 23, 2022 at 06:13:27PM +0100, Alexander Bluhm wrote:
> > > > > In my opinion tracepoints give insight at minimal cost. It is worth
> > > > > it to have it in GENERIC to make it easy to use.
> > > >
> > > > After release I want to revive the btrace of refcounts discussion.
> > > >
> > > > As mpi@ mentioned the idea of dt(4) is to have these trace points
> > > > in GENERIC kernel. If you want to hunt a bug, just turn it on.
> > > > Refounting is a common place for bugs, leaks can be detected easily.
> > > >
> > > > The alternative are some defines that you can compile in and access
> > > > from ddb. This is more work and you would have to implement it for
> > > > every recount.
> > > > https://marc.info/?l=openbsd-tech&m=163786435916039&w=2
> > > >
> > > > There is no measuarable performance difference. dt(4) is written
> > > > in a way that is is only one additional branch. At least my goal
> > > > is to add trace points to useful places when we identify them.
> > >
> > > DT_INDEX_ENTER() still checks the index first, so it has two branches
> > > in practice.
> > >
> > > I think dt_tracing should be checked first so that it serves as
> > > a gateway to the trace code. Under normal operation, the check's
> > > outcome is always the same, which is easy even for simple branch
> > > predictors.
> >
> > Reordering the check is easy. Now dt_tracing is first.
> >
> > > I have a slight suspicion that dt(4) is now becoming a way to add code
> > > that would be otherwise unacceptable. Also, how "durable" are trace
> > > points perceived? Is an added trace point an achieved advantage that
> > > is difficult to remove even when its utility has diminished? There is
> > > a similarity to (ad hoc) debug printfs too.
> >
> > As I understand dt(4) it is a replacement for debug printfs. But
> > it has advantages. I can be turnd on selectively from userland.
> > It does not spam the console, but can be processed in userland. It
> > is always there, you don't have to recompile.
> >
> > Of course you always have the printf or tracepoint at the worng
> > place. I think people debugging the code should move them to
> > the useful places. Then we may end with generally useful tool.
> > A least that is my hope.
> >
> > There are obvious places to debug. We have syscall entry and return.
> > And I think reference counting is also generally interesting.
>
> I'm happy if this can help debugging real reference counting issues. Do
> you have a script that could be committed to /usr/share/btrace to show
> how to track reference counting using these probes?
Script looks like this:
#!/usr/sbin/btrace
tracepoint:refcnt:inpcb{
printf("%s %x %u %+d\n", probe, arg0, arg1, arg2)
}
Note that output should be -1 instead of +4294967295, but that is
a different problem.
tracepoint:refcnt:inpcb fd80793885c0 2 +1
tracepoint:refcnt:inpcb fd80793885c0 3 +4294967295
tracepoint:refcnt:inpcb fd80793885c0 2 +1
tracepoint:refcnt:inpcb fd80793885c0 3 +4294967295
tracepoint:refcnt:inpcb fd80793885c0 2 +1
tracepoint:refcnt:inpcb fd80793885c0 3 +4294967295
Or with kernel stack:
#!/usr/sbin/btrace
tracepoint:refcnt:inpcb{
printf("%s %x %u %+d%s\n", probe, arg0, arg1, arg2, kstack)
}
tracepoint:refcnt:inpcb fd80793885c0 3 +4294967295
refcnt_rele+0x88
in_pcbunref+0x24
pf_find_state+0x2a6
pf_test_state+0x172
pf_test+0xd17
ip6_output+0xd14
tcp_output+0x164f
tcp_usrreq+0x386
sosend+0x37c
dofilewritev+0x14d
sys_write+0x51
syscall+0x314
Xsyscall+0x128
kernel
> > Index: dev/dt/dt_prov_static.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
> > retrieving revision 1.13
> > diff -u -p -r1.13 dt_prov_static.c
> > --- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
> > +++ dev/dt/dt_prov_static.c 8 Apr 2022 09:40:29 -
> > @@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
> > DT_STATIC_PROBE0(smr, wakeup);
> > DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
> >
> > +/*
> > + * reference counting
> > + */
> > +DT_STATIC_PROBE0(refcnt, none);
> > +DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
> > +DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
> >
> > /*
> > * List of all static probes
> > @@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
> > &_DT_STATIC_P(smr, barrier_exit),
> > &_DT_STATIC_P(smr, wakeup),
> > &_DT_STATIC_P(smr, thread),
> > + /* refcnt */
> > + &_DT_STATIC_P(refcnt, none),
> > + &_DT_STATIC_P(refcnt, inpcb),
> > + &_DT_STATIC_P(refcnt, tdb),
> > };
> >
> > +struct dt_probe *const *dtps_index_refcnt;
> > +
> > int
> > dt_pr
Re: refcount btrace
On 08/04/22(Fri) 12:16, Alexander Bluhm wrote:
> On Fri, Apr 08, 2022 at 02:39:34AM +, Visa Hankala wrote:
> > On Thu, Apr 07, 2022 at 07:55:11PM +0200, Alexander Bluhm wrote:
> > > On Wed, Mar 23, 2022 at 06:13:27PM +0100, Alexander Bluhm wrote:
> > > > In my opinion tracepoints give insight at minimal cost. It is worth
> > > > it to have it in GENERIC to make it easy to use.
> > >
> > > After release I want to revive the btrace of refcounts discussion.
> > >
> > > As mpi@ mentioned the idea of dt(4) is to have these trace points
> > > in GENERIC kernel. If you want to hunt a bug, just turn it on.
> > > Refounting is a common place for bugs, leaks can be detected easily.
> > >
> > > The alternative are some defines that you can compile in and access
> > > from ddb. This is more work and you would have to implement it for
> > > every recount.
> > > https://marc.info/?l=openbsd-tech&m=163786435916039&w=2
> > >
> > > There is no measuarable performance difference. dt(4) is written
> > > in a way that is is only one additional branch. At least my goal
> > > is to add trace points to useful places when we identify them.
> >
> > DT_INDEX_ENTER() still checks the index first, so it has two branches
> > in practice.
> >
> > I think dt_tracing should be checked first so that it serves as
> > a gateway to the trace code. Under normal operation, the check's
> > outcome is always the same, which is easy even for simple branch
> > predictors.
>
> Reordering the check is easy. Now dt_tracing is first.
>
> > I have a slight suspicion that dt(4) is now becoming a way to add code
> > that would be otherwise unacceptable. Also, how "durable" are trace
> > points perceived? Is an added trace point an achieved advantage that
> > is difficult to remove even when its utility has diminished? There is
> > a similarity to (ad hoc) debug printfs too.
>
> As I understand dt(4) it is a replacement for debug printfs. But
> it has advantages. I can be turnd on selectively from userland.
> It does not spam the console, but can be processed in userland. It
> is always there, you don't have to recompile.
>
> Of course you always have the printf or tracepoint at the worng
> place. I think people debugging the code should move them to
> the useful places. Then we may end with generally useful tool.
> A least that is my hope.
>
> There are obvious places to debug. We have syscall entry and return.
> And I think reference counting is also generally interesting.
I'm happy if this can help debugging real reference counting issues. Do
you have a script that could be committed to /usr/share/btrace to show
how to track reference counting using these probes?
> Index: dev/dt/dt_prov_static.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 dt_prov_static.c
> --- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
> +++ dev/dt/dt_prov_static.c 8 Apr 2022 09:40:29 -
> @@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
> DT_STATIC_PROBE0(smr, wakeup);
> DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
>
> +/*
> + * reference counting
> + */
> +DT_STATIC_PROBE0(refcnt, none);
> +DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
> +DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
>
> /*
> * List of all static probes
> @@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
> &_DT_STATIC_P(smr, barrier_exit),
> &_DT_STATIC_P(smr, wakeup),
> &_DT_STATIC_P(smr, thread),
> + /* refcnt */
> + &_DT_STATIC_P(refcnt, none),
> + &_DT_STATIC_P(refcnt, inpcb),
> + &_DT_STATIC_P(refcnt, tdb),
> };
>
> +struct dt_probe *const *dtps_index_refcnt;
> +
> int
> dt_prov_static_init(void)
> {
> int i;
>
> - for (i = 0; i < nitems(dtps_static); i++)
> + for (i = 0; i < nitems(dtps_static); i++) {
> + if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
> + dtps_index_refcnt = &dtps_static[i];
> dt_dev_register_probe(dtps_static[i]);
> + }
>
> return i;
> }
> Index: dev/dt/dtvar.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 dtvar.h
> --- dev/dt/dtvar.h27 Feb 2022 10:14:01 - 1.13
> +++ dev/dt/dtvar.h8 Apr 2022 09:42:19 -
> @@ -313,11 +313,30 @@ extern volatile uint32_tdt_tracing; /*
> #define DT_STATIC_ENTER(func, name, args...) do {
> \
> extern struct dt_probe _DT_STATIC_P(func, name);\
> struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
> - struct dt_provider *dtpv = dtp->dtp_prov; \
> \
> if (__pr
Re: refcount btrace
On Fri, Apr 08, 2022 at 02:39:34AM +, Visa Hankala wrote:
> On Thu, Apr 07, 2022 at 07:55:11PM +0200, Alexander Bluhm wrote:
> > On Wed, Mar 23, 2022 at 06:13:27PM +0100, Alexander Bluhm wrote:
> > > In my opinion tracepoints give insight at minimal cost. It is worth
> > > it to have it in GENERIC to make it easy to use.
> >
> > After release I want to revive the btrace of refcounts discussion.
> >
> > As mpi@ mentioned the idea of dt(4) is to have these trace points
> > in GENERIC kernel. If you want to hunt a bug, just turn it on.
> > Refounting is a common place for bugs, leaks can be detected easily.
> >
> > The alternative are some defines that you can compile in and access
> > from ddb. This is more work and you would have to implement it for
> > every recount.
> > https://marc.info/?l=openbsd-tech&m=163786435916039&w=2
> >
> > There is no measuarable performance difference. dt(4) is written
> > in a way that is is only one additional branch. At least my goal
> > is to add trace points to useful places when we identify them.
>
> DT_INDEX_ENTER() still checks the index first, so it has two branches
> in practice.
>
> I think dt_tracing should be checked first so that it serves as
> a gateway to the trace code. Under normal operation, the check's
> outcome is always the same, which is easy even for simple branch
> predictors.
Reordering the check is easy. Now dt_tracing is first.
> I have a slight suspicion that dt(4) is now becoming a way to add code
> that would be otherwise unacceptable. Also, how "durable" are trace
> points perceived? Is an added trace point an achieved advantage that
> is difficult to remove even when its utility has diminished? There is
> a similarity to (ad hoc) debug printfs too.
As I understand dt(4) it is a replacement for debug printfs. But
it has advantages. I can be turnd on selectively from userland.
It does not spam the console, but can be processed in userland. It
is always there, you don't have to recompile.
Of course you always have the printf or tracepoint at the worng
place. I think people debugging the code should move them to
the useful places. Then we may end with generally useful tool.
A least that is my hope.
There are obvious places to debug. We have syscall entry and return.
And I think reference counting is also generally interesting.
bluhm
Index: dev/dt/dt_prov_static.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
retrieving revision 1.13
diff -u -p -r1.13 dt_prov_static.c
--- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
+++ dev/dt/dt_prov_static.c 8 Apr 2022 09:40:29 -
@@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
DT_STATIC_PROBE0(smr, wakeup);
DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
+/*
+ * reference counting
+ */
+DT_STATIC_PROBE0(refcnt, none);
+DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
+DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
/*
* List of all static probes
@@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
&_DT_STATIC_P(smr, barrier_exit),
&_DT_STATIC_P(smr, wakeup),
&_DT_STATIC_P(smr, thread),
+ /* refcnt */
+ &_DT_STATIC_P(refcnt, none),
+ &_DT_STATIC_P(refcnt, inpcb),
+ &_DT_STATIC_P(refcnt, tdb),
};
+struct dt_probe *const *dtps_index_refcnt;
+
int
dt_prov_static_init(void)
{
int i;
- for (i = 0; i < nitems(dtps_static); i++)
+ for (i = 0; i < nitems(dtps_static); i++) {
+ if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
+ dtps_index_refcnt = &dtps_static[i];
dt_dev_register_probe(dtps_static[i]);
+ }
return i;
}
Index: dev/dt/dtvar.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
retrieving revision 1.13
diff -u -p -r1.13 dtvar.h
--- dev/dt/dtvar.h 27 Feb 2022 10:14:01 - 1.13
+++ dev/dt/dtvar.h 8 Apr 2022 09:42:19 -
@@ -313,11 +313,30 @@ extern volatile uint32_t dt_tracing; /*
#defineDT_STATIC_ENTER(func, name, args...) do {
\
extern struct dt_probe _DT_STATIC_P(func, name);\
struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
- struct dt_provider *dtpv = dtp->dtp_prov; \
\
if (__predict_false(dt_tracing) && \
__predict_false(dtp->dtp_recording)) { \
+ struct dt_provider *dtpv = dtp->dtp_prov; \
+ \
dtpv->dtpv_enter(dtpv, dtp, args); \
+ }
Re: refcount btrace
On Thu, Apr 07, 2022 at 07:55:11PM +0200, Alexander Bluhm wrote: > On Wed, Mar 23, 2022 at 06:13:27PM +0100, Alexander Bluhm wrote: > > In my opinion tracepoints give insight at minimal cost. It is worth > > it to have it in GENERIC to make it easy to use. > > After release I want to revive the btrace of refcounts discussion. > > As mpi@ mentioned the idea of dt(4) is to have these trace points > in GENERIC kernel. If you want to hunt a bug, just turn it on. > Refounting is a common place for bugs, leaks can be detected easily. > > The alternative are some defines that you can compile in and access > from ddb. This is more work and you would have to implement it for > every recount. > https://marc.info/?l=openbsd-tech&m=163786435916039&w=2 > > There is no measuarable performance difference. dt(4) is written > in a way that is is only one additional branch. At least my goal > is to add trace points to useful places when we identify them. DT_INDEX_ENTER() still checks the index first, so it has two branches in practice. I think dt_tracing should be checked first so that it serves as a gateway to the trace code. Under normal operation, the check's outcome is always the same, which is easy even for simple branch predictors. I have a slight suspicion that dt(4) is now becoming a way to add code that would be otherwise unacceptable. Also, how "durable" are trace points perceived? Is an added trace point an achieved advantage that is difficult to remove even when its utility has diminished? There is a similarity to (ad hoc) debug printfs too.
Re: refcount btrace
On Wed, Mar 23, 2022 at 06:13:27PM +0100, Alexander Bluhm wrote:
> In my opinion tracepoints give insight at minimal cost. It is worth
> it to have it in GENERIC to make it easy to use.
After release I want to revive the btrace of refcounts discussion.
As mpi@ mentioned the idea of dt(4) is to have these trace points
in GENERIC kernel. If you want to hunt a bug, just turn it on.
Refounting is a common place for bugs, leaks can be detected easily.
The alternative are some defines that you can compile in and access
from ddb. This is more work and you would have to implement it for
every recount.
https://marc.info/?l=openbsd-tech&m=163786435916039&w=2
There is no measuarable performance difference. dt(4) is written
in a way that is is only one additional branch. At least my goal
is to add trace points to useful places when we identify them.
ok?
bluhm
Index: dev/dt/dt_prov_static.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
retrieving revision 1.13
diff -u -p -r1.13 dt_prov_static.c
--- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
+++ dev/dt/dt_prov_static.c 7 Apr 2022 17:32:23 -
@@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
DT_STATIC_PROBE0(smr, wakeup);
DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
+/*
+ * reference counting
+ */
+DT_STATIC_PROBE0(refcnt, none);
+DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
+DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
/*
* List of all static probes
@@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
&_DT_STATIC_P(smr, barrier_exit),
&_DT_STATIC_P(smr, wakeup),
&_DT_STATIC_P(smr, thread),
+ /* refcnt */
+ &_DT_STATIC_P(refcnt, none),
+ &_DT_STATIC_P(refcnt, inpcb),
+ &_DT_STATIC_P(refcnt, tdb),
};
+struct dt_probe *const *dtps_index_refcnt;
+
int
dt_prov_static_init(void)
{
int i;
- for (i = 0; i < nitems(dtps_static); i++)
+ for (i = 0; i < nitems(dtps_static); i++) {
+ if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
+ dtps_index_refcnt = &dtps_static[i];
dt_dev_register_probe(dtps_static[i]);
+ }
return i;
}
Index: dev/dt/dtvar.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
retrieving revision 1.13
diff -u -p -r1.13 dtvar.h
--- dev/dt/dtvar.h 27 Feb 2022 10:14:01 - 1.13
+++ dev/dt/dtvar.h 7 Apr 2022 17:41:55 -
@@ -313,11 +313,30 @@ extern volatile uint32_t dt_tracing; /*
#defineDT_STATIC_ENTER(func, name, args...) do {
\
extern struct dt_probe _DT_STATIC_P(func, name);\
struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
- struct dt_provider *dtpv = dtp->dtp_prov; \
\
if (__predict_false(dt_tracing) && \
__predict_false(dtp->dtp_recording)) { \
+ struct dt_provider *dtpv = dtp->dtp_prov; \
+ \
dtpv->dtpv_enter(dtpv, dtp, args); \
+ } \
+} while (0)
+
+#define _DT_INDEX_P(func) (dtps_index_##func)
+
+#define DT_INDEX_ENTER(func, index, args...) do { \
+ extern struct dt_probe **_DT_INDEX_P(func); \
+ \
+ if (__predict_false(index > 0) && \
+ __predict_false(dt_tracing) && \
+ __predict_true(_DT_INDEX_P(func) != NULL)) {\
+ struct dt_probe *dtp = _DT_INDEX_P(func)[index];\
+ \
+ if(__predict_false(dtp->dtp_recording)) { \
+ struct dt_provider *dtpv = dtp->dtp_prov; \
+ \
+ dtpv->dtpv_enter(dtpv, dtp, args); \
+ } \
} \
} while (0)
Index: kern/kern_synch.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.185
diff -u -p -r1.185 kern_synch.c
--- kern/kern_synch.c 18 Mar 2022 15:32:06 - 1.185
+++ kern/kern_synch.c 7 Apr 2022 16:25:45 -
@@ -804,7 +804,15 @
Re: refcount btrace
On Mon, Mar 21, 2022 at 01:22:22PM +0100, Martin Pieuchot wrote: > On 20/03/22(Sun) 05:39, Visa Hankala wrote: > > On Sat, Mar 19, 2022 at 12:10:11AM +0100, Alexander Bluhm wrote: > > > On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote: > > > > On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote: > > > > > I would like to use btrace to debug refernce counting. The idea > > > > > is to a a tracepoint for every type of refcnt we have. When it > > > > > changes, print the actual object, the current counter and the change > > > > > value. > > > > > > > > > Do we want that feature? > > > > > > > > I am against this in its current form. The code would become more > > > > complex, and the trace points can affect timing. There is a risk that > > > > the kernel behaves slightly differently when dt has been compiled in. > > > > > > On our main architectures dt(4) is in GENERIC. I see your timing > > > point for uvm structures. > > > > In my opinion, having dt(4) enabled by default is another reason why > > there should be no carte blanche for adding trace points. Each trace > > point adds a tiny amount of bloat. Few users will use the tracing > > facility. > > > > Maybe high-rate trace points could be behind a build option... > > The whole point of dt(4) is to be able to debug GENERIC kernel. I doubt > the cost of an additional if () block matters. The idea of dt(4) is that developer or end user with instructions can debug a running kernel without recompiling. So we have to put trace points at places where we gain much information. I did some meassurement with and without dt. Note that I configure my tests machines with sysctl kern.allowdt=1. I had to disable it in kernel diff. http://bluhm.genua.de/perform/results/2022-03-21T09%3A08%3A37Z/perform.html I see difference from moving the kernel objects. Even reboot and testing again has more variance than dt(4). The story is different when btrace(8) is actually running. Look at the numbers in the right column. http://bluhm.genua.de/perform/results/2022-03-21T09%3A08%3A37Z/2022-03-21T00%3A00%3A00Z/perform.html For the network test it does not matter, as our IP stack uses only one or maybe two cores. On a 4 core machine btrace userland can use 1 core. When compiling the kernel in the "make-bsd-j4" test row, the build time goes up as btrace takes CPU time from the compiler. In my opinion tracepoints give insight at minimal cost. It is worth it to have it in GENERIC to make it easy to use. bluhm
Re: refcount btrace
On 20/03/22(Sun) 05:39, Visa Hankala wrote: > On Sat, Mar 19, 2022 at 12:10:11AM +0100, Alexander Bluhm wrote: > > On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote: > > > On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote: > > > > I would like to use btrace to debug refernce counting. The idea > > > > is to a a tracepoint for every type of refcnt we have. When it > > > > changes, print the actual object, the current counter and the change > > > > value. > > > > > > > Do we want that feature? > > > > > > I am against this in its current form. The code would become more > > > complex, and the trace points can affect timing. There is a risk that > > > the kernel behaves slightly differently when dt has been compiled in. > > > > On our main architectures dt(4) is in GENERIC. I see your timing > > point for uvm structures. > > In my opinion, having dt(4) enabled by default is another reason why > there should be no carte blanche for adding trace points. Each trace > point adds a tiny amount of bloat. Few users will use the tracing > facility. > > Maybe high-rate trace points could be behind a build option... The whole point of dt(4) is to be able to debug GENERIC kernel. I doubt the cost of an additional if () block matters.
Re: refcount btrace
On Sat, Mar 19, 2022 at 12:10:11AM +0100, Alexander Bluhm wrote: > On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote: > > On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote: > > > I would like to use btrace to debug refernce counting. The idea > > > is to a a tracepoint for every type of refcnt we have. When it > > > changes, print the actual object, the current counter and the change > > > value. > > > > > Do we want that feature? > > > > I am against this in its current form. The code would become more > > complex, and the trace points can affect timing. There is a risk that > > the kernel behaves slightly differently when dt has been compiled in. > > On our main architectures dt(4) is in GENERIC. I see your timing > point for uvm structures. In my opinion, having dt(4) enabled by default is another reason why there should be no carte blanche for adding trace points. Each trace point adds a tiny amount of bloat. Few users will use the tracing facility. Maybe high-rate trace points could be behind a build option... > What do you think about this? The check starts with a > __predict_false(index > 0) in #define DT_INDEX_ENTER. The r_traceidx > is very likely in the same cache line as r_refs. So the additional > overhead of the branch should be small compared to the atomic > operation. The __predict_false(dt_tracing) might take longer as > it is a global variable. I have no hard data to back up my claim, but I think dt_tracing should be checked first. This would make the situation easier for branch prediction. It is likely that dt_tracing is already in cache.
Re: refcount btrace
On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote:
> On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote:
> > I would like to use btrace to debug refernce counting. The idea
> > is to a a tracepoint for every type of refcnt we have. When it
> > changes, print the actual object, the current counter and the change
> > value.
>
> > Do we want that feature?
>
> I am against this in its current form. The code would become more
> complex, and the trace points can affect timing. There is a risk that
> the kernel behaves slightly differently when dt has been compiled in.
On our main architectures dt(4) is in GENERIC. I see your timing
point for uvm structures.
What do you think about this? The check starts with a
__predict_false(index > 0) in #define DT_INDEX_ENTER. The r_traceidx
is very likely in the same cache line as r_refs. So the additional
overhead of the branch should be small compared to the atomic
operation. The __predict_false(dt_tracing) might take longer as
it is a global variable.
Default is not to trace refcnt. But I would like to have it for
network objects. For sending network packets the additional branch
instruction depending on a global variable does not count.
bluhm
Index: dev/dt/dt_prov_static.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
retrieving revision 1.13
diff -u -p -r1.13 dt_prov_static.c
--- dev/dt/dt_prov_static.c 17 Mar 2022 14:53:59 - 1.13
+++ dev/dt/dt_prov_static.c 18 Mar 2022 20:35:02 -
@@ -2,6 +2,7 @@
/*
* Copyright (c) 2019 Martin Pieuchot
+ * Copyright (c) 2022 Alexander Bluhm
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -87,6 +88,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
DT_STATIC_PROBE0(smr, wakeup);
DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
+/*
+ * reference counting
+ */
+DT_STATIC_PROBE0(refcnt, none);
+DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
+DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
/*
* List of all static probes
@@ -127,15 +134,24 @@ struct dt_probe *const dtps_static[] = {
&_DT_STATIC_P(smr, barrier_exit),
&_DT_STATIC_P(smr, wakeup),
&_DT_STATIC_P(smr, thread),
+ /* refcnt */
+ &_DT_STATIC_P(refcnt, none),
+ &_DT_STATIC_P(refcnt, inpcb),
+ &_DT_STATIC_P(refcnt, tdb),
};
+struct dt_probe *const *dtps_index_refcnt;
+
int
dt_prov_static_init(void)
{
int i;
- for (i = 0; i < nitems(dtps_static); i++)
+ for (i = 0; i < nitems(dtps_static); i++) {
+ if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
+ dtps_index_refcnt = &dtps_static[i];
dt_dev_register_probe(dtps_static[i]);
+ }
return i;
}
Index: dev/dt/dtvar.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
retrieving revision 1.13
diff -u -p -r1.13 dtvar.h
--- dev/dt/dtvar.h 27 Feb 2022 10:14:01 - 1.13
+++ dev/dt/dtvar.h 18 Mar 2022 20:58:28 -
@@ -2,6 +2,7 @@
/*
* Copyright (c) 2019 Martin Pieuchot
+ * Copyright (c) 2022 Alexander Bluhm
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -313,11 +314,30 @@ extern volatile uint32_t dt_tracing; /*
#defineDT_STATIC_ENTER(func, name, args...) do {
\
extern struct dt_probe _DT_STATIC_P(func, name);\
struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
- struct dt_provider *dtpv = dtp->dtp_prov; \
\
if (__predict_false(dt_tracing) && \
__predict_false(dtp->dtp_recording)) { \
+ struct dt_provider *dtpv = dtp->dtp_prov; \
+ \
dtpv->dtpv_enter(dtpv, dtp, args); \
+ } \
+} while (0)
+
+#define _DT_INDEX_P(func) (dtps_index_##func)
+
+#define DT_INDEX_ENTER(func, index, args...) do { \
+ extern struct dt_probe **_DT_INDEX_P(func); \
+ \
+ if (__predict_false(index > 0) && \
+ __predict_false(dt_tracing) && \
+ __predict_true(_DT_INDEX_P(func) != NULL)) {\
+ struct dt_probe *dtp = _DT_INDEX_P(func)[index];\
+
Re: refcount btrace
On Thu, Mar 17, 2022 at 06:16:51PM +0100, Alexander Bluhm wrote:
> On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote:
> > On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote:
> > > I would like to use btrace to debug refernce counting. The idea
> > > is to a a tracepoint for every type of refcnt we have. When it
> > > changes, print the actual object, the current counter and the change
> > > value.
> >
> > > Do we want that feature?
> >
> > I am against this in its current form. The code would become more
> > complex, and the trace points can affect timing. There is a risk that
> > the kernel behaves slightly differently when dt has been compiled in.
>
> Can we get in this part then?
>
> - Remove DIAGNOSTIC to keep similar in non DIAGNOSTIC case.
> - Rename refcnt to refs. refcnt is the struct, refs contains the
> r_refs value.
> - Add KASSERT(refs != ~0) in refcnt_finalize().
> - Always use u_int refs so I can insert my btrace diff easily.
I think this is fine.
OK visa@
> Index: kern/kern_synch.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
> retrieving revision 1.184
> diff -u -p -r1.184 kern_synch.c
> --- kern/kern_synch.c 16 Mar 2022 14:13:01 - 1.184
> +++ kern/kern_synch.c 17 Mar 2022 16:12:50 -
> @@ -810,25 +810,21 @@ refcnt_init(struct refcnt *r)
> void
> refcnt_take(struct refcnt *r)
> {
> -#ifdef DIAGNOSTIC
> - u_int refcnt;
> + u_int refs;
>
> - refcnt = atomic_inc_int_nv(&r->r_refs);
> - KASSERT(refcnt != 0);
> -#else
> - atomic_inc_int(&r->r_refs);
> -#endif
> + refs = atomic_inc_int_nv(&r->r_refs);
> + KASSERT(refs != 0);
> + (void)refs;
> }
>
> int
> refcnt_rele(struct refcnt *r)
> {
> - u_int refcnt;
> + u_int refs;
>
> - refcnt = atomic_dec_int_nv(&r->r_refs);
> - KASSERT(refcnt != ~0);
> -
> - return (refcnt == 0);
> + refs = atomic_dec_int_nv(&r->r_refs);
> + KASSERT(refs != ~0);
> + return (refs == 0);
> }
>
> void
> @@ -842,26 +838,33 @@ void
> refcnt_finalize(struct refcnt *r, const char *wmesg)
> {
> struct sleep_state sls;
> - u_int refcnt;
> + u_int refs;
>
> - refcnt = atomic_dec_int_nv(&r->r_refs);
> - while (refcnt) {
> + refs = atomic_dec_int_nv(&r->r_refs);
> + KASSERT(refs != ~0);
> + while (refs) {
> sleep_setup(&sls, r, PWAIT, wmesg, 0);
> - refcnt = atomic_load_int(&r->r_refs);
> - sleep_finish(&sls, refcnt);
> + refs = atomic_load_int(&r->r_refs);
> + sleep_finish(&sls, refs);
> }
> }
>
> int
> refcnt_shared(struct refcnt *r)
> {
> - return (atomic_load_int(&r->r_refs) > 1);
> + u_int refs;
> +
> + refs = atomic_load_int(&r->r_refs);
> + return (refs > 1);
> }
>
> unsigned int
> refcnt_read(struct refcnt *r)
> {
> - return (atomic_load_int(&r->r_refs));
> + u_int refs;
> +
> + refs = atomic_load_int(&r->r_refs);
> + return (refs);
> }
>
> void
>
Re: refcount btrace
On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote:
> On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote:
> > I would like to use btrace to debug refernce counting. The idea
> > is to a a tracepoint for every type of refcnt we have. When it
> > changes, print the actual object, the current counter and the change
> > value.
>
> > Do we want that feature?
>
> I am against this in its current form. The code would become more
> complex, and the trace points can affect timing. There is a risk that
> the kernel behaves slightly differently when dt has been compiled in.
Can we get in this part then?
- Remove DIAGNOSTIC to keep similar in non DIAGNOSTIC case.
- Rename refcnt to refs. refcnt is the struct, refs contains the
r_refs value.
- Add KASSERT(refs != ~0) in refcnt_finalize().
- Always use u_int refs so I can insert my btrace diff easily.
Maybe I can optimize btrace diff later.
bluhm
Index: kern/kern_synch.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.184
diff -u -p -r1.184 kern_synch.c
--- kern/kern_synch.c 16 Mar 2022 14:13:01 - 1.184
+++ kern/kern_synch.c 17 Mar 2022 16:12:50 -
@@ -810,25 +810,21 @@ refcnt_init(struct refcnt *r)
void
refcnt_take(struct refcnt *r)
{
-#ifdef DIAGNOSTIC
- u_int refcnt;
+ u_int refs;
- refcnt = atomic_inc_int_nv(&r->r_refs);
- KASSERT(refcnt != 0);
-#else
- atomic_inc_int(&r->r_refs);
-#endif
+ refs = atomic_inc_int_nv(&r->r_refs);
+ KASSERT(refs != 0);
+ (void)refs;
}
int
refcnt_rele(struct refcnt *r)
{
- u_int refcnt;
+ u_int refs;
- refcnt = atomic_dec_int_nv(&r->r_refs);
- KASSERT(refcnt != ~0);
-
- return (refcnt == 0);
+ refs = atomic_dec_int_nv(&r->r_refs);
+ KASSERT(refs != ~0);
+ return (refs == 0);
}
void
@@ -842,26 +838,33 @@ void
refcnt_finalize(struct refcnt *r, const char *wmesg)
{
struct sleep_state sls;
- u_int refcnt;
+ u_int refs;
- refcnt = atomic_dec_int_nv(&r->r_refs);
- while (refcnt) {
+ refs = atomic_dec_int_nv(&r->r_refs);
+ KASSERT(refs != ~0);
+ while (refs) {
sleep_setup(&sls, r, PWAIT, wmesg, 0);
- refcnt = atomic_load_int(&r->r_refs);
- sleep_finish(&sls, refcnt);
+ refs = atomic_load_int(&r->r_refs);
+ sleep_finish(&sls, refs);
}
}
int
refcnt_shared(struct refcnt *r)
{
- return (atomic_load_int(&r->r_refs) > 1);
+ u_int refs;
+
+ refs = atomic_load_int(&r->r_refs);
+ return (refs > 1);
}
unsigned int
refcnt_read(struct refcnt *r)
{
- return (atomic_load_int(&r->r_refs));
+ u_int refs;
+
+ refs = atomic_load_int(&r->r_refs);
+ return (refs);
}
void
Re: refcount btrace
On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote: > I would like to use btrace to debug refernce counting. The idea > is to a a tracepoint for every type of refcnt we have. When it > changes, print the actual object, the current counter and the change > value. > Do we want that feature? I am against this in its current form. The code would become more complex, and the trace points can affect timing. There is a risk that the kernel behaves slightly differently when dt has been compiled in.
refcount btrace
Hi,
I would like to use btrace to debug refernce counting. The idea
is to a a tracepoint for every type of refcnt we have. When it
changes, print the actual object, the current counter and the change
value.
#!/usr/sbin/btrace
tracepoint:refcnt:inpcb{
printf("%s %x %u %+d\n", probe, arg0, arg1, arg2)
}
It should look like this:
tracepoint:refcnt:inpcb fd8078e31840 0 +1
tracepoint:refcnt:inpcb fd8078e31840 1 +1
tracepoint:refcnt:inpcb fd8078e31840 2 +1
tracepoint:refcnt:inpcb fd8078e31840 3 -1
tracepoint:refcnt:inpcb fd8078e31840 2 +1
tracepoint:refcnt:inpcb fd8078e31840 3 -1
tracepoint:refcnt:inpcb fd8078e31840 2 -1
tracepoint:refcnt:inpcb fd8078e31840 1 -1
Unfortunately btrace cannot deal with negative numbers right now.
So it looks like this, but that can be fixed independently.
tracepoint:refcnt:inpcb fd8078e31840 0 +1
tracepoint:refcnt:inpcb fd8078e31840 1 +1
tracepoint:refcnt:inpcb fd8078e31840 2 +1
tracepoint:refcnt:inpcb fd8078e31840 3 +4294967295
tracepoint:refcnt:inpcb fd8078e31840 2 +1
tracepoint:refcnt:inpcb fd8078e31840 3 +4294967295
tracepoint:refcnt:inpcb fd8078e31840 2 +4294967295
tracepoint:refcnt:inpcb fd8078e31840 1 +4294967295
To debug leaks, btrace can also print kernel stack trace.
tracepoint:refcnt:inpcb fd8078e31840 0 +1
in_pcballoc+0x92
tcp_attach+0xd1
sonewconn+0x23d
syn_cache_get+0x1bf
tcp_input+0x885
ip_deliver+0xd3
ip6_input_if+0x762
ipv6_input+0x39
ether_input+0x3a2
if_input_process+0x6f
ifiq_process+0x69
taskq_thread+0xdc
proc_trampoline+0x17
kernel
tracepoint:refcnt:inpcb fd8078e31840 1 +1
in_pcbref+0x29
pf_inp_link+0x4e
tcp_input+0x8d2
ip_deliver+0xd3
ip6_input_if+0x762
ipv6_input+0x39
ether_input+0x3a2
if_input_process+0x6f
ifiq_process+0x69
taskq_thread+0xdc
proc_trampoline+0x17
kernel
tracepoint:refcnt:inpcb fd8078e31840 2 +1
in_pcbref+0x29
pf_mbuf_link_inpcb+0x27
tcp_output+0x1455
tcp_usrreq+0x386
sosend+0x37c
dofilewritev+0x14d
sys_write+0x51
syscall+0x314
Xsyscall+0x128
kernel
I register the tracepoint when initializing the refcnt. There
exists a global array of possible refcnt types. I implemented it
only for inpcb and tdb as a prove of concept.
Do we want that feature?
bluhm
Index: dev/dt/dt_prov_static.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
retrieving revision 1.12
diff -u -p -r1.12 dt_prov_static.c
--- dev/dt/dt_prov_static.c 26 Jan 2022 06:31:31 - 1.12
+++ dev/dt/dt_prov_static.c 16 Mar 2022 23:22:34 -
@@ -2,6 +2,7 @@
/*
* Copyright (c) 2019 Martin Pieuchot
+ * Copyright (c) 2022 Alexander Bluhm
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -87,6 +88,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
DT_STATIC_PROBE0(smr, wakeup);
DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
+/*
+ * reference counting
+ */
+DT_STATIC_PROBE0(refcnt, none);
+DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
+DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
/*
* List of all static probes
@@ -127,15 +134,24 @@ struct dt_probe *dtps_static[] = {
&_DT_STATIC_P(smr, barrier_exit),
&_DT_STATIC_P(smr, wakeup),
&_DT_STATIC_P(smr, thread),
+ /* refcnt */
+ &_DT_STATIC_P(refcnt, none),
+ &_DT_STATIC_P(refcnt, inpcb),
+ &_DT_STATIC_P(refcnt, tdb),
};
+struct dt_probe **dtps_index_refcnt;
+
int
dt_prov_static_init(void)
{
int i;
- for (i = 0; i < nitems(dtps_static); i++)
+ for (i = 0; i < nitems(dtps_static); i++) {
+ if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
+ dtps_index_refcnt = &dtps_static[i];
dt_dev_register_probe(dtps_static[i]);
+ }
return i;
}
Index: dev/dt/dtvar.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
retrieving revision 1.13
diff -u -p -r1.13 dtvar.h
--- dev/dt/dtvar.h 27 Feb 2022 10:14:01 - 1.13
+++ dev/dt/dtvar.h 16 Mar 2022 23:22:34 -
@@ -2,6 +2,7 @@
/*
* Copyright (c) 2019 Martin Pieuchot
+ * Copyright (c) 2022 Alexander Bluhm
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -313,11 +314,29 @@ extern volatile uint32_t dt_tracing; /*
#defineDT_STATIC_ENTER(func, name, args...) do {
\
extern struct dt_probe _DT_STATIC_P(func, name);\
struct dt_probe *dtp = &_DT_STATIC_P(func, name); \
- struct dt_provider *dtpv = dtp->dtp_prov; \
\
if (__predict_false(dt_tracing
