Re: [PATCH] clocksource: Store reg field within struct clocksource

Thomas Gleixner
2015-11-19T11:14:46Z

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::