From owner-svn-src-head@FreeBSD.ORG Tue Aug 20 13:50:58 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 3AFDF53E; Tue, 20 Aug 2013 13:50:58 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 0BDC6209A; Tue, 20 Aug 2013 13:50:58 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 52AA5B924; Tue, 20 Aug 2013 09:50:55 -0400 (EDT) From: John Baldwin To: Mark Johnston 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 Date: Tue, 20 Aug 2013 08:14:26 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p28; KDE/4.5.5; amd64; ; ) References: <201308140042.r7E0gMtf054550@svn.freebsd.org> <201308191542.30797.jhb@freebsd.org> <20130819201445.GB4765@charmander.sandvine.com> In-Reply-To: <20130819201445.GB4765@charmander.sandvine.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201308200814.26924.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 20 Aug 2013 09:50:55 -0400 (EDT) Cc: Davide Italiano , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Aug 2013 13:50:58 -0000 On Monday, August 19, 2013 4:14:45 pm Mark Johnston wrote: > On Mon, Aug 19, 2013 at 03:42:30PM -0400, John Baldwin wrote: > > On Friday, August 16, 2013 1:41:31 pm Mark Johnston wrote: > > > On Fri, Aug 16, 2013 at 07:13:16PM +0200, Davide Italiano wrote: > > > > [trim old mails] > > > > > > > > > diff --git a/sys/sys/pmckern.h b/sys/sys/pmckern.h > > > > > index e3e18a6..90585de 100644 > > > > > --- a/sys/sys/pmckern.h > > > > > +++ b/sys/sys/pmckern.h > > > > > @@ -51,13 +51,11 @@ > > > > > #define PMC_FN_CSW_IN 2 > > > > > #define PMC_FN_CSW_OUT 3 > > > > > #define PMC_FN_DO_SAMPLES 4 > > > > > -#define PMC_FN_KLD_LOAD 5 > > > > > -#define PMC_FN_KLD_UNLOAD 6 > > > > > -#define PMC_FN_MMAP 7 > > > > > -#define PMC_FN_MUNMAP 8 > > > > > -#define PMC_FN_USER_CALLCHAIN 9 > > > > > -#define PMC_FN_USER_CALLCHAIN_SOFT 10 > > > > > -#define PMC_FN_SOFT_SAMPLING 11 > > > > > +#define PMC_FN_MMAP 5 > > > > > +#define PMC_FN_MUNMAP 6 > > > > > +#define PMC_FN_USER_CALLCHAIN 7 > > > > > +#define PMC_FN_USER_CALLCHAIN_SOFT 8 > > > > > +#define PMC_FN_SOFT_SAMPLING 9 > > > > > > > > > > > > > I've skimmed over your patch quickly so I could miss something, but I > > > > worry about this change breaking the KBI. > > > > Does this make sense for you? > > > > > > I think you're right. I considered this last night, but it didn't occur > > > to me that external modules might try to invoke these hooks. I'm not > > > sure if such modules exist, but it's better to be safe. I updated the > > > patch here: > > > > > > http://people.freebsd.org/~markj/patches/hwpmc-eh/hwpmc-eh-3.diff > > > > This generally looks correct. I would probably not call it > > KLD_LOCK_ASSERT_READ() as it asserts any lock (not specifically a > > read lock). Normally I would use macros like this: > > > > KLD_WLOCK/WUNLOCK > > KLD_RLOCK/RUNLOCK > > KLD_ASSERT_LOCKED/KLD_ASSERT_WLOCKED > > > > However, the existing macros all were named to not assume read > > locking and then some read locking was added on the side. I'm > > not sure exactly what macro name makes the most sense, and would > > in fact be tempted to just retire all the KLD_* macros for locking > > and use sx(9) directly, but that is a fairly minor point. (And if > > done should be a separate commit). > > Yeah, KLD_LOCK_ASSERT_READ() is awkward, but I couldn't come up with > anything better without renaming the existing macros. I'm fine with > getting rid of them and using the sx_* calls directly; it'll be the same > amount of churn as changing the macro names anyway. > > So how about this: I'll get rid of the macros in one commit, change the > locking rules for linker_file_lookup_set() using my existing patch but > without the KLD lock macros, and then convert the hwpmc(4) hooks to use > the new event handlers (making sure to avoid breaking the KBI as Davide > pointed out). That sounds great to me. Thanks! -- John Baldwin