Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752502AbbFZIie (ORCPT ); Fri, 26 Jun 2015 04:38:34 -0400 Received: from foss.arm.com ([217.140.101.70]:59258 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752432AbbFZIiQ (ORCPT ); Fri, 26 Jun 2015 04:38:16 -0400 Message-ID: <558D0F74.4050704@arm.com> Date: Fri, 26 Jun 2015 09:38:12 +0100 From: Sudeep Holla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Preeti U Murthy , 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: <558CDE57.3050008@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3718 Lines: 98 On 26/06/15 06:08, Preeti U Murthy wrote: > On 06/25/2015 09:00 PM, Sudeep Holla wrote: >> >> >> On 25/06/15 14:55, Thomas Gleixner wrote: >>> On Thu, 25 Jun 2015, Sudeep Holla wrote: >>> >>>> tick_broadcast_enter returns 0 when CPU can switch to broadcast >>>> timer and non-zero otherwise. However when GENERIC_CLOCKEVENTS_BROADCAST >>>> and TICK_ONESHOT are disabled, tick_broadcast_oneshot_control returns 0 >>>> which indicates to the CPUIdle framework that the CPU can enter deeper >>>> idle states even when the CPU local timer will be shutdown. If the >>>> target state needs broadcast but not broadcast timer is available, then >>>> the CPU can not resume back from that idle state. >>>> >>>> This patch returns error when there's no broadcast timer support >>>> available so that CPUIdle framework prevents the CPU from entering any >>>> idle states losing the local timer. >>> >>> That's wrong and breaks stuff which does not require the broadcast >>> nonsense. >>> >> >> OK, sorry for not considering that case. >> >>> If TICK_ONESHOT is disabled, then everything is in periodic mode and >>> tick_broadcast_enter() rightfully returns 0. Ditto for 'highres=off' >>> on the command line. >>> >>> But there is a case which is not correctly handled right now. That's >>> what you are trying to solve in the wrong way. >>> >> >> Correct I was trying to solve exactly the case mentioned below. >> >>> 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. >>> >> >> Agreed, thanks for the quick patch, I have tested it and it works fine. >> You can add >> >> Tested-by: Sudeep Holla > > What about the case where GENERIC_CLOCKEVENTS_BROADCAST=y and > TICK_ONESHOT=n (HZ_PERIODIC=y) ? Have you tested this ? > Yes I did test this config, but not the one through cmdline which tglx is suggesting in the other mail. It doesn't hang but all cpus are in shallow idle states(WFI in ARM) and no progress in the boot. But IMO that's different issue and that config is not tested for long time and need more investigation. I will get into that ASAP but that's least used configuration. > This will hang the kernel at boot if you are using the hrtimer mode of > broadcast. This is because the local timers of all cpus are shutdown > when the cpuidle driver registers itself, on finding out that there are > idle states where local tick devices stop. The broadcast tick device is > then in charge of waking up the cpus at every period. In hrtimer mode of > broadcast, there is no such real device and we hang. > No sure what you mean by this. IIUC when you select HIGH_RES_TIMERS, TICK_ONESHOT is selected by default. So I don't understand how to get HZ_PERIODIC=y, HIGH_RES_TIMERS=n and hrtimer mode of broadcast. Am I missing something ? > There was a patch sent out recently to fix this on powerpc. > https://lkml.org/lkml/2015/6/24/42 > Yes I saw that and IIUC you don't register the idle states where local timer stops, correct ? But I am not seeing the hang on ARM as described in the log, I will spend more time to check if I am missing something and not testing the right configuration. Regards, Sudeep -- 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/