Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 Aug 2013 15:48:11 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Mark Johnston <markj@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: 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
Message-ID:  <201308141548.11407.jhb@freebsd.org>
In-Reply-To: <CAMw1wOyogNU0Tzj26B=3uZ3Tf-jQ=QoJmgvar5s2GMDEgUbk2g@mail.gmail.com>
References:  <201308140042.r7E0gMtf054550@svn.freebsd.org> <201308140819.13854.jhb@freebsd.org> <CAMw1wOyogNU0Tzj26B=3uZ3Tf-jQ=QoJmgvar5s2GMDEgUbk2g@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, August 14, 2013 2:14:53 pm Mark Johnston wrote:
> On Wed, Aug 14, 2013 at 8:19 AM, John Baldwin <jhb@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



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