From owner-svn-src-projects@FreeBSD.ORG Mon Oct 29 13:06:23 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 5F58741F; Mon, 29 Oct 2012 13:06:23 +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 DA39D8FC08; Mon, 29 Oct 2012 13:06:21 +0000 (UTC) Received: by mail-lb0-f182.google.com with SMTP id b5so3870627lbd.13 for ; Mon, 29 Oct 2012 06:06:20 -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=MH71nGbLyyvJwMcW66tOhPSzMICW/sOienKvRAZ9sfc=; b=RGxM+pCaFKIMs709XCs/n9Xtx4MKOxqsmDyS4bSO89Xg10mlUsHSCuMUwgrIWOC29w GFiGFXLJtUB6ngfshE/YKkdkrsMOwqIWg2GTccKItb8vwo1MQ9yTjNLFSMnUIidTt+V3 KFXQ4yUY0WtF9xfL1ZbEqN1caVFxTCgo0DI9dglvtnZrD1XUtOhcnhZNoZfsDxi7xv3O zW4Nw7F9m5GmdYN3x6N3nmqyRHvopA91+7LmMh/U/rhpicC+lDp9sgOcnSc9g7IQhubP BbhrvjLt/1mPmkyvtsWFfHhTSjhQzxfH+bb3LSSiJjpdOjlZduO6pUimP5M91SLX7tzJ +7jw== MIME-Version: 1.0 Received: by 10.112.41.36 with SMTP id c4mr11283274lbl.75.1351515980608; Mon, 29 Oct 2012 06:06:20 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.112.30.37 with HTTP; Mon, 29 Oct 2012 06:06:20 -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: Mon, 29 Oct 2012 13:06:20 +0000 X-Google-Sender-Auth: p2RzMen0pQE3PB1L6tUJ4zllo34 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: Mon, 29 Oct 2012 13:06:23 -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. Correct, that is a further argument for having inlined critical_enter(), even if the actual calling cames from spinlock_enter(), not critical_enter(), which must be MD (that's on FreeBSD, not sure what happens in your OS). > 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. > > OTOH, I couldn't get uninlining of mtx_lock() and mtx_unlock() to work. > These are a little smaller than the spinlock versions, mainly since they > don't inline critical*(). The loss from unlining them cannot be > compensated for by uninlining critical*() in them, and I found just 1 > workload where uninlining them was slower: udp packets per second bandwidth > tests. I don't think that uninling mtx_lock()/unlock() (btw, on which hw are you testing them if you are still able to catch performance penalties by branch misprediction?!) is a good idea, likely what would be a better one is to both inline critical_enter() and spinlock_enter(). Thanks, Attilio -- Peace can only be achieved by understanding - A. Einstein