Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 13 Jul 2013 19:42:00 -0400
From:      Mark Johnston <markj@freebsd.org>
To:        Mikolaj Golub <trociny@FreeBSD.org>
Cc:        freebsd-dtrace@freebsd.org
Subject:   Re: [RFC] reworking FreeBSD's SDT implementation
Message-ID:  <20130713234200.GA40803@raichu>
In-Reply-To: <20130711210215.GB7506@gmail.com>
References:  <20130703041023.GA82673@raichu> <20130711024500.GA67976@raichu> <20130711210215.GB7506@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 12, 2013 at 12:02:16AM +0300, Mikolaj Golub wrote:
> Hi Mark,
> 
> On Wed, Jul 10, 2013 at 10:45:08PM -0400, Mark Johnston wrote:
> 
> > The final draft of my patch is here:
> > http://people.freebsd.org/~markj/patches/sdt-module-info/20130710-sdt-module-info.diff
> > 
> > I've committed the mod_lock stuff and man page separately. If no one has
> > any objections, I'll commit the patch sometime this weekend.
> > Review and comments are still very welcome!
> 
> One thing noticed. I don't think '#if defined(sun)' directives in
> dtrace.c are correct now:
> 
> > -#if defined(sun)
> >  /*
> >   * DTrace Hook Functions
> >   */
> >  static void
> > -dtrace_module_loaded(modctl_t *ctl)
> > +dtrace_module_loaded(struct linker_file *lf)
> 
> ctl is unconditionally removed
> 
> >  {
> >  	dtrace_provider_t *prv;
> > 
> > @@ -15166,14 +15150,16 @@ dtrace_module_loaded(modctl_t *ctl)
> >  	mutex_enter(&mod_lock);
> >  #endif
> > 
> > +#if defined(sun)
> >  	ASSERT(ctl->mod_busy);
> > +#endif
> 
> while here it is used if sun is defined.
> 
> > 
> >  	/*
> >  	 * We're going to call each providers per-module provide operation
> >  	 * specifying only this module.
> >  	 */
> >  	for (prv = dtrace_provider; prv != NULL; prv = prv->dtpv_next)
> > -		prv->dtpv_pops.dtps_provide_module(prv->dtpv_arg, ctl);
> > +		prv->dtpv_pops.dtps_provide_module(prv->dtpv_arg, lf);
> 
> And this looks wrong for sun case.
> 
> The same issues are in dtrace_module_unloaded().

Thanks for pointing this out - I've fixed these problems in the
following diff:

http://people.freebsd.org/~markj/patches/sdt-module-info/20130713-sdt-module-info.diff

It turns out that we typedef modctl_t to struct linker_file, so I don't
need to have separate cases for defined(sun) and !defined(sun) in a few
places.

-Mark



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130713234200.GA40803>