Date: Wed, 14 Aug 2013 17:11:17 -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: <201308141711.17660.jhb@freebsd.org> In-Reply-To: <CAMw1wOzjK_eK4FH_wf2pNDWZPJ30qpcY3%2B0C1=vixVm4A4skTw@mail.gmail.com> References: <201308140042.r7E0gMtf054550@svn.freebsd.org> <201308141548.11407.jhb@freebsd.org> <CAMw1wOzjK_eK4FH_wf2pNDWZPJ30qpcY3%2B0C1=vixVm4A4skTw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, August 14, 2013 5:06:06 pm Mark Johnston wrote: > On Wed, Aug 14, 2013 at 3:48 PM, John Baldwin <jhb@freebsd.org> wrote: > > > 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. > > > > Sorry if I'm being dense, but I thought it wasn't generally possible to > determine > whether curthread holds a given shared lock. Fair enough. We could probably either add _locked variant that asserts either one (you can do an assert for a shared lock), or just require all callers to get the lock and make the non _locked variant basically be the _locked variant. I would prefer the latter if it is not too invasive. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308141711.17660.jhb>