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

Groups > linux.kernel > #1266522

Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection

Show all headers | View raw

On Tue, 10 Nov 2015 14:23:24 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> > +static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer
> > *hrtimer) +{
> > +	struct hrtimer *hrt = this_cpu_ptr(&idle_inject_timer);
> > +	int cpu = smp_processor_id();
> > +	ktime_t now, delta, period;
> > +	bool status;
> > +
> > +	now = hrtimer_cb_get_time(hrt);  
> 
> You're not interested in the current time.
> 
> > +
> > +	status = raw_cpu_read(idle_injected);
> > +	if (status) {
> > +		/*
> > +		 * We were injecting idle in the last phase, let's
> > forward the
> > +		 * timer to the next period
> > +		 *
> > +		 * status: 1             0                1
> > 0
> > +		 * ____          ____________________
> > _______
> > +		 *     |________|                    |_________|
> > +		 *
> > +		 *     |duration|      interval      |
> > +		 *
> > +		 *              ^ we are here
> > +		 *                  forward to here: ^
> > +		 */
> > +		delta = ktime_sub(now, inject_start_time);
> > +		period = ktime_add(ms_to_ktime(duration),
> > +				ms_to_ktime(inject_interval));
> > +		delta = ktime_roundup(delta, period);
> > +		hrtimer_set_expires(hrt, ktime_add(delta,
> > inject_start_time));  
> 
> This doesn't make any sense. Who cares what the current time is.
> 
> > +	} else {
> > +		/*
> > +		 * We were not injecting idle in the last phase,
> > let's forward
> > +		 * timer after forced idle duration
> > +		 * ____          ____________________
> > _______
> > +		 *     |________|                    |_________|
> > +		 *
> > +		 *     |duration|      interval      |
> > +		 *
> > +		 *     ^ we are here
> > +		 *              ^ forward timer to here
> > +		 */
> > +		hrtimer_set_expires(hrt,
> > ktime_add(ms_to_ktime(duration), now));  
> 
> Same here, we don't care about the current time. The timer was at the
> previous start of injection, just forward it a whole period to find
> the next injection slot.
> 
> > +	}  
> 
> It looks like what you want is:
> 
> 	hrtimer_forward(hrt, period);
> 
> unconditionally.
In the ideal world yes. But my thinking was that timers may not be so
accurate to deliver interrupts, over the time the timeout error may
accumulate so that eventually timers will be out of sync. That is why
at the beginning of the period I realign the timer based on a common
start time so that timers will never drift off. I use current time and
the common start time to locate the next absolute timeout not the
relative timeout.
Overall, my idea was to make it more robust and handles runtime
parameters (percentage, duration) smoothly.

I will fix the rest comments in the patch.

Thank you,

Jacob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

Thread

[RFC PATCH v2 3/3] sched: introduce synchronized idle injection Jacob Pan <jacob.jun.pan@linux.intel.com> - 2015-11-10 01:30 +0100
  Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Peter Zijlstra <peterz@infradead.org> - 2015-11-10 14:30 +0100
    Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Jacob Pan <jacob.jun.pan@linux.intel.com> - 2015-11-10 15:10 +0100
      Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Peter Zijlstra <peterz@infradead.org> - 2015-11-10 16:00 +0100
        Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Peter Zijlstra <peterz@infradead.org> - 2015-11-10 17:40 +0100
          Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Jacob Pan <jacob.jun.pan@linux.intel.com> - 2015-11-10 18:00 +0100
            Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Peter Zijlstra <peterz@infradead.org> - 2015-11-10 18:10 +0100
              Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Jacob Pan <jacob.jun.pan@linux.intel.com> - 2015-11-10 18:20 +0100
        Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Jacob Pan <jacob.jun.pan@linux.intel.com> - 2015-11-10 17:40 +0100
    Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection Jacob Pan <jacob.jun.pan@linux.intel.com> - 2015-11-10 19:50 +0100

csiph-web