From: Nitish Ambastha <nits.ambastha@gmail.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Nitish Ambastha <nitish.a@samsung.com>,
pavel@ucw.cz, len.brown@intel.com, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, cpgs@samsung.com
Subject: Re: [PATCHv3 1/1] kernel/power/autosleep.c: check for pm_suspend() return before queueing suspend again
Date: Thu, 23 Jul 2015 01:41:51 +0530 [thread overview]
Message-ID: <CAHWDpXiVRrwuVy=0_w_YkYySWyaC7CrJ5b3jY0atD4WxBZM-=g@mail.gmail.com> (raw)
In-Reply-To: <1909321.djDBBPKRbt@vostro.rjw.lan>
Hi Rafael,
Thank you very much for your comments and feedback
I have planned to drop this one now. Before I close, I would just
share few more comments and idea behind this.
Before queueing next suspend, we track wakeup event counts
consistently for drivers registered to wakeup events, to know if
events are pending or completed. However we cannot identify or track
the same for other drivers which are not registered with wakeup
events.
Since we can't track the events and behavior of these drivers, we
cannot predict if the next attempt of suspend will be successful on
those drivers, while the current attempt failed. This resulting in a
possibility of tight suspend resume if there is suspend failures by
these drivers more frequently.
So, my opinion was to prevent immediate suspend requests when some
drivers (which are not registered to wakeup events) failed to suspend
in current attempt. I might be wrong, but this is how I understand it.
Regards
Nitish Ambastha
On Thu, Jul 16, 2015 at 3:59 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, July 14, 2015 09:34:08 AM Nitish Ambastha wrote:
>> On Tue, Jul 14, 2015 at 5:13 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Tuesday, July 14, 2015 01:38:02 AM Nitish Ambastha wrote:
>> >> Prevent tight loop for suspend-resume when some
>> >> devices failed to suspend
>> >
>> > This *still* doesn't explain what problem you're *really* trying to address.
>> >
>> > Even if a driver returns an error code from one of its suspend callbacks,
>> > you should get final_count == initial_count in the final check and we'll
>> > schedule the timeout.
>> >
>> > So there is a failure scenarion you're trying to address where that check is
>> > not sufficient, but you're not saying what the scenario is.
>> >
>> As I mentioned earlier, if some driver failed to suspend, and during
>> resume if *somebody* called pm_stay_awake() or pm_wakeup_event()
>> meantime, and then pm_relax(), final_count and initial_count will not
>> be the same in try_to_suspend(). We observed this behavior with
>> battery monitor thread on being restarted
>
> But that means there was a valid wakeup event, doesn't it?
>
>> In these scenarios, it will be considered a *valid wakeup* event and
>> it will try to queue suspend immediately, though the actual reason of
>> resume was driver returning error code.
>
> Even if a wakeup event occurs in addition to a driver failing the suspend, it
> is still valid.
>
> So it looks like you want to schedule the timeout unconditionally in case of
> a failed suspend, but then you need to filter out -EBUSY (which is returned
> on valid wakeup events). Essentially, that would slow down autosleep, but
> how does that help exactly?
>
> Thanks,
> Rafael
>
prev parent reply other threads:[~2015-07-22 20:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-29 12:34 [PATCH 1/1] kernel/power/autosleep.c: check for pm_suspend() return before queueing suspend again Nitish Ambastha
2015-06-29 9:21 ` Pavel Machek
2015-06-29 18:54 ` [PATCHv2 " Nitish Ambastha
2015-06-29 19:56 ` Rafael J. Wysocki
2015-06-29 20:07 ` Rafael J. Wysocki
2015-06-30 19:22 ` Nitish Ambastha
2015-06-30 20:01 ` Rafael J. Wysocki
2015-07-04 18:06 ` Nitish Ambastha
2015-07-13 20:08 ` [PATCHv3 " Nitish Ambastha
2015-07-13 20:24 ` Nitish Ambastha
2015-07-13 23:43 ` Rafael J. Wysocki
2015-07-14 4:04 ` Nitish Ambastha
2015-07-15 22:29 ` Rafael J. Wysocki
2015-07-22 20:11 ` Nitish Ambastha [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHWDpXiVRrwuVy=0_w_YkYySWyaC7CrJ5b3jY0atD4WxBZM-=g@mail.gmail.com' \
--to=nits.ambastha@gmail.com \
--cc=cpgs@samsung.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=nitish.a@samsung.com \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.