Messages in this thread Patch in this message | | | Date | Thu, 19 Nov 2015 12:14:01 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH] clocksource: Store reg field within struct clocksource |
| |
On Thu, 19 Nov 2015, Russell King - ARM Linux wrote:
> On Thu, Nov 19, 2015 at 11:42:48AM +0100, Thomas Gleixner wrote: > > On Thu, 19 Nov 2015, Russell King - ARM Linux wrote: > > > The basic cause of this problem is the ____cacheline_aligned annotation > > > which effectively prevents wrapping struct clocksource to provide > > > implementation specific data. > > > > > > Maybe your idea is that struct clocksource should be bloated with all > > > implementation specific data in the long term? > > > > Certainly not. That mmio use case is sane enough, but you are right, > > that we should try to lift that ____cacheline_aligned restriction. > > I don't think the cache line alignment of struct clocksource matters > anymore - the core timekeeping code no longer uses struct clocksource > but instead uses struct timekeeper, which caches much of the data from > struct clocksource. The only member of struct clocksource which it > does access is max_cycles, which is more than 32 bytes into struct > clocksource. > > So, I see no reason to waste memory with all these struct clocksources > being bloated out to cacheline alignments. In addition, once > ____cacheline_aligned is removed, I see no reason for Marc's change > either.
Right. I completely forgot that I rewrote the core part some time ago. I'm older than 50, so I'm entitled to use the beginning Alzheimer excuse. :)
So yes, the alignment of the clocksource struct is not longer relevant. The case where we access clocksource->max_cycles is when CONFIG_DEBUG_TIMEKEEPING is enabled, which imposes worse performance problems to timekeeping than the extra cacheline.
So the simple solution for this issue is indeed the one liner below.
Thanks,
tglx
8<-------------------
--- tip.orig/include/linux/clocksource.h +++ tip/include/linux/clocksource.h @@ -95,7 +95,7 @@ struct clocksource { cycle_t wd_last; #endif struct module *owner; -} ____cacheline_aligned; +} /* * Clock source flags bits::
| |