lkml.org 
[lkml]   [2016]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2016-11-24 12:05    [W:3.548 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site