From: Mark Rutland <mark.rutland@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Leo Yan <leo.yan@linaro.org>, Will Deacon <will@kernel.org>,
Marc Zyngier <maz@kernel.org>, Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Thomas Gleixner <tglx@linutronix.de>,
Allison Randal <allison@lohutok.net>,
Alexios Zavras <alexios.zavras@intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Kate Stewart <kstewart@linuxfoundation.org>,
Enrico Weigelt <info@metux.net>,
"Ahmed S. Darwish" <a.darwish@linutronix.de>,
Paul Cercueil <paul@crapouillou.net>,
"Ben Dooks (Codethink)" <ben.dooks@codethink.co.uk>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 0/3] arm64: perf_event: Fix time offset prior to epoch
Date: Tue, 12 May 2020 11:01:14 +0100 [thread overview]
Message-ID: <20200512100114.GA60359@C02TD0UTHF1T.local> (raw)
In-Reply-To: <20200512091918.GH2978@hirez.programming.kicks-ass.net>
On Tue, May 12, 2020 at 11:19:18AM +0200, Peter Zijlstra wrote:
> On Tue, May 12, 2020 at 02:38:12PM +0800, Leo Yan wrote:
> > @@ -1165,28 +1166,26 @@ device_initcall(armv8_pmu_driver_init)
> > void arch_perf_update_userpage(struct perf_event *event,
> > struct perf_event_mmap_page *userpg, u64 now)
> > {
> > + struct clock_read_data *rd;
> > + unsigned int seq;
> >
> > /*
> > * Internal timekeeping for enabled/running/stopped times
> > * is always computed with the sched_clock.
> > */
> > userpg->cap_user_time = 1;
> > + userpg->cap_user_time_zero = 1;
> >
> > + do {
> > + rd = sched_clock_read_begin(&seq);
> > +
> > + userpg->time_mult = rd->mult;
> > + userpg->time_shift = rd->shift;
> > + userpg->time_zero = rd->epoch_ns;
> > +
> > + userpg->time_zero -= (rd->epoch_cyc * rd->mult) >> rd->shift;
>
> Damn, I think this is broken vs the counter wrapping.
>
> So what the sched_clock code does is:
>
> cyc_to_ns((cyc - rd->epoch_cyc) & rd->sched_clock_mask, rd->mult, rd->shift)
>
> But because the perf interface assumes a simple linear relation, we
> can't express that properly.
>
> Now, your arm64 counter is 56 bits, so wrapping is rare, but still, we
> should probably fix that. And that probably needs an ABI extention
> *sigh*.
FWIW, its's /at least/ 56 bits wide, and the ARM ARM says that it
shouldn't wrap in fewer than 40 years, so no correct implementation
should wrap before the 2050s.
If it's wider than 56 bits, the 56-bit portion could wrap more quickly
than that, so we should probably always treat it as 64-bits.
From ARMv8.6 it's always 64 bits wide @ a nominal 1GHz, and a 64-bit
wrap will take ~584.9 years (with a 56-bit wrap taking ~834 days).
See D11.1.2 "The system counter" in the latest ARM ARM (0487F.b):
https://static.docs.arm.com/ddi0487/fb/DDI0487F_b_armv8_arm.pdf?_ga=2.83012310.1749782910.1589218924-1447552059.1588172444
https://developer.arm.com/docs/ddi0487/latest
Thanks,
Mark.
next prev parent reply other threads:[~2020-05-12 10:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-05 13:55 Leo Yan
2020-05-05 13:55 ` [PATCH v2 1/3] time/sched_clock: Add new variant sched_clock_register_epoch() Leo Yan
2020-05-05 13:55 ` [PATCH v2 2/3] clocksource/drivers/arm_arch_timer: Handle time offset prior to epoch Leo Yan
2020-05-05 13:55 ` [PATCH v2 3/3] arm64: perf_event: Fix time_offset for arch timer Leo Yan
2020-05-11 9:22 ` [PATCH v2 0/3] arm64: perf_event: Fix time offset prior to epoch Peter Zijlstra
2020-05-11 9:25 ` Peter Zijlstra
2020-05-12 6:38 ` Leo Yan
2020-05-12 8:57 ` Peter Zijlstra
2020-05-12 9:19 ` Peter Zijlstra
2020-05-12 10:01 ` Mark Rutland [this message]
2020-05-12 11:21 ` Peter Zijlstra
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=20200512100114.GA60359@C02TD0UTHF1T.local \
--to=mark.rutland@arm.com \
--cc=a.darwish@linutronix.de \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=alexios.zavras@intel.com \
--cc=allison@lohutok.net \
--cc=ben.dooks@codethink.co.uk \
--cc=catalin.marinas@arm.com \
--cc=daniel.lezcano@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=info@metux.net \
--cc=jolsa@redhat.com \
--cc=kstewart@linuxfoundation.org \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=paul@crapouillou.net \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®