svn commit: r254309 - in head: share/man/man9 sys/cddl/contrib/opensolaris/uts/common/dtrace sys/cddl/dev/dtrace sys/cddl/dev/sdt sys/kern sys/sys

[John Baldwin]

On Wednesday, August 14, 2013 2:14:53 pm Mark Johnston wrote:
> On Wed, Aug 14, 2013 at 8:19 AM, John Baldwin <jhb at freebsd.org> wrote:
> 
> > On Tuesday, August 13, 2013 8:42:22 pm Mark Johnston wrote:
> > > Author: markj
> > > Date: Wed Aug 14 00:42:21 2013
> > > New Revision: 254309
> > > URL: http://svnweb.freebsd.org/changeset/base/254309
> > >
> > > Log:
> > >   Use kld_{load,unload} instead of mod_{load,unload} for the linker file
> > load
> > >   and unload event handlers added in r254266.
> > >
> > >   Reported by:        jhb
> > >   X-MFC with: r254266
> >
> > Thanks!  BTW, it would be really nice to replace HWPMC_HOOKS in
> > kern_linker.c with
> > EVENTHANDLER calls.  I think kld_load would just work (though you might
> > need to
> > downgrade the lock before you run it).  For kld_unload it seems you want
> > two events,
> > a kld_unload_try for your newly added event (since it can reject a
> > kld_unload), and
> > perhaps kld_unload at the end where the current HWPMC_HOOK is.  Just an
> > idea if
> > someone is looking for something to do.  I know there are other modules
> > that need
> > to hook into linker events like this, and making HWPMC_HOOKS more generic
> > would be
> > a big help.
> >
> 
> I will look into doing this. The DTrace SDT kld_load handler will not work
> properly if the
> linker lock is downgraded first because of the following code in
> linker_file_lookup_set():
> 
> locked = KLD_LOCK();
> if (!locked)
>         KLD_LOCK();
> 
> In particular, it checks to see if the kld lock is exclusively held and
> locks it if not, which
> obviously causes problems if the thread holds the shared lock.
> 
> The answer might be to just run the hwpmc handler with the exclusive lock
> held. Or perhaps
> we just need a linker_file_lookup_set_locked(), assuming that it's ok to
> look up a linker set
> with the shared lock held.

It is probably fine to do a lookup with a shared lock.  We could also fix the
linker code to only lock if there is not an existing shared or exclusive lock.

-- 
John Baldwin