public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: "Linux-pm mailing list" <linux-pm@lists.linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Ming Lei <tom.leiming@gmail.com>
Subject: Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily
Date: Tue, 30 Nov 2010 23:27:24 +0100	[thread overview]
Message-ID: <201011302327.24222.rjw@sisk.pl> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1011301008110.1686-100000@iolanthe.rowland.org>

On Tuesday, November 30, 2010, Alan Stern wrote:
> On Mon, 29 Nov 2010, Rafael J. Wysocki wrote:
> 
> > > Oh, I see.  This is a tricky issue.  Every driver for a device that can
> > > have wakeup-enabled children needs to worry about the race between
> > > suspending the device and receiving a wakeup request from a child.  
> > > For example, in drivers/usb/core/hcd-pci.c, the suspend_common()
> > > routine goes out of its way to return -EBUSY if device_may_wakeup() is
> > > true and the controller's root hub has a pending wakeup request.
> > > 
> > > How should drivers handle this in general?  Should we make an effort to
> > > convert them to use the wakeup framework so they they can let the PM
> > > core take care of these races?
> > 
> > I think so.
> > 
> > We also need to put a pm_check_wakeup_events() check into dpm_suspend() IMO,
> > so that we abort the suspending of devices as soon as a wakeup event is
> > reported.
> 
> You might as well add that into this patch.

I'll do that in a separate patch.

> > > Do we have to consider similar races during runtime suspend?
> > 
> > Ideally, yes, but I'm not sure if that's generally possible.  IMO, it won't be
> > a big deal if a parent device is suspended and immediately resumed occasionally
> > due to a pending wakeup signal from one of its children.  It may be a problem
> > if that happens too often, though.
> 
> Okay.
> 
> > Does it mean you're fine with the patch?
> 
> Provided you repair the error that Lei Ming pointed out.  That's the 
> problem with functions that return Boolean values -- you have to name 
> them very carefully.  Ideally the name should be a predicate or a
> question.

I already have fixed it.

The name is unfortunate indeed, perhaps it's better to call that function
pm_new_wakeup_events() or something like this.

Thanks,
Rafael

  reply	other threads:[~2010-11-30 22:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-28 12:12 [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily Rafael J. Wysocki
2010-11-28 15:35 ` Alan Stern
2010-11-28 22:52   ` Rafael J. Wysocki
2010-11-29  3:05     ` Alan Stern
2010-11-29 22:04       ` Rafael J. Wysocki
2010-11-30 15:13         ` Alan Stern
2010-11-30 22:27           ` Rafael J. Wysocki [this message]
2010-12-01 15:15             ` Alan Stern
2010-12-01 23:50               ` Rafael J. Wysocki
2010-12-02 15:38                 ` Alan Stern
2010-12-02 19:42                   ` Rafael J. Wysocki
2010-11-30 13:07 ` Ming Lei
2010-11-30 22:23   ` Rafael J. Wysocki

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=201011302327.24222.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tom.leiming@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox