* [PATCH] uprobe: do not call populate_args() for pid probes @ 2025-08-14 3:58 Kris Van Hees 2025-08-14 16:47 ` [DTrace-devel] " Eugene Loh 0 siblings, 1 reply; 5+ messages in thread From: Kris Van Hees @ 2025-08-14 3:58 UTC (permalink / raw) To: dtrace, dtrace-devel Only USDT (regular and is-enabled) probes need populate_args(). Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com> --- libdtrace/dt_prov_uprobe.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c index c63d58383..605ae8397 100644 --- a/libdtrace/dt_prov_uprobe.c +++ b/libdtrace/dt_prov_uprobe.c @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp, */ } - if (populate_args(dtp, psp, upp) < 0) - goto fail; + if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) { + if (populate_args(dtp, psp, upp) < 0) + goto fail; + } return uprp; -- 2.45.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes 2025-08-14 3:58 [PATCH] uprobe: do not call populate_args() for pid probes Kris Van Hees @ 2025-08-14 16:47 ` Eugene Loh 2025-08-14 19:27 ` Kris Van Hees 0 siblings, 1 reply; 5+ messages in thread From: Eugene Loh @ 2025-08-14 16:47 UTC (permalink / raw) To: Kris Van Hees, dtrace, dtrace-devel Reviewed-by: Eugene Loh <eugene.loh@oracle.com> FWIW, with every patch I wonder: "Are there any test issues?" (Like, should we test something here? Were we? Etc.) I assume in this particular case, problems were revealed by patch "Fix various memory leaks related to stapsdt and usdt probes" on tests test/unittest/usdt/tst.pidargmap.sh test/unittest/usdt/tst.pidargs.sh test/unittest/usdt/tst.pidprobes.sh The present patch is simply to remediate those revealed problems. To me, that sort of information is useful in a commit message. On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote: > Only USDT (regular and is-enabled) probes need populate_args(). > > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com> > --- > libdtrace/dt_prov_uprobe.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c > index c63d58383..605ae8397 100644 > --- a/libdtrace/dt_prov_uprobe.c > +++ b/libdtrace/dt_prov_uprobe.c > @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp, > */ > } > > - if (populate_args(dtp, psp, upp) < 0) > - goto fail; > + if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) { > + if (populate_args(dtp, psp, upp) < 0) > + goto fail; > + } > > return uprp; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes 2025-08-14 16:47 ` [DTrace-devel] " Eugene Loh @ 2025-08-14 19:27 ` Kris Van Hees 2025-08-15 6:13 ` Eugene Loh 0 siblings, 1 reply; 5+ messages in thread From: Kris Van Hees @ 2025-08-14 19:27 UTC (permalink / raw) To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel On Thu, Aug 14, 2025 at 12:47:25PM -0400, Eugene Loh wrote: > Reviewed-by: Eugene Loh <eugene.loh@oracle.com> > > FWIW, with every patch I wonder: "Are there any test issues?" (Like, should > we test something here? Were we? Etc.) I assume in this particular case, > problems were revealed by patch "Fix various memory leaks related to stapsdt > and usdt probes" on tests > test/unittest/usdt/tst.pidargmap.sh > test/unittest/usdt/tst.pidargs.sh > test/unittest/usdt/tst.pidprobes.sh > The present patch is simply to remediate those revealed problems. To me, > that sort of information is useful in a commit message. I can add that in the future, though personally I don't find that as valuable because anyone would have run a test prior to this patch and after the patch, would notice that anyway. But if it makes it easier to assess a patch, etc then I can add that in the future. > On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote: > > Only USDT (regular and is-enabled) probes need populate_args(). > > > > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com> > > --- > > libdtrace/dt_prov_uprobe.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c > > index c63d58383..605ae8397 100644 > > --- a/libdtrace/dt_prov_uprobe.c > > +++ b/libdtrace/dt_prov_uprobe.c > > @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp, > > */ > > } > > - if (populate_args(dtp, psp, upp) < 0) > > - goto fail; > > + if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) { > > + if (populate_args(dtp, psp, upp) < 0) > > + goto fail; > > + } > > return uprp; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes 2025-08-14 19:27 ` Kris Van Hees @ 2025-08-15 6:13 ` Eugene Loh 2025-08-15 14:54 ` Kris Van Hees 0 siblings, 1 reply; 5+ messages in thread From: Eugene Loh @ 2025-08-15 6:13 UTC (permalink / raw) To: Kris Van Hees; +Cc: dtrace, dtrace-devel On 8/14/25 15:27, Kris Van Hees wrote: > On Thu, Aug 14, 2025 at 12:47:25PM -0400, Eugene Loh wrote: >> Reviewed-by: Eugene Loh <eugene.loh@oracle.com> >> >> FWIW, with every patch I wonder: "Are there any test issues?" (Like, should >> we test something here? Were we? Etc.) I assume in this particular case, >> problems were revealed by patch "Fix various memory leaks related to stapsdt >> and usdt probes" on tests >> test/unittest/usdt/tst.pidargmap.sh >> test/unittest/usdt/tst.pidargs.sh >> test/unittest/usdt/tst.pidprobes.sh >> The present patch is simply to remediate those revealed problems. To me, >> that sort of information is useful in a commit message. > I can add that in the future, though personally I don't find that as valuable > because anyone would have run a test prior to this patch and after the patch, > would notice that anyway. > > But if it makes it easier to assess a patch, etc then I can add that in the > future. Cool, yes. A person might run the full test suite before and after, but they might not. Anyhow, it does make it easier to review the patch. But the reason for this email is that I seem to be getting test/unittest/usdt/tst.stapsdt-notes-args.sh: FAIL: expected results differ. test/unittest/usdt/tst.stapsdt-notes-isenabled.sh: FAIL: expected results differ. test/unittest/usdt/tst.stapsdt-notes-lib.sh: FAIL: expected results differ. test/unittest/usdt/tst.stapsdt-notes.sh: FAIL: expected results differ. with this patch. >> On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote: >>> Only USDT (regular and is-enabled) probes need populate_args(). >>> >>> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com> >>> --- >>> libdtrace/dt_prov_uprobe.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c >>> index c63d58383..605ae8397 100644 >>> --- a/libdtrace/dt_prov_uprobe.c >>> +++ b/libdtrace/dt_prov_uprobe.c >>> @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp, >>> */ >>> } >>> - if (populate_args(dtp, psp, upp) < 0) >>> - goto fail; >>> + if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) { >>> + if (populate_args(dtp, psp, upp) < 0) >>> + goto fail; >>> + } >>> return uprp; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes 2025-08-15 6:13 ` Eugene Loh @ 2025-08-15 14:54 ` Kris Van Hees 0 siblings, 0 replies; 5+ messages in thread From: Kris Van Hees @ 2025-08-15 14:54 UTC (permalink / raw) To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel On Fri, Aug 15, 2025 at 02:13:33AM -0400, Eugene Loh wrote: > On 8/14/25 15:27, Kris Van Hees wrote: > > > On Thu, Aug 14, 2025 at 12:47:25PM -0400, Eugene Loh wrote: > > > Reviewed-by: Eugene Loh <eugene.loh@oracle.com> > > > > > > FWIW, with every patch I wonder: "Are there any test issues?" (Like, should > > > we test something here? Were we? Etc.) I assume in this particular case, > > > problems were revealed by patch "Fix various memory leaks related to stapsdt > > > and usdt probes" on tests > > > test/unittest/usdt/tst.pidargmap.sh > > > test/unittest/usdt/tst.pidargs.sh > > > test/unittest/usdt/tst.pidprobes.sh > > > The present patch is simply to remediate those revealed problems. To me, > > > that sort of information is useful in a commit message. > > I can add that in the future, though personally I don't find that as valuable > > because anyone would have run a test prior to this patch and after the patch, > > would notice that anyway. > > > > But if it makes it easier to assess a patch, etc then I can add that in the > > future. > > Cool, yes. A person might run the full test suite before and after, but > they might not. Anyhow, it does make it easier to review the patch. > > But the reason for this email is that I seem to be getting > test/unittest/usdt/tst.stapsdt-notes-args.sh: FAIL: expected results > differ. > test/unittest/usdt/tst.stapsdt-notes-isenabled.sh: FAIL: expected > results differ. > test/unittest/usdt/tst.stapsdt-notes-lib.sh: FAIL: expected results > differ. > test/unittest/usdt/tst.stapsdt-notes.sh: FAIL: expected results > differ. > with this patch. Oops - not sure how that passed on my system when testing. Either way, posting fix in a sec. > > > > On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote: > > > > Only USDT (regular and is-enabled) probes need populate_args(). > > > > > > > > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com> > > > > --- > > > > libdtrace/dt_prov_uprobe.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c > > > > index c63d58383..605ae8397 100644 > > > > --- a/libdtrace/dt_prov_uprobe.c > > > > +++ b/libdtrace/dt_prov_uprobe.c > > > > @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp, > > > > */ > > > > } > > > > - if (populate_args(dtp, psp, upp) < 0) > > > > - goto fail; > > > > + if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) { > > > > + if (populate_args(dtp, psp, upp) < 0) > > > > + goto fail; > > > > + } > > > > return uprp; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-08-15 14:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-14 3:58 [PATCH] uprobe: do not call populate_args() for pid probes Kris Van Hees 2025-08-14 16:47 ` [DTrace-devel] " Eugene Loh 2025-08-14 19:27 ` Kris Van Hees 2025-08-15 6:13 ` Eugene Loh 2025-08-15 14:54 ` Kris Van Hees
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox