From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752440AbbF3TfH (ORCPT ); Tue, 30 Jun 2015 15:35:07 -0400 Received: from v094114.home.net.pl ([79.96.170.134]:52239 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751949AbbF3TfD (ORCPT ); Tue, 30 Jun 2015 15:35:03 -0400 From: "Rafael J. Wysocki" To: Nitish Ambastha Cc: Nitish Ambastha , pavel@ucw.cz, len.brown@intel.com, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, cpgs@samsung.com Subject: Re: [PATCHv2 1/1] kernel/power/autosleep.c: check for pm_suspend() return before queueing suspend again Date: Tue, 30 Jun 2015 22:01:19 +0200 Message-ID: <3120244.uQuxSPo26i@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/4.1.0-rc5+; KDE/4.11.5; x86_64; ; ) In-Reply-To: References: <1435581297-20271-1-git-send-email-nitish.a@samsung.com> <2619760.zM2C8Nu9I9@vostro.rjw.lan> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, July 01, 2015 12:52:43 AM Nitish Ambastha wrote: > Hi Rafael > > Thanks for your feedback > > On Tue, Jun 30, 2015 at 1:37 AM, Rafael J. Wysocki wrote: > > On Monday, June 29, 2015 09:56:18 PM Rafael J. Wysocki wrote: > >> On Tuesday, June 30, 2015 12:24:14 AM Nitish Ambastha wrote: > >> > Prevent tight loop for suspend-resume when some > >> > devices failed to suspend > >> > If some devices failed to suspend, we monitor this > >> > error in try_to_suspend(). pm_suspend() is already > >> > an 'int' returning function, how about checking return > >> > from pm_suspend() before queueing suspend again? > >> > > >> > For devices which do not register for pending events, > >> > this will prevent tight loop for suspend-resume in > >> > suspend abort scenarios due to device suspend failures > > > > Having said the below I'm not sure why the current code doesn't cover this > > for you? > > > > That would be the final_count == initial_count case, no? > > > Agree, this should cover most of the cases, however there are some > cases where final_count may not match initial_count here > > A couple of such scenario I came across is > 1) when tasks are restarted again due to suspend failure, sometimes > battery kernel thread acquires lock for battery monitoring resulting > in either pm_get_wakeup_count() returning false or increment in > final_count from initial_count Locks should not have any effect on the return value of pm_get_wakeup_count() and if false is returned by it, a wakeup event was being processed when it was called. In turn, if pm_get_wakeup_count() returns false or final_count != initial_count, this means that *somebody* called pm_wakeup_event() or equivalent in the meantime and there *was* a valid wakeup event (regardless of or in addition to the driver error). > 2) In some platforms, power transitions are carried from User space > (power manager), these power-manager tries to hold some wake lock > after being restarted on resume And what exactly is the failing scenario in that case? -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.