From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D9A6C433EF for ; Sat, 9 Oct 2021 15:44:39 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4E4DF60F5C for ; Sat, 9 Oct 2021 15:44:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4E4DF60F5C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=rjwysocki.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=usJwS0JDBvEkZMSaULCH3ALrfm8XG8CVoEC3N9TZ540=; b=Xoio7+QgXO5LA1 lpUas0d1XmESgv35ZdnvO/go7Mmzbto58XT7XvBLwaopuejfRN+qdznpaqO3Ep3gKnCQOqqrNb6dW bVIujOThAnqZ+BkkEmbsVn9cWM9RmfYpXFYhnLoOVn6p1fzOQBYDpCm3wCHwbh5nUeY0J6p4ebmdO 2Z7mMZrPEw/NpOTtF2R2I+TFwhLzdkSMCi4b2vtgDa3FuyHtOSV+8HzymonYDuV5ceTWYY7z790uC vhG2x1r7ZH507HokUGixvlPLW95c9n8lhSnagUJ9GfvxpbfhgbF095/f7/mCQnrgMO3nmCKmE6cdn Qa681elmS5ltSj6abgfw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mZEV2-005lvV-Gj; Sat, 09 Oct 2021 15:43:00 +0000 Received: from cloudserver094114.home.pl ([79.96.170.134]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mZEUz-005luU-0h for linux-arm-kernel@lists.infradead.org; Sat, 09 Oct 2021 15:42:58 +0000 Received: from localhost (127.0.0.1) (HELO v370.home.net.pl) by /usr/run/smtp (/usr/run/postfix/private/idea_relay_lmtp) via UNIX with SMTP (IdeaSmtpServer 3.0.0) id 5a75a4ada7a1e827; Sat, 9 Oct 2021 17:42:55 +0200 Received: from kreacher.localnet (89-77-51-84.dynamic.chello.pl [89.77.51.84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by v370.home.net.pl (Postfix) with ESMTPSA id 3D480661ECD; Sat, 9 Oct 2021 17:42:54 +0200 (CEST) From: "Rafael J. Wysocki" To: Maulik Shah , Ulf Hansson Cc: "Rafael J . Wysocki" , Daniel Lezcano , Linux PM , Peter Zijlstra , Vincent Guittot , Len Brown , Bjorn Andersson , Linux ARM , Linux Kernel Mailing List , Srinivas Rao L Subject: Re: [PATCH 1/2] cpuidle: Avoid calls to cpuidle_resume|pause() for s2idle Date: Sat, 09 Oct 2021 17:42:53 +0200 Message-ID: <4665489.GXAFRqVoOG@kreacher> In-Reply-To: References: <20210929144451.113334-1-ulf.hansson@linaro.org> <07e6821c-c221-e90d-c977-4d6b55c1ab8d@codeaurora.org> MIME-Version: 1.0 X-CLIENT-IP: 89.77.51.84 X-CLIENT-HOSTNAME: 89-77-51-84.dynamic.chello.pl X-VADE-SPAMSTATE: clean X-VADE-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedvtddrudeljedguddtfecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfjqffogffrnfdpggftiffpkfenuceurghilhhouhhtmecuudehtdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurhephffvufffkfgjfhgggfgtsehtufertddttdejnecuhfhrohhmpedftfgrfhgrvghlucflrdcuhgihshhotghkihdfuceorhhjfiesrhhjfiihshhotghkihdrnhgvtheqnecuggftrfgrthhtvghrnhepvdejlefghfeiudektdelkeekvddugfeghffggeejgfeukeejleevgffgvdeluddtnecukfhppeekledrjeejrdehuddrkeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepkeelrdejjedrhedurdekgedphhgvlhhopehkrhgvrggthhgvrhdrlhhotggrlhhnvghtpdhmrghilhhfrhhomhepfdftrghfrggvlhculfdrucghhihsohgtkhhifdcuoehrjhifsehrjhifhihsohgtkhhirdhnvghtqedprhgtphhtthhopehmkhhshhgrhhestghouggvrghurhhorhgrrdhorhhgpdhrtghpthhtohepuhhlfhdrhhgrnhhsshhonheslhhinhgrrhhordhorhhgpdhrtghpthhtoheprhgrfhgrvghlsehkvghrnhgvlhdrohhrghdprhgtphhtthhopegurghnihgvlhdrlhgviigtrghnoheslhhinhgrrhhordhorhhgpdhrtghpthhtoheplhhinhhugidqphhmsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthht ohepphgvthgvrhiisehinhhfrhgruggvrggurdhorhhgpdhrtghpthhtohepvhhinhgtvghnthdrghhuihhtthhotheslhhinhgrrhhordhorhhgpdhrtghpthhtoheplhgvnhdrsghrohifnhesihhnthgvlhdrtghomhdprhgtphhtthhopegsjhhorhhnrdgrnhguvghrshhsohhnsehlihhnrghrohdrohhrghdprhgtphhtthhopehlihhnuhigqdgrrhhmqdhkvghrnhgvlheslhhishhtshdrihhnfhhrrgguvggrugdrohhrghdprhgtphhtthhopehlihhnuhigqdhkvghrnhgvlhesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehlshhrrghosegtohguvggruhhrohhrrgdrohhrgh X-DCC--Metrics: v370.home.net.pl 1024; Body=12 Fuz1=12 Fuz2=12 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211009_084257_284506_E38A2F30 X-CRM114-Status: GOOD ( 37.81 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wednesday, October 6, 2021 3:10:55 PM CEST Ulf Hansson wrote: > On Wed, 6 Oct 2021 at 12:22, Maulik Shah wrote: > > > > Hi, > > > > On 9/29/2021 8:14 PM, Ulf Hansson wrote: > > > In s2idle_enter(), cpuidle_resume|pause() are invoked to re-allow calls to > > > the cpuidle callbacks during s2idle operations. This is needed because > > > cpuidle is paused in-between in dpm_suspend_noirq() and dpm_resume_noirq(). > > > > > > However, calling cpuidle_resume|pause() from s2idle_enter() looks a bit > > > superfluous, as it also causes all CPUs to be waken up when the first CPU > > > wakes up from s2idle. > > > > Thanks for the patch. This can be good optimization to avoid waking up > > all CPUs always. > > > > > > > > Therefore, let's drop the calls to cpuidle_resume|pause() from > > > s2idle_enter(). To make this work, let's also adopt the path in the > > > cpuidle_idle_call() to allow cpuidle callbacks to be invoked for s2idle, > > > even if cpuidle has been paused. > > > > > > Signed-off-by: Ulf Hansson > > > --- > > > drivers/cpuidle/cpuidle.c | 7 ++++++- > > > include/linux/cpuidle.h | 2 ++ > > > kernel/power/suspend.c | 2 -- > > > kernel/sched/idle.c | 40 ++++++++++++++++++++++----------------- > > > 4 files changed, 31 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > > > index ef2ea1b12cd8..c76747e497e7 100644 > > > --- a/drivers/cpuidle/cpuidle.c > > > +++ b/drivers/cpuidle/cpuidle.c > > > @@ -49,7 +49,12 @@ void disable_cpuidle(void) > > > bool cpuidle_not_available(struct cpuidle_driver *drv, > > > struct cpuidle_device *dev) > > > { > > > - return off || !initialized || !drv || !dev || !dev->enabled; > > > + return off || !drv || !dev || !dev->enabled; > > > +} > > > + > > > +bool cpuidle_paused(void) > > > +{ > > > + return !initialized; > > > } > > > > > > /** > > > diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h > > > index fce476275e16..51698b385ab5 100644 > > > --- a/include/linux/cpuidle.h > > > +++ b/include/linux/cpuidle.h > > > @@ -165,6 +165,7 @@ extern void cpuidle_pause_and_lock(void); > > > extern void cpuidle_resume_and_unlock(void); > > > extern void cpuidle_pause(void); > > > extern void cpuidle_resume(void); > > > +extern bool cpuidle_paused(void); > > > extern int cpuidle_enable_device(struct cpuidle_device *dev); > > > extern void cpuidle_disable_device(struct cpuidle_device *dev); > > > extern int cpuidle_play_dead(void); > > > @@ -204,6 +205,7 @@ static inline void cpuidle_pause_and_lock(void) { } > > > static inline void cpuidle_resume_and_unlock(void) { } > > > static inline void cpuidle_pause(void) { } > > > static inline void cpuidle_resume(void) { } > > > +static inline bool cpuidle_paused(void) {return true; } > > > static inline int cpuidle_enable_device(struct cpuidle_device *dev) > > > {return -ENODEV; } > > > static inline void cpuidle_disable_device(struct cpuidle_device *dev) { } > > > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > > > index eb75f394a059..388a5de4836e 100644 > > > --- a/kernel/power/suspend.c > > > +++ b/kernel/power/suspend.c > > > @@ -97,7 +97,6 @@ static void s2idle_enter(void) > > > raw_spin_unlock_irq(&s2idle_lock); > > > > > > cpus_read_lock(); > > > - cpuidle_resume(); > > > > > > /* Push all the CPUs into the idle loop. */ > > > wake_up_all_idle_cpus(); > > > > wake_up_all_idle_cpus() will still cause all CPUs to be woken up when > > first cpu wakes up. > > > > say for example, > > 1. device goes to s2idle suspend. > > 2. one CPU wakes up to handle irq (irq is not a wake irq but left > > enabled at GIC because of IRQF_NOSUSPEND flag) so such irq will not > > break suspend. > > 3. The cpu handles the irq. > > 4. same cpu don't break s2idle_loop() and goes to s2idle_enter() where > > it wakes up all existing idle cpus due to wake_up_all_idle_cpus() > > 5. all of CPUs again enter s2idle. > > > > to avoid waking up all CPUs in above case, something like below snip may > > help (i have not tested yet), > > > > when CPUs are in s2idle_loop(), > > > > 1. set the s2idle state to enter. > > 2. wake up all cpus from shallow state, so that they can re-enter > > deepest state. > > 3. Forever loop until a break with some wake irq. > > 4. clear the s2idle state. > > 5. wake up all cpus from deepest state so that they can now stay in > > shallow state/running state. > > > > void s2idle_loop(void) > > { > > > > + s2idle_state = S2IDLE_STATE_ENTER; > > + /* Push all the CPUs to enter deepest available state */ > > + wake_up_all_idle_cpus(); > > for (;;) { > > if (s2idle_ops && s2idle_ops->wake) { > > if (s2idle_ops->wake()) So generally you don't know what will break loose in the ->wake() callback and so you don't know what idle states the CPUs will end up in before s2idle_enter(). You could optimize for the case when ->wake() is not present, though. > > .. > > s2idle_enter(); > > } > > + s2idle_state = S2IDLE_STATE_NONE; > > + /* Push all the CPUs to enter default_idle() from this point */ > > + wake_up_all_idle_cpus(); > > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel