[DTrace-devel] [PATCH 11/12] Abort if provider population fails for any provider

[Kris Van Hees]

On Tue, Jan 09, 2024 at 01:44:51AM -0500, Eugene Loh via DTrace-devel wrote:
> It seems to me, we have to go through the providers and clean up error
> returns if we want to do this.
> 
> Most providers seems to have populate() return 0 on error.  E.g.,
> dt_prov_dtrace.c.  It calls dt_provider_create(), which if it runs out of
> memory, calls dt_set_errno(EDT_NOMEM) and returns NULL.  So
> dt_prov_dtrace.c's populate() sees the NULL and then returns 0. This patch
> decides that populate ran just fine.

Hm, I'll have a look because I forgot that there might indeed be cases where
0 probes being reported is technically an error (as it ought to be with the
dtrace provider - though the current change does not change behaviour from
what was done before - ignore it).  So, thank you for catching that the dtrace
provider was actually having a potential case (before this patch also) where
a failure would simply be ignored.

> One solution may be to fix this patch so that n<=0 is an error.
> 
> But we might still want it okay for a provider not to return any probes. 
> E.g., lockstat with a back-rev kernel, etc.  So then it's a matter of going
> through and having providers' populate() functions return -1 when they
> intend error.  E.g., dt_sdt_populate() should.  That would fix ip, lockstat,
> and sched.  Then go through and fix cpc, dtrace, fbt, proc, profile, rawtp,
> sdt, and syscall.

Yes, I agree.  The existing behavious has been to simply ignore any possible
failure in populate which certainly was a bad situation.  But I do need to
audit the other providers to mke this patch actually be effevtive for *all*,
despite the existing (bad) behaviour.

> Anyhow, looks like basically all the other providers need to switch to -1
> for error for this patch to work;  they currently return 0 for error.

Indeed.  Working on v2.

> On 1/5/24 00:31, Kris Van Hees via DTrace-devel wrote:
> > diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> > @@ -1208,6 +1208,9 @@ dtrace_init(dtrace_hdl_t *dtp)
> >   		int	n;
> >   		n = dt_providers[i]->populate(dtp);
> > +		if (n < 0)
> > +			return -1;		/* errno is already set */
> > +
> >   		dt_dprintf("loaded %d probes for %s\n", n,
> >   			   dt_providers[i]->name);
> >   	}
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel