Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751849AbbFZE7Q (ORCPT ); Fri, 26 Jun 2015 00:59:16 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:38379 "EHLO e33.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751627AbbFZE7I (ORCPT ); Fri, 26 Jun 2015 00:59:08 -0400 X-Helo: d03dlp01.boulder.ibm.com X-MailFrom: preeti@linux.vnet.ibm.com X-RcptTo: linux-kernel@vger.kernel.org Message-ID: <558CDC14.7030709@linux.vnet.ibm.com> Date: Fri, 26 Jun 2015 10:29:00 +0530 From: Preeti U Murthy User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Thomas Gleixner , Sudeep Holla CC: linux-kernel@vger.kernel.org, Suzuki Poulose , Lorenzo Pieralisi , Catalin Marinas , "Rafael J. Wysocki" Subject: Re: [PATCH] clockevents: return error from tick_broadcast_oneshot_control if !GENERIC_CLOCKEVENTS_BROADCAST References: <1435228065-2428-1-git-send-email-sudeep.holla@arm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15062604-0009-0000-0000-00000C052B3F Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5368 Lines: 163 On 06/25/2015 07:25 PM, Thomas Gleixner wrote: > But there is a case which is not correctly handled right now. That's > what you are trying to solve in the wrong way. > > If > GENERIC_CLOCKEVENTS_BROADCAST=n > > or > > GENERIC_CLOCKEVENTS_BROADCAST=y and no broadcast device is available, > > AND cpu local tick device has the C3STOP flag set, > > then we have no way to tell the idle code that going deep is not > allowed. > > So we need to be smarter than blindly changing a return > value. Completely untested patch below. > > Thanks, > > tglx > --- > diff --git a/include/linux/tick.h b/include/linux/tick.h > index 4191b5623a28..8731a58dd747 100644 > --- a/include/linux/tick.h > +++ b/include/linux/tick.h > @@ -63,11 +63,7 @@ extern void tick_broadcast_control(enum tick_broadcast_mode mode); > static inline void tick_broadcast_control(enum tick_broadcast_mode mode) { } > #endif /* BROADCAST */ > > -#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT) > extern int tick_broadcast_oneshot_control(enum tick_broadcast_state state); > -#else > -static inline int tick_broadcast_oneshot_control(enum tick_broadcast_state state) { return 0; } > -#endif > > static inline void tick_broadcast_enable(void) > { > diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c > index d39f32cdd1b5..52c8d01b956b 100644 > --- a/kernel/time/tick-broadcast.c > +++ b/kernel/time/tick-broadcast.c > @@ -662,24 +662,15 @@ static void broadcast_shutdown_local(struct clock_event_device *bc, > clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN); > } > > -/** > - * tick_broadcast_oneshot_control - Enter/exit broadcast oneshot mode > - * @state: The target state (enter/exit) > - * > - * The system enters/leaves a state, where affected devices might stop > - * Returns 0 on success, -EBUSY if the cpu is used to broadcast wakeups. > - * > - * Called with interrupts disabled, so clockevents_lock is not > - * required here because the local clock event device cannot go away > - * under us. > - */ > -int tick_broadcast_oneshot_control(enum tick_broadcast_state state) > +int __tick_broadcast_oneshot_control(enum tick_broadcast_state state) > { > struct clock_event_device *bc, *dev; > - struct tick_device *td; > int cpu, ret = 0; > ktime_t now; > > + if (!tick_broadcast_device.evtdev) > + return -EBUSY; > + > /* > * Periodic mode does not care about the enter/exit of power > * states > @@ -687,15 +678,7 @@ int tick_broadcast_oneshot_control(enum tick_broadcast_state state) > if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC) > return 0; > > - /* > - * We are called with preemtion disabled from the depth of the > - * idle code, so we can't be moved away. > - */ > - td = this_cpu_ptr(&tick_cpu_device); > - dev = td->evtdev; > - > - if (!(dev->features & CLOCK_EVT_FEAT_C3STOP)) > - return 0; > + dev = this_cpu_ptr(&tick_cpu_device)->evtdev; > > raw_spin_lock(&tick_broadcast_lock); > bc = tick_broadcast_device.evtdev; > @@ -938,6 +921,13 @@ bool tick_broadcast_oneshot_available(void) > return bc ? bc->features & CLOCK_EVT_FEAT_ONESHOT : false; > } > > +#else > +int __tick_broadcast_oneshot_control(enum tick_broadcast_state state) > +{ > + if (!tick_broadcast_device.evtdev) > + return -EBUSY; > + return 0; > +} > #endif > > void __init tick_broadcast_init(void) > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > index 17f144450050..f66c11a30348 100644 > --- a/kernel/time/tick-common.c > +++ b/kernel/time/tick-common.c > @@ -342,6 +342,27 @@ out_bc: > tick_install_broadcast_device(newdev); > } > > +/** > + * tick_broadcast_oneshot_control - Enter/exit broadcast oneshot mode > + * @state: The target state (enter/exit) > + * > + * The system enters/leaves a state, where affected devices might stop > + * Returns 0 on success, -EBUSY if the cpu is used to broadcast wakeups. > + * > + * Called with interrupts disabled, so clockevents_lock is not > + * required here because the local clock event device cannot go away > + * under us. > + */ > +int tick_broadcast_oneshot_control(enum tick_broadcast_state state) > +{ > + struct tick_device *td = this_cpu_ptr(&tick_cpu_device); > + > + if (!(td->evtdev->features & CLOCK_EVT_FEAT_C3STOP)) > + return 0; > + > + return __tick_broadcast_oneshot_control(state); > +} > + > #ifdef CONFIG_HOTPLUG_CPU > /* > * Transfer the do_timer job away from a dying cpu. > diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h > index 42fdf4958bcc..a4a8d4e9baa1 100644 > --- a/kernel/time/tick-sched.h > +++ b/kernel/time/tick-sched.h > @@ -71,4 +71,14 @@ extern void tick_cancel_sched_timer(int cpu); > static inline void tick_cancel_sched_timer(int cpu) { } > #endif > > +#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST > +extern int __tick_broadcast_oneshot_control(enum tick_broadcast_state state); > +#else > +static inline int > +__tick_broadcast_oneshot_control(enum tick_broadcast_state state) > +{ > + return -EBUSY; > +} > +#endif > + > #endif Reviewed-by: Preeti U Murthy > -- 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/