Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752514AbbFZMfa (ORCPT ); Fri, 26 Jun 2015 08:35:30 -0400 Received: from e35.co.us.ibm.com ([32.97.110.153]:60766 "EHLO e35.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751902AbbFZMee (ORCPT ); Fri, 26 Jun 2015 08:34:34 -0400 X-Helo: d03dlp03.boulder.ibm.com X-MailFrom: preeti@linux.vnet.ibm.com X-RcptTo: linux-kernel@vger.kernel.org Message-ID: <558D46C5.8080307@linux.vnet.ibm.com> Date: Fri, 26 Jun 2015 18:04:13 +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 CC: Sudeep Holla , "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> <558C1E97.4020206@arm.com> <558CDE57.3050008@linux.vnet.ibm.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: 15062612-0013-0000-0000-00001050D155 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4093 Lines: 112 On 06/26/2015 01:17 PM, Thomas Gleixner wrote: > diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c > index d39f32cdd1b5..281ce29d295e 100644 > --- a/kernel/time/tick-broadcast.c > +++ b/kernel/time/tick-broadcast.c > @@ -662,46 +662,39 @@ 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; > > - /* > - * Periodic mode does not care about the enter/exit of power > - * states > - */ > - if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC) > - return 0; > + if (!tick_broadcast_device.evtdev) > + return -EBUSY; > > - /* > - * 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; > cpu = smp_processor_id(); > > if (state == TICK_BROADCAST_ENTER) { > + /* > + * If the current CPU owns the hrtimer broadcast > + * mechanism, it cannot go deep idle and we do not add > + * the CPU to the broadcast mask. We don't have to go > + * through the EXIT path as the local timer is not > + * shutdown. > + */ > + ret = broadcast_needs_cpu(bc, cpu); > + > + /* > + * If the hrtimer broadcast check tells us that the > + * cpu cannot go deep idle, or if the broadcast device > + * is in periodic mode, we just return. > + */ > + if (ret || tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC) > + goto out; The check on PERIODIC mode is good, but I don't see the point of moving broadcast_needs_cpu() up above. broadcast_shutdown_local() calls broadcast_needs_cpu() internally. Besides, by jumping to 'out', we will miss programming the broadcast hrtimer in tick_broadcast_set_event() below, if the cpu happen to be the broadcast cpu(which is why it was not allowed to go to deep idle). > + > if (!cpumask_test_and_set_cpu(cpu, tick_broadcast_oneshot_mask)) { > WARN_ON_ONCE(cpumask_test_cpu(cpu, tick_broadcast_pending_mask)); > broadcast_shutdown_local(bc, dev); > @@ -717,16 +710,6 @@ int tick_broadcast_oneshot_control(enum tick_broadcast_state state) > dev->next_event.tv64 < bc->next_event.tv64) > tick_broadcast_set_event(bc, cpu, dev->next_event); > } > - /* > - * If the current CPU owns the hrtimer broadcast > - * mechanism, it cannot go deep idle and we remove the > - * CPU from the broadcast mask. We don't have to go > - * through the EXIT path as the local timer is not > - * shutdown. > - */ > - ret = broadcast_needs_cpu(bc, cpu); > - if (ret) > - cpumask_clear_cpu(cpu, tick_broadcast_oneshot_mask); > } else { > if (cpumask_test_and_clear_cpu(cpu, tick_broadcast_oneshot_mask)) { > clockevents_switch_state(dev, CLOCK_EVT_STATE_ONESHOT); > @@ -938,6 +921,13 @@ bool tick_broadcast_oneshot_available(void) > return bc ? bc->features & CLOCK_EVT_FEAT_ONESHOT : false; > } > Regards 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/