* Cache line size definition in arch/arm/mm/Kconfig
@ 2015-03-25 14:35 Mason
2015-03-27 11:42 ` Mason
0 siblings, 1 reply; 7+ messages in thread
From: Mason @ 2015-03-25 14:35 UTC (permalink / raw)
To: linux-arm-kernel
Hello everyone,
AFAICT, L1 cache line size is specified in arch/arm/mm/Kconfig
config ARM_L1_CACHE_SHIFT_6
bool
default y if CPU_V7
help
Setting ARM L1 cache line size to 64 Bytes.
config ARM_L1_CACHE_SHIFT
int
default 6 if ARM_L1_CACHE_SHIFT_6
default 5
I'm using a Cortex A9 MPCore. If I'm not mistaken, the cache line size
is 32 bytes, even though this CPU is ARMv7.
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388g/Caccifbd.html
> The Cortex-A9 processor has separate instruction and data caches.
> The caches have the following features:
>
> Each cache can be disabled independently. See System Control Register.
> Both caches are 4-way set-associative.
> The cache line length is eight words.
> On a cache miss, critical word first filling of the cache is performed.
> You can configure the instruction and data caches independently during implementation to sizes of 16KB, 32KB, or 64KB.
> To reduce power consumption, the number of full cache reads is reduced by taking advantage of the sequential nature of many cache operations. If a cache read is sequential to the previous cache read, and the read is within the same cache line, only the data RAM set that was previously read is accessed.
How do I set ARM_L1_CACHE_SHIFT_6 to 'n' in my platform Kconfig?
Or perhaps I should "override" ARM_L1_CACHE_SHIFT to 5 (again in
my platform Kconfig). I don't know the syntax to do that.
Could someone point out the correct way?
Regards.
^ permalink raw reply [flat|nested] 7+ messages in thread* Cache line size definition in arch/arm/mm/Kconfig 2015-03-25 14:35 Cache line size definition in arch/arm/mm/Kconfig Mason @ 2015-03-27 11:42 ` Mason 2015-03-27 12:06 ` Russell King - ARM Linux 0 siblings, 1 reply; 7+ messages in thread From: Mason @ 2015-03-27 11:42 UTC (permalink / raw) To: linux-arm-kernel On 25/03/2015 15:35, Mason wrote: > AFAICT, L1 cache line size is specified in arch/arm/mm/Kconfig > > config ARM_L1_CACHE_SHIFT_6 > bool > default y if CPU_V7 > help > Setting ARM L1 cache line size to 64 Bytes. > > config ARM_L1_CACHE_SHIFT > int > default 6 if ARM_L1_CACHE_SHIFT_6 > default 5 > > > I'm using a Cortex A9 MPCore. If I'm not mistaken, the cache line size > is 32 bytes, even though this CPU is ARMv7. > > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388g/Caccifbd.html > >> The Cortex-A9 processor has separate instruction and data caches. >> The caches have the following features: >> >> Each cache can be disabled independently. See System Control Register. >> Both caches are 4-way set-associative. >> The cache line length is eight words. >> On a cache miss, critical word first filling of the cache is performed. >> You can configure the instruction and data caches independently during implementation to sizes of 16KB, 32KB, or 64KB. >> To reduce power consumption, the number of full cache reads is reduced by taking advantage of the sequential nature of many cache operations. If a cache read is sequential to the previous cache read, and the read is within the same cache line, only the data RAM set that was previously read is accessed. > > How do I set ARM_L1_CACHE_SHIFT_6 to 'n' in my platform Kconfig? > > Or perhaps I should "override" ARM_L1_CACHE_SHIFT to 5 (again in > my platform Kconfig). I don't know the syntax to do that. > > Could someone point out the correct way? Would someone care to comment? :-) Someone mentioned that some implementations prefetch 64 bytes from L2D into L1D, so it might make sense to align "hot" data to 64 bytes to benefit from this mechanism? It seems we might waste a few hundred/thousand bytes if aligning to cache line is a frequent operation, though. On a tangential note, it seems likely that Linux could determine the size of L1 at run-time, by poking the right implementation-specific register. Is it not better to get the information at run-time, rather than as a compile-time constant? (I have the same general question about PERIPH_BASE, patch incoming.) Thoughts? Regards. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Cache line size definition in arch/arm/mm/Kconfig 2015-03-27 11:42 ` Mason @ 2015-03-27 12:06 ` Russell King - ARM Linux 2015-03-27 13:45 ` Mason 0 siblings, 1 reply; 7+ messages in thread From: Russell King - ARM Linux @ 2015-03-27 12:06 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 27, 2015 at 12:42:54PM +0100, Mason wrote: > On 25/03/2015 15:35, Mason wrote: > > >AFAICT, L1 cache line size is specified in arch/arm/mm/Kconfig > > > >config ARM_L1_CACHE_SHIFT_6 > > bool > > default y if CPU_V7 > > help > > Setting ARM L1 cache line size to 64 Bytes. > > > >config ARM_L1_CACHE_SHIFT > > int > > default 6 if ARM_L1_CACHE_SHIFT_6 > > default 5 > > > > > >I'm using a Cortex A9 MPCore. If I'm not mistaken, the cache line size > >is 32 bytes, even though this CPU is ARMv7. > > > >http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388g/Caccifbd.html > > > >>The Cortex-A9 processor has separate instruction and data caches. > >>The caches have the following features: > >> > >> Each cache can be disabled independently. See System Control Register. > >> Both caches are 4-way set-associative. > >> The cache line length is eight words. > >> On a cache miss, critical word first filling of the cache is performed. > >> You can configure the instruction and data caches independently during implementation to sizes of 16KB, 32KB, or 64KB. > >> To reduce power consumption, the number of full cache reads is reduced by taking advantage of the sequential nature of many cache operations. If a cache read is sequential to the previous cache read, and the read is within the same cache line, only the data RAM set that was previously read is accessed. > > > >How do I set ARM_L1_CACHE_SHIFT_6 to 'n' in my platform Kconfig? > > > >Or perhaps I should "override" ARM_L1_CACHE_SHIFT to 5 (again in > >my platform Kconfig). I don't know the syntax to do that. > > > >Could someone point out the correct way? > > Would someone care to comment? :-) No :) What you've found is the _static_ L1 cache line size setting, which is used at _compile_ time to align structures while building. To allow maximum flexibility - and because there are 64-byte cache line ARMv7 implementations around - we have decided that the _compile time_ cache line size will be 64 bytes. As far as cache operations are concerned, they will know the correct cache line size for the CPU which they're running on, so the code will adapt. It has the side effect that some allocators also assume that the L1 cache line size is 64 bytes. It's better to have a larger than necessary cache line size than a smaller one, because a larger one is automatically aligned to the smaller sizes. In other words, this is totally intentional. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Cache line size definition in arch/arm/mm/Kconfig 2015-03-27 12:06 ` Russell King - ARM Linux @ 2015-03-27 13:45 ` Mason 2015-04-01 11:42 ` Mason 0 siblings, 1 reply; 7+ messages in thread From: Mason @ 2015-03-27 13:45 UTC (permalink / raw) To: linux-arm-kernel On 27/03/2015 13:06, Russell King - ARM Linux wrote: > On Fri, Mar 27, 2015 at 12:42:54PM +0100, Mason wrote: >> On 25/03/2015 15:35, Mason wrote: >> >>> AFAICT, L1 cache line size is specified in arch/arm/mm/Kconfig >>> >>> config ARM_L1_CACHE_SHIFT_6 >>> bool >>> default y if CPU_V7 >>> help >>> Setting ARM L1 cache line size to 64 Bytes. >>> >>> config ARM_L1_CACHE_SHIFT >>> int >>> default 6 if ARM_L1_CACHE_SHIFT_6 >>> default 5 >>> >>> >>> I'm using a Cortex A9 MPCore. If I'm not mistaken, the cache line size >>> is 32 bytes, even though this CPU is ARMv7. >>> >>> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388g/Caccifbd.html >>> >>>> The Cortex-A9 processor has separate instruction and data caches. >>>> The caches have the following features: >>>> >>>> Each cache can be disabled independently. See System Control Register. >>>> Both caches are 4-way set-associative. >>>> The cache line length is eight words. >>>> On a cache miss, critical word first filling of the cache is performed. >>>> You can configure the instruction and data caches independently during implementation to sizes of 16KB, 32KB, or 64KB. >>>> To reduce power consumption, the number of full cache reads is reduced by taking advantage of the sequential nature of many cache operations. If a cache read is sequential to the previous cache read, and the read is within the same cache line, only the data RAM set that was previously read is accessed. >>> >>> How do I set ARM_L1_CACHE_SHIFT_6 to 'n' in my platform Kconfig? >>> >>> Or perhaps I should "override" ARM_L1_CACHE_SHIFT to 5 (again in >>> my platform Kconfig). I don't know the syntax to do that. >>> >>> Could someone point out the correct way? >> >> Would someone care to comment? :-) > > No :) I'm glad that you've decided to disagree with yourself! :-) > What you've found is the _static_ L1 cache line size setting, which is > used at _compile_ time to align structures while building. To allow > maximum flexibility - and because there are 64-byte cache line ARMv7 > implementations around - we have decided that the _compile time_ > cache line size will be 64 bytes. Right, I had a complete brain malfunction there. Compiler needs to be told the cache line size to properly align relevant objects. > As far as cache operations are concerned, they will know the correct > cache line size for the CPU which they're running on, so the code > will adapt. > > It has the side effect that some allocators also assume that the L1 > cache line size is 64 bytes. > > It's better to have a larger than necessary cache line size than a > smaller one, because a larger one is automatically aligned to the > smaller sizes. > > In other words, this is totally intentional. I don't understand why I should not override ARM_L1_CACHE_SHIFT to 5 in my platform-specific Kconfig, since I know I have a 32-byte cache line size? Oh and while I have your attention ;-) I have alignment-related questions about clocksource_mmio_init() (commit 442c8176d2) wrt Thomas Gleixner's 369db4c952 patch. (I think the two patches do not play nice.) 369db4c952 moved some struct clocksource fields around to group hot fields in a single cache line at the beginning of the struct, and marked the struct as cache aligned. This works as expected with static structures. However, I don't think it works as expected with dynamically allocated struct clocksource. It seems to me (and I may very well be wrong!) that struct clocksource_mmio should have the clksrc field at the beginning of the struct, and we should use an allocation function that returns cache aligned memory? struct clocksource_mmio { struct clocksource clksrc; void __iomem *reg; }; cs = kmagic_cache_alloc(sizeof *cs, GFP_KERNEL); That way clksrc would effectively be cache aligned, right? One thing that caught me off-guard: when CONFIG_ARCH_CLOCKSOURCE_DATA and CONFIG_CLOCKSOURCE_WATCHDOG are undefined, struct clocksource weighs 80 bytes on a 32-bit system. I would expect the "reg" field at the end to "fit in the hole", but in fact, gcc seems to "stretch" struct clocksource before considering other fields. This may be a bug in gcc's extension? Regards. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Cache line size definition in arch/arm/mm/Kconfig 2015-03-27 13:45 ` Mason @ 2015-04-01 11:42 ` Mason 2015-04-01 11:50 ` Russell King - ARM Linux 0 siblings, 1 reply; 7+ messages in thread From: Mason @ 2015-04-01 11:42 UTC (permalink / raw) To: linux-arm-kernel On 27/03/2015 14:45, Mason wrote: > arch/arm/mm/Kconfig > > config ARM_L1_CACHE_SHIFT_6 > bool > default y if CPU_V7 > help > Setting ARM L1 cache line size to 64 Bytes. > > config ARM_L1_CACHE_SHIFT > int > default 6 if ARM_L1_CACHE_SHIFT_6 > default 5 > > I don't understand why I should not override ARM_L1_CACHE_SHIFT to 5 > in my platform-specific Kconfig, since I know I have a 32-byte cache > line size? [large snip] It seems to me it would make sense to override ARM_L1_CACHE_SHIFT in the platform Kconfig. Or am I missing something obvious? (Perhaps it is expected that the gains would be minimal?) ARM_L1_CACHE_SHIFT = 6 2 .rodata 0008ea58 c026a000 c026a000 0026a000 2**6 20 .data 00027aa0 c033a000 c033a000 0034a000 2**6 ARM_L1_CACHE_SHIFT = 5 2 .rodata 0008e608 c026b000 c026b000 0026b000 2**5 20 .data 000252c0 c033a000 c033a000 0034a000 2**5 1104 bytes saved in .rodata (0.2%) 10208 bytes saved in .data (6.3%) This is for a minimal kernel (no drivers, only core). The space optimization seems to be not insignificant. > Oh and while I have your attention ;-) I have alignment-related > questions about clocksource_mmio_init() (commit 442c8176d2) wrt > Thomas Gleixner's 369db4c952 patch. (I think the two patches > do not play nice.) > > 369db4c952 moved some struct clocksource fields around to group > hot fields in a single cache line at the beginning of the struct, > and marked the struct as cache aligned. This works as expected > with static structures. Example patch for illustration purposes (only compile-tested) (There is probably a much more elegant way to get 32-byte aligned memory allocations.) Regards. 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 <linux/init.h> #include <linux/slab.h> -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,22 @@ 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 + 31, GFP_KERNEL); if (!cs) return -ENOMEM; + cs = PTR_ALIGN(cs, 32); 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..8b1d689 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Cache line size definition in arch/arm/mm/Kconfig 2015-04-01 11:42 ` Mason @ 2015-04-01 11:50 ` Russell King - ARM Linux 2015-04-01 14:06 ` Mason 0 siblings, 1 reply; 7+ messages in thread From: Russell King - ARM Linux @ 2015-04-01 11:50 UTC (permalink / raw) To: linux-arm-kernel 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.) > > Regards. > > > 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 <linux/init.h> > #include <linux/slab.h> > -struct clocksource_mmio { > - void __iomem *reg; > - struct clocksource clksrc; Just swap the order of these. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Cache line size definition in arch/arm/mm/Kconfig 2015-04-01 11:50 ` Russell King - ARM Linux @ 2015-04-01 14:06 ` Mason 0 siblings, 0 replies; 7+ messages in thread From: Mason @ 2015-04-01 14:06 UTC (permalink / raw) To: linux-arm-kernel 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 <linux/init.h> >> #include <linux/slab.h> >> -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 <linux/init.h> #include <linux/slab.h> -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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-01 14:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-25 14:35 Cache line size definition in arch/arm/mm/Kconfig Mason 2015-03-27 11:42 ` Mason 2015-03-27 12:06 ` Russell King - ARM Linux 2015-03-27 13:45 ` Mason 2015-04-01 11:42 ` Mason 2015-04-01 11:50 ` Russell King - ARM Linux 2015-04-01 14:06 ` Mason
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.