LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Michael Rodin <michael-git@rodin.online>
Cc: x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>
Subject: Re: [PATCH] arch/x86/entry/vsyscall/vsyscall_gtod.c: remove __read_mostly from vclocks_used
Date: Sat, 23 Jun 2018 00:47:12 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1806222333070.1589@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20180604172711.18962-1-michael-git@rodin.online>

Michael,

On Mon, 4 Jun 2018, Michael Rodin wrote:

> The variable "vclocks_used" doesn't appear to be "read mostly".
> Measurements of the access frequency with perf stat [1] and
> perf report show, that approximately half of the accesses to
> this variable are write accesses and happen in update_vsyscall()
> in the file arch/x86/entry/vsyscall/vsyscall_gtod.c.
> The measurements were done with the kernel 4.13.0-43-generic used by
> ubuntu as well as with the stable kernel 4.16.7 with a custom config.
> I've used "perf bench sched" and iperf3 as workloads.
> 
> This was discovered during my master thesis in the CADOS project [2].

Nice find, but ...

The point is that the content of that variable changes once in a blue moon,
so the intent of marking it read_mostly is almost correct. Almost, because
the unconditional write even with the ever repeating same value is not
free. To the best of my knowledge there is no CPU implementation which
supports optimizing silent stores, but I might be just not up to date or
wrong as usual.

So there is a cost to the write and there are two things which have to be
looked at before just doing the obvious knee jerk reaction
's/read_mostly//'.

1) Does it make sense to avoid the write if the value hasn't changed? That
   might be pretty much a free lunch because the variable has to be read in
   the first place in order to OR the supplied value to it before writing
   it back.

2) The variable is in a totally separate cache line than what the following
   stores target, i.e. vsyscall_gtod_data

So it might make sense just to do #1 but as vsyscall_gtod_data is anyway
going to be dirtied it probably makes even more sense to move that
vclocks_used variable into struct vsyscall_gtod_data right away to reduce
the amount of cache lines touched by update_vsyscall().

Care to have a look at that?

Thanks,

	tglx

  reply	other threads:[~2018-06-22 22:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-04 17:27 Michael Rodin
2018-06-22 22:47 ` Thomas Gleixner [this message]
2018-06-22 23:25   ` Andy Lutomirski
2018-06-23  9:04     ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1806222333070.1589@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=michael-git@rodin.online \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH] arch/x86/entry/vsyscall/vsyscall_gtod.c: remove __read_mostly from vclocks_used' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).