From owner-svn-src-head@FreeBSD.ORG Mon Aug 19 20:14:03 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 471B87B0; Mon, 19 Aug 2013 20:14:03 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qa0-x22a.google.com (mail-qa0-x22a.google.com [IPv6:2607:f8b0:400d:c00::22a]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id B4278266A; Mon, 19 Aug 2013 20:14:02 +0000 (UTC) Received: by mail-qa0-f42.google.com with SMTP id bv4so2079271qab.1 for ; Mon, 19 Aug 2013 13:14:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=ny/fjoQV2xtJGYZ59XB6ZrRjPd7Tk74j2v3sf8Cd47E=; b=OZMVF+cgz/Nqk/T8n4Y7hKSoP1pmIrZijyQr4Miai2HOl6rXPHcCeU19g5H740bVov LjQ+BngkxcY5ZZ8wOd8/J42ovo1DCQLfRgIt4gPhUZiouThXkF3+NPHo2uLQ7ziqYdDQ 0x8Xl1Vb8bNyyckAB9L0eHjV5UY9wDaDMqq3ng2SvdKlv5JGYyIWy25evu9KcbUY8ZPo IEbb+/8pJMLrBITZRhBdcF5w/GcrLqrGcDgxcPcM7fUj34eKXm9Akeo9W7R4S/CfqZgH gQQ6hz6hon4BWekKATMeW8YBO0Cmv/DjxB7huZAtrN8x6UJTYsDAx1vWd0bDD5dLQ080 Pl7g== X-Received: by 10.49.73.227 with SMTP id o3mr17741891qev.23.1376943241904; Mon, 19 Aug 2013 13:14:01 -0700 (PDT) Received: from charmander.sandvine.com ([64.7.137.182]) by mx.google.com with ESMTPSA id m5sm19002473qaa.13.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 19 Aug 2013 13:14:01 -0700 (PDT) Sender: Mark Johnston Date: Mon, 19 Aug 2013 16:14:45 -0400 From: Mark Johnston To: John Baldwin 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: <20130819201445.GB4765@charmander.sandvine.com> References: <201308140042.r7E0gMtf054550@svn.freebsd.org> <20130816174131.GB1888@charmander.sandvine.com> <201308191542.30797.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201308191542.30797.jhb@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) 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 20:14:03 -0000 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). -Mark