All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Linux PM mailing list <linux-pm@lists.linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Jesse Barnes <jbarnes@virtuousgeek.org>
Subject: Re: [PATCH] PCI / PM: Block races between runtime PM and system sleep
Date: Wed, 22 Jun 2011 10:20:28 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1106221011240.1977-100000__35897.6957192228$1308752586$gmane$org@iolanthe.rowland.org> (raw)
In-Reply-To: <201106220149.17380.rjw@sisk.pl>

On Wed, 22 Jun 2011, Rafael J. Wysocki wrote:

> > It's probably okay to prevent pm_runtime_suspend() from working during
> > .suspend() or .resume(), but it's not a good idea to prevent
> > pm_runtime_resume() from working then.
> 
> OK, but taking a reference by means of pm_runtime_get_noresume() won't
> block pm_runtime_resume().

Exactly my point -- we don't need to (and don't want to) block 
pm_runtime_resume during the .suspend() and .resume() callbacks.

> > During the .suspend and .resume callbacks, races with
> > .runtime_suspend() can be prevented by calling
> > pm_runtime_get_noresume() just before .suspend() and then calling
> > pm_runtime_put_sync() just after .resume().
> 
> So, you seem to suggest to call pm_runtime_get_noresume() in
> __device_suspend() and pm_runtime_put_sync() in device_resume().

Yes.  Also perhaps call pm_runtime_barrier() immediately after 
get_noresume.

> That would be fine by me, perhaps up to the "sync" part of the "put".

The main feature of this design is that it allows runtime PM to work 
between .resume() and .complete().  If you do a put_noidle instead of 
put_sync then you may prevent runtime PM from working properly.

> > >  (a _put_sync() at this point will likely invoke
> > > .runtime_idle() from the subsystem before executing .complete(), which may
> > > not be desirable)?
> > 
> > It should be allowed.  The purpose of .complete() is not to re-enable
> > runtime power management of the device; it is to release resources
> > (like memory) allocated during .prepare() and perhaps also to allow new
> > children to be registered under the device.
> 
> Right.  But does "allowed" mean the core _should_ do it at this point?
> We may as well call pm_runtime_idle() directly from rpm_complete(), but
> perhaps it's better to call it from device_resume(), so that it runs in
> parallel for async devices.

Calling pm_runtime_put_noidle() followed by pm_runtime_idle() is 
essentially the same as calling pm_runtime_put_sync() anyway.

If a subsystem really does want to block runtime PM between the 
.resume() and .complete() callbacks, it can do its own get_noresume and 
put_sync -- just as you have done with PCI.

Alan Stern

  reply	other threads:[~2011-06-22 14:20 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-19 19:49 [PATCH] PCI / PM: Block races between runtime PM and system sleep Rafael J. Wysocki
2011-06-20 14:46 ` Alan Stern
2011-06-20 14:46 ` Alan Stern
2011-06-20 19:42   ` Rafael J. Wysocki
2011-06-20 19:42   ` Rafael J. Wysocki
2011-06-20 21:00     ` Alan Stern
2011-06-20 21:28       ` Rafael J. Wysocki
2011-06-20 21:28       ` Rafael J. Wysocki
2011-06-21 14:52         ` Alan Stern
2011-06-21 14:52         ` Alan Stern
2011-06-21 23:49           ` Rafael J. Wysocki
2011-06-21 23:49           ` Rafael J. Wysocki
2011-06-22 14:20             ` Alan Stern [this message]
2011-06-22 14:20             ` Alan Stern
2011-06-23 17:46               ` Rafael J. Wysocki
2011-06-23 17:46               ` Rafael J. Wysocki
2011-06-23 18:35                 ` Alan Stern
2011-06-23 20:49                   ` Rafael J. Wysocki
2011-06-23 20:49                   ` Rafael J. Wysocki
2011-06-23 21:02                     ` Alan Stern
2011-06-23 21:02                     ` Alan Stern
2011-06-23 21:16                       ` Rafael J. Wysocki
2011-06-23 21:16                       ` Rafael J. Wysocki
2011-06-23 21:38                         ` Alan Stern
2011-06-23 22:35                           ` Rafael J. Wysocki
2011-06-23 22:35                           ` Rafael J. Wysocki
2011-06-23 22:59                             ` Rafael J. Wysocki
2011-06-23 22:59                             ` Rafael J. Wysocki
2011-06-26  2:39                               ` Alan Stern
2011-06-26 12:22                                 ` Rafael J. Wysocki
2011-06-26 12:22                                   ` Rafael J. Wysocki
2011-06-26  2:39                               ` Alan Stern
2011-06-23 21:38                         ` Alan Stern
2011-06-23 18:35                 ` Alan Stern
2011-06-20 21:00     ` Alan Stern
2011-06-19 19:49 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='Pine.LNX.4.44L0.1106221011240.1977-100000__35897.6957192228$1308752586$gmane$org@iolanthe.rowland.org' \
    --to=stern@rowland.harvard.edu \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=rjw@sisk.pl \
    /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.