From patchwork Wed Apr 1 14:06:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mason X-Patchwork-Id: 6140531 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8EE599F2EC for ; Wed, 1 Apr 2015 14:10:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6669B202D1 for ; Wed, 1 Apr 2015 14:10:45 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 68F21202A1 for ; Wed, 1 Apr 2015 14:10:44 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YdJIi-0001cu-OK; Wed, 01 Apr 2015 14:07:24 +0000 Received: from smtp2-g21.free.fr ([212.27.42.2]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YdJIa-0001RH-Kr for linux-arm-kernel@lists.infradead.org; Wed, 01 Apr 2015 14:07:20 +0000 Received: from [172.27.0.114] (unknown [83.142.147.193]) (Authenticated sender: shill) by smtp2-g21.free.fr (Postfix) with ESMTPSA id CDB134B01FD; Wed, 1 Apr 2015 16:04:52 +0200 (CEST) Message-ID: <551BFB77.50403@free.fr> Date: Wed, 01 Apr 2015 16:06:47 +0200 From: Mason User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:35.0) Gecko/20100101 Firefox/35.0 SeaMonkey/2.32.1 MIME-Version: 1.0 To: Russell King - ARM Linux Subject: Re: Cache line size definition in arch/arm/mm/Kconfig References: <5512C7A4.3000302@free.fr> <5515423E.4020802@free.fr> <20150327120601.GB4019@n2100.arm.linux.org.uk> <55155EE9.6020600@free.fr> <551BD9BF.8090808@free.fr> <20150401115013.GA24899@n2100.arm.linux.org.uk> In-Reply-To: <20150401115013.GA24899@n2100.arm.linux.org.uk> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150401_070716_839469_622CB29D X-CRM114-Status: GOOD ( 16.26 ) X-Spam-Score: -0.7 (/) Cc: Thomas Gleixner , Ingo Molnar , Linux ARM , Nicolas Pitre X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 01/04/2015 13:50, Russell King - ARM Linux wrote: > On Wed, Apr 01, 2015 at 01:42:55PM +0200, Mason wrote: >> Example patch for illustration purposes (only compile-tested) >> >> (There is probably a much more elegant way to get 32-byte aligned >> memory allocations.) >> >> diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c >> index c0e2512..77b91c5 100644 >> --- a/drivers/clocksource/mmio.c >> +++ b/drivers/clocksource/mmio.c >> @@ -10,34 +10,24 @@ >> #include >> #include >> -struct clocksource_mmio { >> - void __iomem *reg; >> - struct clocksource clksrc; > > Just swap the order of these. (Assuming arm32 && ARM_L1_CACHE_SHIFT = 6) Because struct clocksource has attribute ____cacheline_aligned, sizeof(struct clocksource) = 128 Thus the layout for struct { void __iomem *reg; struct clocksource clksrc; } would be reg(4) pad1(60) clksrc(80) pad2(48) and the layout for struct { struct clocksource clksrc; void __iomem *reg; } would be clksrc(80) pad2(48) reg(4) pad1(60) If kmalloc returns 32-byte aligned pointer (which seems to be the case) then clksrc is correctly aligned in both cases. The reason for stuffing reg inside struct clocksource would be to avoid the 64-byte overhead from the padding. Also (but this only a matter of taste) it seems the code is marginally clearer without the "container_of" gymnastics. Regards. diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c index c0e2512..d6583ed 100644 --- a/drivers/clocksource/mmio.c +++ b/drivers/clocksource/mmio.c @@ -10,34 +10,24 @@ #include #include -struct clocksource_mmio { - void __iomem *reg; - struct clocksource clksrc; -}; - -static inline struct clocksource_mmio *to_mmio_clksrc(struct clocksource *c) -{ - return container_of(c, struct clocksource_mmio, clksrc); -} - cycle_t clocksource_mmio_readl_up(struct clocksource *c) { - return readl_relaxed(to_mmio_clksrc(c)->reg); + return readl_relaxed(c->reg); } cycle_t clocksource_mmio_readl_down(struct clocksource *c) { - return ~readl_relaxed(to_mmio_clksrc(c)->reg); + return ~readl_relaxed(c->reg); } cycle_t clocksource_mmio_readw_up(struct clocksource *c) { - return readw_relaxed(to_mmio_clksrc(c)->reg); + return readw_relaxed(c->reg); } cycle_t clocksource_mmio_readw_down(struct clocksource *c) { - return ~(unsigned)readw_relaxed(to_mmio_clksrc(c)->reg); + return ~(unsigned)readw_relaxed(c->reg); } /** @@ -53,21 +43,21 @@ int __init clocksource_mmio_init(void __iomem *base, const char *name, unsigned long hz, int rating, unsigned bits, cycle_t (*read)(struct clocksource *)) { - struct clocksource_mmio *cs; + struct clocksource *cs; if (bits > 32 || bits < 16) return -EINVAL; - cs = kzalloc(sizeof(struct clocksource_mmio), GFP_KERNEL); + cs = kzalloc(sizeof *cs, GFP_KERNEL); if (!cs) return -ENOMEM; cs->reg = base; - cs->clksrc.name = name; - cs->clksrc.rating = rating; - cs->clksrc.read = read; - cs->clksrc.mask = CLOCKSOURCE_MASK(bits); - cs->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS; + cs->name = name; + cs->rating = rating; + cs->read = read; + cs->mask = CLOCKSOURCE_MASK(bits); + cs->flags = CLOCK_SOURCE_IS_CONTINUOUS; - return clocksource_register_hz(&cs->clksrc, hz); + return clocksource_register_hz(cs, hz); } diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h index 879065d..9fdb5cf 100644 --- a/include/linux/clocksource.h +++ b/include/linux/clocksource.h @@ -189,6 +189,9 @@ struct clocksource { unsigned long flags; void (*suspend)(struct clocksource *cs); void (*resume)(struct clocksource *cs); +#ifdef CONFIG_CLKSRC_MMIO + void __iomem *reg; +#endif /* private: */ #ifdef CONFIG_CLOCKSOURCE_WATCHDOG