public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>, Pavel Machek <pavel@ucw.cz>,
	pm list <linux-pm@lists.linux-foundation.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Greg KH <greg@kroah.com>, Len Brown <lenb@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Alexey Starikovskiy <astarikovskiy@suse.de>,
	David Brownell <david-b@pacbell.net>
Subject: Re: [RFC][PATCH 1/3] PM: Introduce new top level suspend and hibernation callbacks
Date: Thu, 20 Mar 2008 14:33:28 +1100	[thread overview]
Message-ID: <1205984008.26869.408.camel@pasglop> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0803191101050.4958-100000@netrider.rowland.org>


> That was in fact the original plan, before Rafael changed over to
> calling prepare() just prior to suspend().

Heh, ok :-)

> Your analysis is right.  Removal isn't a problem; we don't care if 
> drivers are prohibited from registering new children when they are 
> unbound.  In general we want to make it possible to unregister devices 
> at any time during the suspend-resume procedure (except of course that 
> a device can't be unregistered while one of its methods is running).
> 
> An important point I tried to make in the earlier email is that drivers
> will want a simple way to know when it is illegal for them to register
> new children.  For example, suppose the registration is done by a
> workqueue routine.  The most reliable way for the driver to insure that
> the routine won't try to register new children improperly is to have
> the routine check a flag which gets set _before_ prepare() is called.

I don't totally agree here. Drivers could have their own flag set
internally with appropriate locking. The problem with your approach is
locking. Just setting a flag is mostly useless, -unless- there
appropriate locking between setter and testers. 

For example, the flags we talked about previously, the one set _after_
prepare that causes device_add() to fail, this one must have its setting
done within the same mutex/lock as device_add() internally uses to
protect the list. Or something along those lines.

Now, your idea of having the core set some other flag to tell drivers to
stop doing addition wouldn't work here because the driver would -not-
hold the core device list mutex between testing it and performing the
addition.

As a general matter, the driver would need to internally, using it's own
locking mechanisms, ensure that it will not do any further device_add,
and that's not something the core can help with. In our case, that could
be acheived by flushing the work queues and ensuring we don't queue
another one, or using some driver internal semaphore to set a "stop
adding" flag whatever... 

However, I think this is mostly a non-issue, because the core _does_
provide something here that is useful for drivers who don't want to
bother with the above: The failure return from device_add. If drivers
don't want to do something akin to what I described, they can just be
made to deal gracefully with the failure from device_add that would
happen due to the core internal flag being set after the return from
prepare().

So I don't think we need anything else.

Cheers,
Ben.



  reply	other threads:[~2008-03-20  3:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-16 23:20 [RFC][PATCH 0/3] PM: Rework suspend and hibernation code for devices Rafael J. Wysocki
2008-03-16 23:22 ` [RFC][PATCH 1/3] PM: Introduce new top level suspend and hibernation callbacks Rafael J. Wysocki
2008-03-18 10:01   ` Pavel Machek
2008-03-18 15:10     ` Alan Stern
2008-03-18 23:54       ` Pavel Machek
2008-03-19  1:04         ` Rafael J. Wysocki
2008-03-19  2:44           ` Alan Stern
2008-03-19  3:49             ` Benjamin Herrenschmidt
2008-03-19 15:19               ` Alan Stern
2008-03-20  3:33                 ` Benjamin Herrenschmidt [this message]
2008-03-20 14:45                   ` Alan Stern
2008-03-20 22:32                     ` Benjamin Herrenschmidt
2008-03-20 18:26                   ` Alan Stern
2008-03-20 22:34                     ` Benjamin Herrenschmidt
2008-03-20 22:57                       ` Alan Stern
2008-03-19  0:53   ` Greg KH
2008-03-19  9:15     ` Pavel Machek
2008-03-19 13:22     ` Rafael J. Wysocki
2008-03-19 18:07       ` Greg KH
2008-03-16 23:24 ` [RFC][PATCH 2/3] PM: New suspend and hibernation callbacks for platform bus type Rafael J. Wysocki
2008-03-18 10:02   ` Pavel Machek
2008-03-16 23:25 ` [RFC][PATCH 3/3] PM: New suspend and hibernation callbacks for PCI " Rafael J. Wysocki
2008-03-18 10:02   ` Pavel Machek
2008-03-19  0:55   ` Greg KH
2008-03-19 13:24     ` Rafael J. Wysocki
2008-03-19 18:06       ` Greg KH

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=1205984008.26869.408.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=astarikovskiy@suse.de \
    --cc=david-b@pacbell.net \
    --cc=greg@kroah.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=pavel@ucw.cz \
    --cc=rjw@sisk.pl \
    --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