Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1487386

Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset latency

Show all headers | View raw

On Tue, 20 Sep 2016, Binoy Jayan wrote:
> +#ifdef CONFIG_TRACE_DELAYED_TIMER_OFFSETS
> +static inline void latency_hrtimer_timing_start(struct hrtimer *timer,
> +					 struct hrtimer_clock_base *new_base,
> +					 ktime_t tim)
> +{
> +	ktime_t now = new_base->get_time();
> +
> +	if (ktime_to_ns(tim) < ktime_to_ns(now))
> +		timer->tim_expiry = now;
> +	else
> +		timer->tim_expiry = ktime_set(0, 0);

You still fail to explain why this get_time() magic is required.

This is executed unconditionally when the config switch is enabled and does
not depend on whether the trace functionality is enabled or not. So you are
imposing the extra get_time() call, which can be expensive depending on the
underlying hardware, on every hrtimer start invocation.

Tracing is supposed to have ZERO impact when it is not used and even when
it's in use then the impact should be kept as low as possible. The above
does none of that.

Neither did you provide a proper rationale for this infrastructure in the
changelog.

You can repost that over and over and it will not go anywhere if you don't
start to address the review comments I give you.

Thanks,

	tglx

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH v7 0/5] *** Latency histograms *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-20 16:00 +0200
  [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset latency Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-20 16:00 +0200
    Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset  latency Thomas Gleixner <tglx@linutronix.de> - 2016-09-20 16:30 +0200
      Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset latency Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-21 08:10 +0200
        Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset  latency Thomas Gleixner <tglx@linutronix.de> - 2016-09-21 18:10 +0200
          Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset latency Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-22 11:20 +0200
            Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset  latency Thomas Gleixner <tglx@linutronix.de> - 2016-09-22 19:40 +0200
              Re: [RFC PATCH v7 4/5] tracing: Measure delayed hrtimer offset  latency Mark Brown <broonie@kernel.org> - 2016-09-22 21:30 +0200
  [RFC PATCH v7 2/5] tracing: Add hist trigger support for generic fields Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-20 16:00 +0200
  [RFC PATCH v7 5/5] tracing: wakeup latency events and histograms Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-20 16:00 +0200
  [RFC PATCH v7 3/5] tracing: Add preemptirqsoff timing events Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-20 16:00 +0200
  [RFC PATCH v7 1/5] tracing: Dereference pointers without RCU checks Binoy Jayan <binoy.jayan@linaro.org> - 2016-09-20 16:00 +0200

csiph-web