Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1273043

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

Show all headers | View raw

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] clocksource: Store reg field within struct clocksource Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-11-18 14:50 +0100
  Re: [PATCH] clocksource: Store reg field within struct clocksource Måns Rullgård <mans@mansr.com> - 2015-11-18 15:00 +0100
  Re: [PATCH] clocksource: Store reg field within struct clocksource Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-11-18 18:30 +0100
    Re: [PATCH] clocksource: Store reg field within struct clocksource Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-11-19 10:40 +0100
      Re: [PATCH] clocksource: Store reg field within struct clocksource Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-11-19 11:40 +0100
        Re: [PATCH] clocksource: Store reg field within struct clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-11-19 11:40 +0100
          Re: [PATCH] clocksource: Store reg field within struct clocksource Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-11-19 11:50 +0100
    Re: [PATCH] clocksource: Store reg field within struct clocksource Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-11-19 11:40 +0100
      Re: [PATCH] clocksource: Store reg field within struct clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-11-19 11:50 +0100
        Re: [PATCH] clocksource: Store reg field within struct clocksource Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-11-19 12:10 +0100
          Re: [PATCH] clocksource: Store reg field within struct clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-11-19 12:20 +0100
            Re: [PATCH] clocksource: Store reg field within struct clocksource Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-11-19 13:30 +0100
              Re: [PATCH] clocksource: Store reg field within struct clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-11-19 15:00 +0100
                Re: [PATCH] clocksource: Store reg field within struct clocksource Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-11-24 13:40 +0100
            [tip:timers/core] timekeeping:   Lift clocksource cacheline restriction tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2015-11-25 22:40 +0100
      Re: [PATCH] clocksource: Store reg field within struct clocksource Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-11-19 12:00 +0100
        Re: [PATCH] clocksource: Store reg field within struct clocksource Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-11-19 12:30 +0100
          Re: [PATCH] clocksource: Store reg field within struct clocksource Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-11-19 13:50 +0100
    Re: [PATCH] clocksource: Store reg field within struct clocksource Thomas Gleixner <tglx@linutronix.de> - 2015-11-19 11:40 +0100

csiph-web