Linux DTrace development list
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: Eugene Loh <eugene.loh@oracle.com>
Cc: Kris Van Hees <kris.van.hees@oracle.com>,
	dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes
Date: Thu, 14 Aug 2025 15:27:10 -0400	[thread overview]
Message-ID: <aJ44jo+ogadLIWau@oracle.com> (raw)
In-Reply-To: <8e075b06-1d39-3d9c-3924-61e948827485@oracle.com>

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;

  reply	other threads:[~2025-08-14 19:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2025-08-15  6:13     ` Eugene Loh
2025-08-15 14:54       ` Kris Van Hees

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJ44jo+ogadLIWau@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox