From: Alan Stern <stern@rowland.harvard.edu>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Linux PM mailing list <linux-pm@lists.linux-foundation.org>,
LKML <linux-kernel@vger.kernel.org>,
Jesse Barnes <jbarnes@virtuousgeek.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH] PCI / PM: Block races between runtime PM and system sleep
Date: Tue, 21 Jun 2011 10:52:05 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.44L0.1106211031410.2327-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <201106202328.54100.rjw@sisk.pl>
On Mon, 20 Jun 2011, Rafael J. Wysocki wrote:
> > Ah, okay. The PCI part makes sense then.
>
> OK, so the appended patch is a modification of the $subject one using
> pm_runtime_put_sync() instead of pm_runtime_put_noidle().
Yes, it looks good.
> So, your point is that while .suspend() or .resume() are running, the
> synchronization between runtime PM and system suspend/resume should be the
> subsystem's problem, right?
Almost but not quite. I was talking about the time period between
.prepare() and .suspend() (and also the time period between .resume()
and .complete()).
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.
> I actually see a reason for doing this. Namely, I don't really think
> driver writers should be bothered with preventing races between different
> PM callbacks from happening. Runtime PM takes care of that at run time,
> the design of the system suspend/resume code ensures that the callbacks
> for the same device are executed sequentially, but if we allow runtime PM
> callbacks to be executed in parallel with system suspend/resume callbacks,
> someone has to prevent those callbacks from racing with each other.
>
> Now, if you agree that that shouldn't be a driver's task, then it has to
> be the subsystem's one and I'm not sure what a subsystem can do other than
> disabling runtime PM or at least taking a reference on every device before
> calling device drivers' .suspend() callbacks.
>
> Please note, I think that .prepare() and .complete() are somewhat special,
> so perhaps we should allow those to race with runtime PM callbacks, but IMO
> allowing .suspend() and .resume() to race with .runtime_suspend() and
> .runtime_resume() is not a good idea.
Races in the period after .suspend() and before .resume() will be
handled by disabling runtime PM when .suspend() returns and enabling it
before calling .resume().
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().
Races with .runtime_resume() can be handled to some extent by putting a
runtime barrier immediately after the pm_runtime_get_noresume() call,
but that's not a perfect solution. Is it good enough?
> > What I'm suggesting is to revert the commit but at the same time,
> > move the get_noresume() into __device_suspend() and the put_sync() into
> > device_resume().
>
> What about doing pm_runtime_get_noresume() and the pm_runtime_barrier()
> in dpm_prepare(), but _after_ calling device_prepare() and doing
> pm_runtime_put_noidle() in dpm_complete() _before_ calling .complete()
> from the subsystem
This does not address the issue of allowing runtime suspends in the
windows between .prepare() - .suspend() and .resume() - .complete().
> (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.
Alan Stern
next prev parent reply other threads:[~2011-06-21 14:52 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 [this message]
2011-06-21 23:49 ` Rafael J. Wysocki
2011-06-21 23:49 ` Rafael J. Wysocki
2011-06-22 14:20 ` Alan Stern
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.1106211031410.2327-100000@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.