From owner-svn-src-projects@FreeBSD.ORG Thu Sep 13 15:45:24 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 350EF106564A; Thu, 13 Sep 2012 15:45:24 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 0619D8FC08; Thu, 13 Sep 2012 15:45:24 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 45091B983; Thu, 13 Sep 2012 11:45:23 -0400 (EDT) From: John Baldwin To: attilio@freebsd.org Date: Thu, 13 Sep 2012 11:32:20 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p17; KDE/4.5.5; amd64; ; ) References: <201207301350.q6UDobCI099069@svn.freebsd.org> <201209130910.50876.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201209131132.21103.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Thu, 13 Sep 2012 11:45:23 -0400 (EDT) Cc: Davide Italiano , mlaier@freebsd.org, svn-src-projects@freebsd.org, Konstantin Belousov , src-committers@freebsd.org, Stephan Uphoff Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Sep 2012 15:45:24 -0000 On Thursday, September 13, 2012 10:38:54 am Attilio Rao wrote: > On Thu, Sep 13, 2012 at 2:10 PM, John Baldwin wrote: > > On Wednesday, September 12, 2012 9:36:58 pm Attilio Rao wrote: > >> On Thu, Aug 2, 2012 at 10:07 PM, John Baldwin wrote: > >> > On Thursday, August 02, 2012 4:56:03 pm Attilio Rao wrote: > >> >> On 7/30/12, John Baldwin wrote: > >> >> > --- //depot/projects/smpng/sys/kern/kern_rmlock.c 2012-03-25 > >> >> > 18:45:29.000000000 0000 > >> >> > +++ //depot/user/jhb/lock/kern/kern_rmlock.c 2012-06-18 21:20:58.000000000 > >> >> > 0000 > >> >> > @@ -70,6 +70,9 @@ > >> >> > } > >> >> > > >> >> > static void assert_rm(const struct lock_object *lock, int what); > >> >> > +#ifdef DDB > >> >> > +static void db_show_rm(const struct lock_object *lock); > >> >> > +#endif > >> >> > static void lock_rm(struct lock_object *lock, int how); > >> >> > #ifdef KDTRACE_HOOKS > >> >> > static int owner_rm(const struct lock_object *lock, struct thread > >> >> > **owner); > >> >> > >> >> While here, did you consider also: > >> >> - Abstracting compiler_memory_barrier() into a MI, compiler dependent function? > >> >> - Fix rm_queue with DCPU possibly > >> > > >> > Mostly I just wanted to fill in missing functionality and fixup the > >> > RM_SLEEPABLE bits a bit. > >> > >> So what do you think about the following patch? If you agree I will > >> send to pho@ for testing in a batch with other patches. > > > > It's not super clear to me that having it be static vs dynamic is all that > > big of a deal. However, your approach in general is better, and it certainly > > should have been using PCPU_GET() for the curcpu case all along rather than > > inlining pcpu_find(). > > You mean what is the performance difference between static vs dynamic? > Or you mean, why we want such patch at all? > In the former question there is a further indirection (pc_dynamic > access), for the latter question the patched code avoids namespace > pollution at all and makes the code more readable. More why we want it. I think most of your readability fixes would work just as well if it remained static and we used PCPU_GET(). However, I think your changes are fine. FYI, much of subr_rmlock.c goes out of its way to optimize for performance (such as inlining critical_enter(), critical_exit(), and pcpu_find()), so adding the new indirection goes against the grain of that. -- John Baldwin