From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754887AbbDIIDs (ORCPT <rfc822;w@1wt.eu>);
	Thu, 9 Apr 2015 04:03:48 -0400
Received: from bombadil.infradead.org ([198.137.202.9]:35508 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751699AbbDIIDq (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 9 Apr 2015 04:03:46 -0400
Date: Thu, 9 Apr 2015 10:03:32 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
        Viresh Kumar <viresh.kumar@linaro.org>, Ingo Molnar <mingo@redhat.com>,
        linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org,
        Preeti U Murthy <preeti@linux.vnet.ibm.com>
Subject: Re: [PATCH] hrtimer: Replace cpu_base->active_bases with a direct
 check of the active list
Message-ID: <20150409080332.GL5029@twins.programming.kicks-ass.net>
References: <cover.1428039899.git.viresh.kumar@linaro.org>
 <b884675ccbd86069d7e9ff6b0de35760c5d6a959.1428039899.git.viresh.kumar@linaro.org>
 <alpine.DEB.2.11.1504082019220.3845@nanos>
 <20150409062841.GB14259@gmail.com>
 <20150409065730.GK27490@worktop.programming.kicks-ass.net>
 <20150409070917.GF14259@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150409070917.GF14259@gmail.com>
User-Agent: Mutt/1.5.21 (2012-12-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Apr 09, 2015 at 09:09:17AM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Thu, Apr 09, 2015 at 08:28:41AM +0200, Ingo Molnar wrote:
> > > Btw., does cpu_base->active_bases even make sense? hrtimer bases are 
> > > fundamentally percpu, and to check whether there are any pending 
> > > timers is a very simple check:
> > > 
> > > 	base->active->next != NULL
> > > 
> > 
> > Yeah, that's 3 pointer dereferences from cpu_base, iow you traded a 
> > single bit test on an already loaded word for 3 potential cacheline 
> > misses.
> 
> But the clock bases are not aligned to cachelines, and we have 4 of 
> them. So in practice when we access one, we'll load the next one 
> anyway.

$ pahole -C hrtimer_clock_base defconfig-build/kernel/time/timer.o 
struct hrtimer_clock_base {
        struct hrtimer_cpu_base *  cpu_base;             /*     0     8 */
        int                        index;                /*     8     4 */
        clockid_t                  clockid;              /*    12     4 */
        struct timerqueue_head     active;               /*    16    16 */
        ktime_t                    resolution;           /*    32     8 */
        ktime_t                    (*get_time)(void);    /*    40     8 */
        ktime_t                    softirq_time;         /*    48     8 */
        ktime_t                    offset;               /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */

        /* size: 64, cachelines: 1, members: 8 */
};

They _should_ be aligned :-)

> Furthermore the simplification is measurable, and a fair bit of it is 
> in various fast paths. I'd rather trade a bit of a cacheline footprint 
> for less overall complexity and faster code.

cacheline misses hurt a lot, and the bitmask isn't really complex.