Messages in this thread | | | Date | Thu, 24 Nov 2016 12:04:54 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH v3 1/3] idle: add support for tasks that inject idle |
| |
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Nov 23, 2016 at 12:13:08PM -0800, Jacob Pan wrote: > > @@ -280,6 +272,58 @@ bool cpu_in_idle(unsigned long pc) > > pc < (unsigned long)__cpuidle_text_end; > > } > > > > +static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer *hrtimer) > > +{ > > + set_tsk_need_resched(current); > > + return HRTIMER_NORESTART; > > +} > > + > > +void play_idle(unsigned long duration_ms) > > +{ > > + struct hrtimer timer; > > + unsigned long end_time; > > + > > + /* > > + * Only FIFO tasks can disable the tick since they don't need the forced > > + * preemption. > > + */ > > + WARN_ON_ONCE(current->policy != SCHED_FIFO); > > + WARN_ON_ONCE(current->nr_cpus_allowed != 1); > > + WARN_ON_ONCE(!(current->flags & PF_KTHREAD)); > > + WARN_ON_ONCE(!(current->flags & PF_NO_SETAFFINITY)); > > + > > + rcu_sleep_check(); > > + preempt_disable(); > > + current->flags |= PF_IDLE; > > + cpuidle_use_deepest_state(true); > > + > > + /* > > + * If duration is 0, we will return after a natural wake event occurs. If > > + * duration is none zero, we will go back to sleep if we were woken up earlier. > > + * We also set up a timer to make sure we don't oversleep in deep idle. > > + */ > > + if (!duration_ms) > > + do_idle(); > > OK, so that doesn't make any sense, you should not be calling this > without a timeout. > > > + else { > > + hrtimer_init_on_stack(&timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); > > + timer.function = idle_inject_timer_fn; > > + hrtimer_start(&timer, ms_to_ktime(duration_ms), > > + HRTIMER_MODE_REL_PINNED); > > + end_time = jiffies + msecs_to_jiffies(duration_ms); > > + > > + while (time_after(end_time, jiffies)) > > + do_idle(); > > + } > > + hrtimer_cancel(&timer); > > + > > + cpuidle_use_deepest_state(false); > > + current->flags &= ~PF_IDLE; > > + > > + preempt_fold_need_resched(); > > + preempt_enable(); > > +} > > +EXPORT_SYMBOL_GPL(play_idle); > > > How about something like so... (since I had to edit, I fixed up most > things Ingo complained about as well). > > Note that it doesn't build because of a distinct lack of > cpuidle_use_deepest_state() in my kernel tree.
Ok, I really like this one, it so much cleaner!
If the patch builds & works:
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
| |