From: Peter Zijlstra <peterz@infradead.org> To: Will Deacon <will@kernel.org> Cc: Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>, 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>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mathieu Poirier <mathieu.poirier@linaro.org>, Mike Leach <mike.leach@linaro.org>, Al Grant <Al.Grant@arm.com>, James Clark <James.Clark@arm.com>, maz@kernel.org, tglx@linutronix.de Subject: Re: [PATCH] arm64: perf_event: Fix time_offset for arch timer Date: Thu, 30 Apr 2020 18:27:50 +0200 [thread overview] Message-ID: <20200430162750.GD13575@hirez.programming.kicks-ass.net> (raw) In-Reply-To: <20200430145823.GA25258@willie-the-truck> On Thu, Apr 30, 2020 at 03:58:24PM +0100, Will Deacon wrote: > On Fri, Mar 20, 2020 at 05:35:45PM +0800, Leo Yan wrote: > > @@ -1164,5 +1165,21 @@ void arch_perf_update_userpage(struct perf_event *event, > > userpg->time_mult >>= 1; > > } > > userpg->time_shift = (u16)shift; > > - userpg->time_offset = -now; > > + > > + /* > > + * Since arch timer is enabled ealier than sched clock registration, > > + * compuate the delta (in nanosecond unit) between the arch timer > > + * counter and sched clock, assign the delta to time_offset and > > + * perf tool can use it for timestamp calculation. > > + * > > + * The formula for conversion arch timer cycle to ns is: > > + * quot = (cyc >> time_shift); > > + * rem = cyc & ((1 << time_shift) - 1); > > + * ns = quot * time_mult + ((rem * time_mult) >> time_shift); > > + */ > > + count = arch_timer_read_counter(); > > + quot = count >> shift; > > + rem = count & ((1 << shift) - 1); > > + ns = quot * userpg->time_mult + ((rem * userpg->time_mult) >> shift); > > + userpg->time_offset = now - ns; > > Hmm, reading the counter and calculating the delta feels horribly > approximate to me. It would be much better if we could get hold of the > initial epoch cycles from the point at which sched_clock was initialised > using the counter. This represents the true cycle delta between the counter > and what sched_clock uses for 0 ns. > > Unfortunately, I can't see a straightforward way to grab that information. > It looks like x86 pulls this directly from the TSC driver. Yeah, and I'm thinking you should do the same. IIRC ARM uses this kernel/time/sched_clock.c thing, and if I read that right, the struct clock_data there has all the bits you need here. So I'm thinking that you might want to add a helper function here to get you the good stuff.
WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org> To: Will Deacon <will@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com>, maz@kernel.org, Al Grant <Al.Grant@arm.com>, Mathieu Poirier <mathieu.poirier@linaro.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo <acme@kernel.org>, Ingo Molnar <mingo@redhat.com>, James Clark <James.Clark@arm.com>, Leo Yan <leo.yan@linaro.org>, Namhyung Kim <namhyung@kernel.org>, tglx@linutronix.de, Jiri Olsa <jolsa@redhat.com>, linux-arm-kernel@lists.infradead.org, Mike Leach <mike.leach@linaro.org> Subject: Re: [PATCH] arm64: perf_event: Fix time_offset for arch timer Date: Thu, 30 Apr 2020 18:27:50 +0200 [thread overview] Message-ID: <20200430162750.GD13575@hirez.programming.kicks-ass.net> (raw) In-Reply-To: <20200430145823.GA25258@willie-the-truck> On Thu, Apr 30, 2020 at 03:58:24PM +0100, Will Deacon wrote: > On Fri, Mar 20, 2020 at 05:35:45PM +0800, Leo Yan wrote: > > @@ -1164,5 +1165,21 @@ void arch_perf_update_userpage(struct perf_event *event, > > userpg->time_mult >>= 1; > > } > > userpg->time_shift = (u16)shift; > > - userpg->time_offset = -now; > > + > > + /* > > + * Since arch timer is enabled ealier than sched clock registration, > > + * compuate the delta (in nanosecond unit) between the arch timer > > + * counter and sched clock, assign the delta to time_offset and > > + * perf tool can use it for timestamp calculation. > > + * > > + * The formula for conversion arch timer cycle to ns is: > > + * quot = (cyc >> time_shift); > > + * rem = cyc & ((1 << time_shift) - 1); > > + * ns = quot * time_mult + ((rem * time_mult) >> time_shift); > > + */ > > + count = arch_timer_read_counter(); > > + quot = count >> shift; > > + rem = count & ((1 << shift) - 1); > > + ns = quot * userpg->time_mult + ((rem * userpg->time_mult) >> shift); > > + userpg->time_offset = now - ns; > > Hmm, reading the counter and calculating the delta feels horribly > approximate to me. It would be much better if we could get hold of the > initial epoch cycles from the point at which sched_clock was initialised > using the counter. This represents the true cycle delta between the counter > and what sched_clock uses for 0 ns. > > Unfortunately, I can't see a straightforward way to grab that information. > It looks like x86 pulls this directly from the TSC driver. Yeah, and I'm thinking you should do the same. IIRC ARM uses this kernel/time/sched_clock.c thing, and if I read that right, the struct clock_data there has all the bits you need here. So I'm thinking that you might want to add a helper function here to get you the good stuff. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-04-30 16:28 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-20 9:35 [PATCH] arm64: perf_event: Fix time_offset for arch timer Leo Yan 2020-03-20 9:35 ` Leo Yan 2020-04-01 1:24 ` Leo Yan 2020-04-01 1:24 ` Leo Yan 2020-04-30 14:58 ` Will Deacon 2020-04-30 14:58 ` Will Deacon 2020-04-30 15:29 ` Marc Zyngier 2020-04-30 15:29 ` Marc Zyngier 2020-04-30 16:04 ` Peter Zijlstra 2020-04-30 16:04 ` Peter Zijlstra 2020-04-30 16:18 ` Will Deacon 2020-04-30 16:18 ` Will Deacon 2020-04-30 17:33 ` Marc Zyngier 2020-04-30 17:33 ` Marc Zyngier 2020-05-01 15:14 ` Leo Yan 2020-05-01 15:14 ` Leo Yan 2020-05-01 15:26 ` Will Deacon 2020-05-01 15:26 ` Will Deacon 2020-05-01 16:10 ` Leo Yan 2020-05-01 16:10 ` Leo Yan 2020-05-01 17:13 ` Will Deacon 2020-05-01 17:13 ` Will Deacon 2020-05-01 15:29 ` Marc Zyngier 2020-05-01 15:29 ` Marc Zyngier 2020-04-30 16:27 ` Peter Zijlstra [this message] 2020-04-30 16:27 ` Peter Zijlstra 2020-04-30 16:45 ` Will Deacon 2020-04-30 16:45 ` Will Deacon
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=20200430162750.GD13575@hirez.programming.kicks-ass.net \ --to=peterz@infradead.org \ --cc=Al.Grant@arm.com \ --cc=James.Clark@arm.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=catalin.marinas@arm.com \ --cc=jolsa@redhat.com \ --cc=leo.yan@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=maz@kernel.org \ --cc=mike.leach@linaro.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.