linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Matthew Garrett <matthew.garrett@nebula.com>,
	Yinghai Lu <yinghai@kernel.org>, Jiang Liu <liuj97@gmail.com>,
	Toshi Kani <toshi.kani@hp.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [RFC] ACPI scan handlers
Date: Thu, 24 Jan 2013 01:26:56 +0100	[thread overview]
Message-ID: <1873429.MS5RQDxTye@vostro.rjw.lan> (raw)

Hi All,

There is a considerable amount of confusion in the ACPI subsystem about what
ACPI drivers are used for.  Namely, some of them are used as "normal" device
drivers that bind to devices and handle them using ACPI control methods (like
the fan or battery drivers), but some of them are just used for handling
namespace events, such as the creation or removal of device nodes (I guess it
would be fair to call that an abuse of the driver core).  These two roles are
quite distinct, which is particularly visible from the confusion about the role
of the .remove() callback.

For the "normal" drivers this callback is simply used to handle situations in
which the driver needs to be unbound from the device, because one of them
(either the device or the driver) is going away.  That operation can't really
fail, it just needs to do the necessary cleanup.

However, for the namespace events handling "drivers" .remove() means that not
only the device node in question, but generally also the whole subtree below it
needs to be prepared for removal, which may involve deleting multiple device
objects belonging to different bus types and so on and which very well may fail
(for example, those devices may be used for such things like swap or they may be
memory banks used by the kernel and it may not be safe to remove them at the
moment etc.).  Moreover, for these things the removal of the "driver" doesn't
really make sense, because it has to be there to handle the namespace events it
is designed to handle or else things will go remarkably awry in some places.

To resolve all that mess I'd like to do the following, which in part is inspired
by the recent Toshi Kani's hotplug framework proposal and in part is based on
some discussions I had with Bjorn and others (the code references made below are
based on the current contens of linux-pm.git/linux-next).

1) Introduce a special data type for "ACPI namespace event handlers" like:

struct acpi_scan_handler {
	const struct acpi_device_id *ids;
	struct list_head list_node;
	int (*attach)(struct acpi_device *adev);
	int (*untie)(struct acpi_device *adev);
	int (*reclaim)(struct acpi_device *adev);
	void (*detach)(struct acpi_device *adev);
};

  an additional ACPI device flag:

struct acpi_device_flags {
...
	u32 untied:1;
...
};

  and an additioanl field in struc acpi_device:

struct acpi_device {
...
	struct acpi_scan_handler *scan_handler;
...
};

  (the roles of these things are described below).

2) Introduce a list of struct acpi_scan_handler objects within the ACPI
   subsystem such that acpi_bus_device_attach() will search that list first and
   if there's a matching object (one whose ids match the device node), it will
   run that object's .attach() callback.

  If that returns 1, it will mean that the handler has claimed the device node
  and is now responsible for it until its .detach() callback is run.  Thus no
  driver can be bound to that device node and no other handler can claim it.
  Then, the device node's scan_handler field will be set to point to the handler
  that's claimed it and its untied flag will be cleared.

  If .attach() returns 0, it will mean that the handler has not recognized the
  device node and some other scan handlers and/or drivers may be tried for it.

  If an error code is returned, it will mean a hard error in which case the
  scanning of the namespace will have to be aborted.

  This way ACPI drivers will only be bound to device nodes that haven't been
  claimed by any scan handlers.

3) Introduce an additional function following the format of acpi_bus_trim(),
   say acpi_bus_untie(), that will walk the namespace starting at the given
   device node and execute the .untie() callbacks from the scan handlers of
   all devices as post-order callbacks.

   If the .untie() callback for the given device node returns 0, it will mean
   that it is now safe to delete that node as long as its scan handler's
   .detach() callback is executed before the deletion.  In that case, the device
   node's untied flag will be set.

   Otherwise (i.e. if an error code is returned), it will mean that the scan
   handler has vetoed the untying and the whole operation should be reversed.
   Then, acpi_bus_untie() will walk the namespace again and execute the
   .reclaim() callbacks from the scan handlers of the device nodes whose untied
   flags are set as pre-order callbacks.

   If .reclaim() returns 0, the device node's untied flag will be cleared, and
   if an error code is returned, it will remain set.

   This will allow us to prepare the subtree below the given device node for
   removal in a reversible way, if needed.  Still, though, it will be possible
   to carry out a forcible removal calling acpi_bus_trim() after
   acpi_bus_untie() even if that has returned an error code (or even
   acpi_bus_trim() without acpi_bus_untie()).

4) Make acpi_bus_device_detach() execute the .detach() callback from the
   scan handler of the device node (if the scan handler is present) and clear
   its scan_handler field along with its untied flag.  However, the untied flags
   will only be cleared after executing the .detach() callbacks, so that those
   callbacks can see whether or not the scan handlers have been successfully
   "untied" from the device nodes.

5) Make acpi_bus_hot_remove_device() (and other pieces of code where that is
   convenient) call acpi_bus_untie() before acpi_bus_trim() and bail out
   cleanly if the untying fails (i.e. is vetoed by one of the scan handlers).

That should take care of the removal problem nicely and as far as I can say
the majority of the ACPI drivers used only for handling namespace events can
be readily converted to struct acpi_scan_handler objects.

I wonder if anyone is seeing any major problems with this at the high level.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

             reply	other threads:[~2013-01-24  0:21 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-24  0:26 Rafael J. Wysocki [this message]
2013-01-25 16:52 ` [RFC] ACPI scan handlers Toshi Kani
2013-01-25 22:11   ` Rafael J. Wysocki
2013-01-25 23:07     ` Toshi Kani
2013-01-26  1:49       ` Rafael J. Wysocki
2013-01-26 14:03         ` Rafael J. Wysocki
2013-01-26 18:42 ` Jiang Liu
2013-01-26 21:46   ` Rafael J. Wysocki
2013-01-28 12:58 ` [PATCH 0/4] " Rafael J. Wysocki
2013-01-28 12:59   ` [PATCH 1/4] ACPI / scan: Introduce struct acpi_scan_handler Rafael J. Wysocki
2013-01-29  2:04     ` Yasuaki Ishimatsu
2013-01-29  2:29       ` Yasuaki Ishimatsu
2013-01-29  2:35     ` Toshi Kani
2013-01-29 11:28       ` Rafael J. Wysocki
2013-01-29 14:50         ` Toshi Kani
2013-01-29 21:32           ` Rafael J. Wysocki
2013-01-29 22:57             ` Toshi Kani
2013-01-29 23:19               ` Rafael J. Wysocki
2013-01-29 23:27                 ` Toshi Kani
2013-01-30 13:18                   ` Rafael J. Wysocki
2013-02-03  0:52                     ` [PATCH] ACPI / scan: Follow priorities of IDs when matching scan handlers Rafael J. Wysocki
2013-02-03  4:54                       ` Yinghai Lu
2013-02-03 13:05                         ` Rafael J. Wysocki
2013-02-05 23:44                       ` Toshi Kani
2013-01-28 13:00   ` [PATCH 2/4] ACPI / PCI: Make PCI root driver use struct acpi_scan_handler Rafael J. Wysocki
2013-01-28 13:00   ` [PATCH 3/4] ACPI / PCI: Make PCI IRQ link " Rafael J. Wysocki
2013-01-28 13:01   ` [PATCH 4/4] ACPI / platform: Use struct acpi_scan_handler for creating devices Rafael J. Wysocki
2013-01-29  2:20     ` Yasuaki Ishimatsu
2013-01-29 11:36       ` Rafael J. Wysocki
2013-01-29 12:30         ` [Update][PATCH " Rafael J. Wysocki
2013-01-29 23:51           ` Yasuaki Ishimatsu
2013-01-29  7:35     ` [PATCH " Mika Westerberg
2013-01-29 12:01       ` Rafael J. Wysocki
2013-01-29  8:05     ` Mika Westerberg
2013-01-29 12:02       ` Rafael J. Wysocki
2013-01-28 21:54   ` [PATCH 0/4] ACPI scan handlers Yinghai Lu
2013-01-29  0:38     ` Rafael J. Wysocki
2013-01-29  2:33   ` Toshi Kani
2013-01-30  1:58   ` Toshi Kani
2013-01-30 13:36     ` Rafael J. Wysocki
2013-02-03 23:45   ` [PATCH 0/2] ACPI scan handler for memory hotplug and container simplification Rafael J. Wysocki
2013-02-03 23:46     ` [PATCH 1/2] ACPI / scan: Make memory hotplug driver use struct acpi_scan_handler Rafael J. Wysocki
2013-02-03 23:47     ` [PATCH 2/2] ACPI / scan: Simplify container driver Rafael J. Wysocki
2013-02-06 22:32       ` Toshi Kani
2013-02-07  0:55         ` Rafael J. Wysocki
2013-02-07  0:51           ` Toshi Kani
2013-02-07  1:32             ` Rafael J. Wysocki
2013-02-07 14:32               ` Toshi Kani
2013-02-07 22:42                 ` Rafael J. Wysocki
2013-02-08  1:05                   ` Toshi Kani
2013-02-08 12:52                     ` Rafael J. Wysocki
2013-02-08 16:24                       ` Toshi Kani
2013-02-07  8:32       ` Yasuaki Ishimatsu
2013-02-07 11:43         ` Rafael J. Wysocki
2013-02-07 14:38           ` Toshi Kani
2013-02-08  0:24       ` [PATCH 0/2] ACPI / scan: Remove useless #ifndef and simplify " Rafael J. Wysocki
2013-02-08  0:25         ` [PATCH 1/2] ACPI / scan: Remove useless #ifndef from acpi_eject_store() Rafael J. Wysocki
2013-02-08  0:27         ` [PATCH 2/2] ACPI / scan: Make container driver use struct acpi_scan_handler Rafael J. Wysocki
2013-02-08  3:32           ` Yinghai Lu
2013-02-08 12:45             ` Rafael J. Wysocki
2013-02-08  3:19         ` [PATCH 0/2] ACPI / scan: Remove useless #ifndef and simplify container driver Yasuaki Ishimatsu
2013-02-08 12:46           ` Rafael J. Wysocki
2013-02-08 16:57         ` Toshi Kani
2013-02-08 19:59           ` Rafael J. Wysocki
2013-02-08 22:41             ` Rafael J. Wysocki
2013-02-08 23:18               ` [PATCH] ACPI: Drop the container.h header file Rafael J. Wysocki
2013-02-08 23:27                 ` Toshi Kani
2013-02-09 14:26         ` [PATCH 0/2] ACPI / scan: Two fixes for device hot-removal Rafael J. Wysocki
2013-02-09 14:29           ` [PATCH 1/2] ACPI / scan: Make acpi_bus_hot_remove_device() acquire the scan lock Rafael J. Wysocki
2013-02-11 23:42             ` Toshi Kani
2013-02-09 14:31           ` [PATCH 2/2] ACPI / scan: Full transition to D3cold in acpi_device_unregister() 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=1873429.MS5RQDxTye@vostro.rjw.lan \
    --to=rjw@sisk.pl \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuj97@gmail.com \
    --cc=matthew.garrett@nebula.com \
    --cc=mika.westerberg@linux.intel.com \
    --cc=toshi.kani@hp.com \
    --cc=yinghai@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).