From owner-svn-src-projects@FreeBSD.ORG Fri Nov 2 09:02:22 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7F1A09DC; Fri, 2 Nov 2012 09:02:22 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx1.freebsd.org (Postfix) with ESMTP id DC6CC8FC0C; Fri, 2 Nov 2012 09:02:19 +0000 (UTC) Received: by mail-lb0-f182.google.com with SMTP id b5so3124131lbd.13 for ; Fri, 02 Nov 2012 02:02:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=OsDeg8XfoM8Wdge1n02IxKsyvYnJHjT+7Hsm3VINP3k=; b=DDUy3x71ScsDG7jrrJX7zqNUak550AyY7fUCKiQLxHHz83MOO5pVjvfdqGcRIO5cZb zp/ziJ4+gW6evEucBViAxRoqFZQx4k8FjkM9zJH/3V/5AJA2dkxXACjhvNK1Q+ivXunK F+6GZXpoqI5IiXbuMXE4gnBlycT/rSw+Y9XHkptLRMWeBMZ1ZK8RypQA4l8iuYLB1IcI bb57dLPMEj2/wIHNE0OAZbvwpC41X8mqwKltNLxK4Os+lgXCU+RSdQ8n65PQVo1ip880 NUt/jqtwCXuXxTVGQmPgyuqFYldKRlItWF4LqWHxvnJy7T1pJ3vDidYE8h+G6cluUtg5 mBOA== MIME-Version: 1.0 Received: by 10.152.123.103 with SMTP id lz7mr989898lab.21.1351846938579; Fri, 02 Nov 2012 02:02:18 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.112.30.37 with HTTP; Fri, 2 Nov 2012 02:02:18 -0700 (PDT) In-Reply-To: <20121029155136.O943@besplex.bde.org> References: <201207301350.q6UDobCI099069@svn.freebsd.org> <201207301732.33474.jhb@freebsd.org> <20121029155136.O943@besplex.bde.org> Date: Fri, 2 Nov 2012 09:02:18 +0000 X-Google-Sender-Auth: L_L7V9WrA_rKhW4rpYZsxR_X46Q Message-ID: Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern From: Attilio Rao To: Bruce Evans Content-Type: text/plain; charset=UTF-8 Cc: src-committers@freebsd.org, John Baldwin , Jeff Roberson , Florian Smeets , Bruce Evans , svn-src-projects@freebsd.org X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Nov 2012 09:02:22 -0000 On 10/29/12, Bruce Evans wrote: > On Mon, 29 Oct 2012, Attilio Rao wrote: > >> Now that sched_pin()/sched_unpin() are fixed I would like to introduce >> this new patch, making critical_enter()/critical_exit() inline: >> http://www.freebsd.org/~attilio/inline_critical.patch >> >> The concept is pretty simple: simple add/dec for critical_enter, exit >> are inlined, the rest is in an "hard path". Debugging enables the hard >> paths by default (really I think that only KTR may be due here, but I >> thought that in case of INVARIANTS this was also wanted, so I added >> the check also for that case). >> >> flo@ has stress-tested the patch already and he may be starting >> serious benchmarks soon. >> The effectiveness of this patch is to determine and it brings again >> the question: better an hard function or the usage of compiler membars >> to avoid issues? I think that this patch should be in only if we can >> absolutely prove a measurable performance bump, otherwise I will just >> add a note to critical_enter()/critical_exit() explaining why they >> should not be inlined at all. > > Inlining of mtx_lock_spin() is bogus unless critical_enter() is inlined. > Similarly for mtx_unlock_spin() and critical_exit(). It saves 1 function > call. but critical_enter() does a function call anyway. critical_exit*( > also has a branch in branch in it that might cost more than the function > call just for mispredicting it. > > My version goes the other way and uninlines mtx_lock_spin() and > mtx_unlock_spin(). Then it inlines (open codes) critical_enter() and > critical_exit() in them. This saves a lot of text space and thus > hopefully saves time too. I couldn't find any cases where it either > saved or lost a significant amount of time. The main cause of time > differences is probably mispredicted branches: with mtx*() inlined, > the branches in it can be predicted separately, so they are more > predictable. However, when they are separate, there may be enough to > thrash the branch predictor. It is probably best to only inline mtx*() > for a few heavily-used locks and only inline critical*() if it is either > in one of these or is in a non-inline function. Otherwise, inlining > mainly gives code bloat that is only harmless if the code is rarely > actually used. But this is hard to configure. is > already a mess to support uninlining for debugging cases. So, I thought more about this and I think that inlining critical_exit() is not really going to bring any benefit here but bloat. This is because nested critical sections are rare rather not, which means you will end up in doing the function call most of the time and plus we have the possible pessimization introduced by the memory clobber (__compiler_membar()) and as you note possible deficiency caming from the branch misprediction*. All that to say that I shrunk my patch to only inline critical_enter(). Respect v1, this patch also does use the fast path in all cases but KTR, does use a common accessor function for both hard and soft versions to bump the counter. In theory inlined critical_enter() should be a net win, but I need to carefully evaluate the difference introduced by the __compiler_membar(), a case very similar to sched_pin(). I will start doing some investigations soon but they would require some good effort for both the widespread nature of critical_enter() and the difference in code gcc will certainly produce because of a less function call and the memory clobber. New patch here: http://www.freebsd.org/~attilio/inline_critical2.patch I feel very unsure style-wise about my add to sys/systm.h because it is already very messy that I cannot give a logical sense to that chunk. Thanks, Attilio * In theory we could use __predict_false() or similar but I don't think this will really help as x86 doesn't have explicit instructions to control branch prediction -- Peace can only be achieved by understanding - A. Einstein