From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941016AbcKXLFB (ORCPT ); Thu, 24 Nov 2016 06:05:01 -0500 Received: from mail-wj0-f196.google.com ([209.85.210.196]:35737 "EHLO mail-wj0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S937926AbcKXLE6 (ORCPT ); Thu, 24 Nov 2016 06:04:58 -0500 Date: Thu, 24 Nov 2016 12:04:54 +0100 From: Ingo Molnar To: Peter Zijlstra Cc: Jacob Pan , Ingo Molnar , Thomas Gleixner , LKML , Linux PM , Rafael Wysocki , Arjan van de Ven , Srinivas Pandruvada , Len Brown , Eduardo Valentin , Zhang Rui , Petr Mladek , Sebastian Andrzej Siewior Subject: Re: [PATCH v3 1/3] idle: add support for tasks that inject idle Message-ID: <20161124110454.GA22551@gmail.com> References: <1479931990-11732-1-git-send-email-jacob.jun.pan@linux.intel.com> <1479931990-11732-2-git-send-email-jacob.jun.pan@linux.intel.com> <20161124095016.GD3092@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161124095016.GD3092@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Peter Zijlstra 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 Thanks, Ingo