linux-kernel - Re: [PATCH] clocksource: Store reg field within struct clocksource

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.11.1511191209000.3898@nanos>
Date:	Thu, 19 Nov 2015 12:14:01 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
cc:	Marc Gonzalez <marc_gonzalez@...madesigns.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Mans Rullgard <mans@...sr.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Nicolas Pitre <nico@...aro.org>,
	Tony Lindgren <tony@...mide.com>,
	Sebastian Frias <sebastian_frias@...madesigns.com>
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::

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