From owner-svn-src-head@FreeBSD.ORG Mon Aug 19 19:42:43 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 30C67C23; Mon, 19 Aug 2013 19:42:43 +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 B62F32483; Mon, 19 Aug 2013 19:42:42 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 6438FB922; Mon, 19 Aug 2013 15:42:40 -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: Mon, 19 Aug 2013 15:42:30 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p28; KDE/4.5.5; amd64; ; ) References: <201308140042.r7E0gMtf054550@svn.freebsd.org> <20130816174131.GB1888@charmander.sandvine.com> In-Reply-To: <20130816174131.GB1888@charmander.sandvine.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201308191542.30797.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 19 Aug 2013 15:42:40 -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: Mon, 19 Aug 2013 19:42:43 -0000 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). Thanks for doing this! -- John Baldwin