Re: make btrace interval event to reciprocal of ticks
On 25/06/20(Thu) 23:45, Yuichiro NAITO wrote: > From: Martin Pieuchot > Subject: Re: make btrace interval event to reciprocal of ticks > Date: Thu, 25 Jun 2020 11:36:48 +0200 > > > On 23/06/20(Tue) 12:04, Yuichiro NAITO wrote: > >> Current btrace has `interval:hz:1` probe that makes periodically events. > >> `interval:hz:1` looks to make events once per second (= 1Hz), > >> but current implementation makes once per tick (= 100Hz on amd64). > >> I think the interval should be counted by reciprocal of ticks so that fit > >> to Hz. > > > > Indeed the current implementations assumes it's a number of ticks. How > > does other tracing tool behave? Is the behavior you suggest similar to > > bpftrace(8) or dtrace(1)? > > I don't know about bpftrace(8). > Dtrace has interval timer in Hz as Lauri says. Seems to be the same, so I'll commit your diff. > > Would it be complicated to add support for other units, like 'ms' or > > 'sec'? In that case 'profile:s:10' would mean every 10sec while > > 'profile:hz:10' would mean every ~6sec, right? > > I feel it's ok to just rename 'interval:hz' to 'interval:ticks' with > no implementation change. (but I hope that 100 is allowed.) > We can know the tick interval from 'hz' in `sysctl -n kern.clockrate`. > So it's easy to understand that 'interval:ticks:N' fires on every N ticks. > > And tick resolution is not so high. > In my patch, 51 Hz ~ 100 Hz is calculated to 10 ms (=1 tick) interval. > It might confuse some users. > > In my usecase, I want 10ms ~ 1sec intervals. > If N is allowed over 99, 'interval:ticks:N' can be useful to me. Well ideally we should be able to specify an interval in second or millisecond. If you or somebody else wants to implement that, I suppose it makes sense now to change `dtrq_rate' to be a uint64_t and encode the interval in nanosecond.
Re: make btrace interval event to reciprocal of ticks
From: Martin Pieuchot Subject: Re: make btrace interval event to reciprocal of ticks Date: Thu, 25 Jun 2020 11:36:48 +0200 > On 23/06/20(Tue) 12:04, Yuichiro NAITO wrote: >> Current btrace has `interval:hz:1` probe that makes periodically events. >> `interval:hz:1` looks to make events once per second (= 1Hz), >> but current implementation makes once per tick (= 100Hz on amd64). >> I think the interval should be counted by reciprocal of ticks so that fit to >> Hz. > > Indeed the current implementations assumes it's a number of ticks. How > does other tracing tool behave? Is the behavior you suggest similar to > bpftrace(8) or dtrace(1)? I don't know about bpftrace(8). Dtrace has interval timer in Hz as Lauri says. > Would it be complicated to add support for other units, like 'ms' or > 'sec'? In that case 'profile:s:10' would mean every 10sec while > 'profile:hz:10' would mean every ~6sec, right? I feel it's ok to just rename 'interval:hz' to 'interval:ticks' with no implementation change. (but I hope that 100 is allowed.) We can know the tick interval from 'hz' in `sysctl -n kern.clockrate`. So it's easy to understand that 'interval:ticks:N' fires on every N ticks. And tick resolution is not so high. In my patch, 51 Hz ~ 100 Hz is calculated to 10 ms (=1 tick) interval. It might confuse some users. In my usecase, I want 10ms ~ 1sec intervals. If N is allowed over 99, 'interval:ticks:N' can be useful to me. -- Yuichiro NAITO naito.yuich...@gmail.com
Re: make btrace interval event to reciprocal of ticks
On Thu, Jun 25 2020 11:36:48 +0200, Martin Pieuchot wrote: > On 23/06/20(Tue) 12:04, Yuichiro NAITO wrote: > > Current btrace has `interval:hz:1` probe that makes periodically events. > > `interval:hz:1` looks to make events once per second (= 1Hz), > > but current implementation makes once per tick (= 100Hz on amd64). > > I think the interval should be counted by reciprocal of ticks so that fit > > to Hz. > > Indeed the current implementations assumes it's a number of ticks. How > does other tracing tool behave? Is the behavior you suggest similar to > bpftrace(8) or dtrace(1)? In DTrace, profile- probes fire at a frequency of hertz, but apparently also support suffixes. http://dtrace.org/guide/chp-profile.html -- Lauri Tirkkonen | lotheac @ IRCnet
Re: make btrace interval event to reciprocal of ticks
On 23/06/20(Tue) 12:04, Yuichiro NAITO wrote:
> Current btrace has `interval:hz:1` probe that makes periodically events.
> `interval:hz:1` looks to make events once per second (= 1Hz),
> but current implementation makes once per tick (= 100Hz on amd64).
> I think the interval should be counted by reciprocal of ticks so that fit to
> Hz.
Indeed the current implementations assumes it's a number of ticks. How
does other tracing tool behave? Is the behavior you suggest similar to
bpftrace(8) or dtrace(1)?
Would it be complicated to add support for other units, like 'ms' or
'sec'? In that case 'profile:s:10' would mean every 10sec while
'profile:hz:10' would mean every ~6sec, right?
> Following patch changes to calculate 'dp_maxtick' fit to Hz.
> 'dtrq_rate' has number of hz. I think it should be allowed same value of 'hz'
> so that we can use `interval:hz:100` that equals to once per tick on amd64.
>
> Index: dt_prov_profile.c
> ===
> RCS file: /cvs/src/sys/dev/dt/dt_prov_profile.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 dt_prov_profile.c
> --- dt_prov_profile.c 25 Mar 2020 14:59:23 - 1.2
> +++ dt_prov_profile.c 23 Jun 2020 02:02:27 -
> @@ -75,7 +75,7 @@ dt_prov_profile_alloc(struct dt_probe *d
> KASSERT(TAILQ_EMPTY(plist));
> KASSERT(dtp == dtpp_profile || dtp == dtpp_interval);
>
> - if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate >= hz)
> + if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate > hz)
> return EOPNOTSUPP;
>
> CPU_INFO_FOREACH(cii, ci) {
> @@ -88,7 +88,7 @@ dt_prov_profile_alloc(struct dt_probe *d
> return ENOMEM;
> }
>
> - dp->dp_maxtick = dtrq->dtrq_rate;
> + dp->dp_maxtick = hz / dtrq->dtrq_rate;
> dp->dp_cpuid = ci->ci_cpuid;
>
> dp->dp_filter = dtrq->dtrq_filter;
>
> --
> Yuichiro NAITO
> naito.yuich...@gmail.com
>
make btrace interval event to reciprocal of ticks
Current btrace has `interval:hz:1` probe that makes periodically events.
`interval:hz:1` looks to make events once per second (= 1Hz),
but current implementation makes once per tick (= 100Hz on amd64).
I think the interval should be counted by reciprocal of ticks so that fit to Hz.
Following patch changes to calculate 'dp_maxtick' fit to Hz.
'dtrq_rate' has number of hz. I think it should be allowed same value of 'hz'
so that we can use `interval:hz:100` that equals to once per tick on amd64.
Index: dt_prov_profile.c
===
RCS file: /cvs/src/sys/dev/dt/dt_prov_profile.c,v
retrieving revision 1.2
diff -u -p -r1.2 dt_prov_profile.c
--- dt_prov_profile.c 25 Mar 2020 14:59:23 - 1.2
+++ dt_prov_profile.c 23 Jun 2020 02:02:27 -
@@ -75,7 +75,7 @@ dt_prov_profile_alloc(struct dt_probe *d
KASSERT(TAILQ_EMPTY(plist));
KASSERT(dtp == dtpp_profile || dtp == dtpp_interval);
- if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate >= hz)
+ if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate > hz)
return EOPNOTSUPP;
CPU_INFO_FOREACH(cii, ci) {
@@ -88,7 +88,7 @@ dt_prov_profile_alloc(struct dt_probe *d
return ENOMEM;
}
- dp->dp_maxtick = dtrq->dtrq_rate;
+ dp->dp_maxtick = hz / dtrq->dtrq_rate;
dp->dp_cpuid = ci->ci_cpuid;
dp->dp_filter = dtrq->dtrq_filter;
--
Yuichiro NAITO
naito.yuich...@gmail.com
