public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	Greg KH <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Len Brown <lenb@kernel.org>, Ingo Molnar <mingo@elte.hu>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	pm list <linux-pm@lists.linux-foundation.org>
Subject: Re: [PATCH] PM: Acquire device locks on suspend
Date: Mon, 7 Jan 2008 17:51:59 +0100	[thread overview]
Message-ID: <200801071752.00138.rjw@sisk.pl> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0801071039330.4609-100000@iolanthe.rowland.org>

On Monday, 7 of January 2008, Alan Stern wrote:
> Let's try to summarize the main issues here:
> 
>      1. We want the PM core to lock all devices during suspend and
> 	hibernation.  This implies that registration and unregistration
> 	at such times can't work, because they need to lock the
> 	device sem in order to make probe and remove method calls.
> 
>      2. Registration calls can be failed, with an error message in the
> 	system log.  However unregistration calls cannot fail.  They
> 	_can_ block until the system resumes, but if the unregistration
> 	call was made from within a suspend or resume method it will
> 	deadlock.  This seems inescapable, but at least we should print
> 	an error in the log so the offending driver can be identified.
> 
>      3. In response to 2, the PM core should have a special routine for
> 	unregistering devices while a suspend is in progress.  Rafael
> 	proposed that the core should unlock the device to permit the
> 	call to go through.  This seems dangerous to me; I would prefer
> 	to leave the locks in place and defer the unregistration until
> 	after the system is back up and the locks have all been 
> 	dropped.  This would avoid all sorts of locking, deadlock, and 
> 	mutual exclusion problems.
> 
> (As a side note: destroy_suspended_device() has a rather limited
> interface anyway, since it can handle only devices that were created by
> create_device().)
> 
>      4. Rafael pointed out that unregistration can occur concurrently
> 	with system suspend.  When this happens we can end up trying to
> 	suspend a device which has already been through 
> 	bus_remove_device(), because it hasn't yet been removed from 
> 	the dpm_active list.  He proposes we make unregistration block
> 	system suspend, just as registration does.
> 
> I don't see 4 as a real problem.  Starting an unregistration before
> the suspend and finishing it afterward should be okay.  Once a device
> has gone through bus_remove_device() it hasn't got a suspend method any
> more, so trying to suspend it won't do anything at all -- the tests in
> suspend_device() will all fail.  Conversely, if bus_remove_device()  
> hasn't run yet then we would end up calling the driver's suspend method
> before the device_del() call returns.  As Johannes pointed out, this is
> a normal race that would exist anyway.
> 
> On the other hand, having unregistration block system suspend wouldn't 
> actually be wrong.  I simply don't think it is necessary.  But note 
> that making the two mutually exclusive would complicate Rafael's 
> synchronous approach for destroy_suspended_device().
> 
>      5. All the discussion about pm_sleep_rwsem and so on is 
> 	implementation details.  Once we have settled on the correct
> 	approach for 1-4, the implementation should be relatively easy.

Please see the patch at: http://lkml.org/lkml/2008/1/6/298 .  It represents my
current idea about how to do that.

Thanks,
Rafael

  reply	other threads:[~2008-01-07 16:49 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-05 18:36 [PATCH] PM: Acquire device locks on suspend Rafael J. Wysocki
2008-01-05 20:08 ` Alan Stern
2008-01-05 20:19   ` Rafael J. Wysocki
2008-01-05 20:39     ` Alan Stern
2008-01-05 21:13       ` Rafael J. Wysocki
2008-01-05 21:41         ` Alan Stern
2008-01-05 21:58           ` Rafael J. Wysocki
2008-01-06  4:04             ` Alan Stern
2008-01-06 13:19               ` Rafael J. Wysocki
2008-01-06 17:06                 ` Alan Stern
2008-01-06 19:05                   ` Rafael J. Wysocki
2008-01-06 19:57                     ` Rafael J. Wysocki
2008-01-06 22:19                       ` Rafael J. Wysocki
2008-01-06 22:21                       ` Alan Stern
2008-01-06 22:34                         ` Rafael J. Wysocki
2008-01-06 22:39                           ` Alan Stern
2008-01-06 22:47                             ` Rafael J. Wysocki
2008-01-06 23:46                               ` Johannes Berg
     [not found]                               ` <49505.::ffff:91.5.86.36.1199663162.squirrel@secure.sipsolutions.net>
2008-01-06 23:59                                 ` [linux-pm] " Rafael J. Wysocki
2008-01-07  0:49                                   ` Rafael J. Wysocki
2008-01-07 16:16                               ` Alan Stern
2008-01-07 16:51                                 ` Rafael J. Wysocki [this message]
2008-01-07 17:23                                   ` Alan Stern
2008-01-07 18:01                                     ` Rafael J. Wysocki
2008-01-07 19:29                                       ` Alan Stern
2008-01-07 20:37                                         ` Rafael J. Wysocki
2008-01-07 21:32                                           ` Alan Stern
2008-01-08  0:25                                             ` Rafael J. Wysocki
2008-01-09 21:01                                               ` Alan Stern
2008-01-09 22:14                                                 ` Rafael J. Wysocki
2008-01-09 22:46                                                   ` Alan Stern
2008-01-09 23:29                                                     ` Rafael J. Wysocki
2008-01-10 15:35                                                       ` Alan Stern
2008-01-10 16:59                                                         ` Rafael J. Wysocki
2008-01-10 17:04                                                           ` Alan Stern
2008-01-06 22:11                     ` Alan Stern
2008-01-06 22:24                       ` Rafael J. Wysocki
2008-01-06 22:31                         ` Alan Stern

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=200801071752.00138.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@suse.de \
    --cc=johannes@sipsolutions.net \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=mingo@elte.hu \
    --cc=stern@rowland.harvard.edu \
    /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