Re: [fixup][PATCH 2/6] ACPI / hotplug: Refuse to hot-remove allobjects with disabled hotplug

From: Toshi Kani
Date: Tue Nov 05 2013 - 19:43:53 EST


On Wed, 2013-11-06 at 00:27 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> In theory, an ACPI device object may be the parent of another
> device object whose hotplug is disabled by user space through its
> scan handler. In that case, the eject operation targeting the
> parent should fail as though the parent's own hotplug was disabled,
> but currently this is not the case, because acpi_scan_hot_remove()
> doesn't check the disable/enable hotplug status of the children
> of the top-most object passed to it.
>
> To fix this, modify acpi_bus_offline_companions() to return an
> error code if hotplug is disabled for the given device object.
> [Also change the name of the function to acpi_bus_offline(),
> because it is not only about companions any more, and change
> the name of acpi_bus_online_companions() accordingly.] Make
> acpi_scan_hot_remove() propagate that error to its callers.
>
:
> +static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
> + void **ret_p)
> {
> struct acpi_device *device = NULL;
> struct acpi_device_physical_node *pn;
> @@ -214,26 +220,32 @@ static int acpi_scan_hot_remove(struct a
> * If the first pass is successful, the second one isn't needed, though.
> */
> errdev = NULL;
> - acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> - NULL, acpi_bus_offline_companions,
> - (void *)false, (void **)&errdev);
> - acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
> + status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> + NULL, acpi_bus_offline, (void *)false,
> + (void **)&errdev);
> + if (status == AE_SUPPORT) {
> + dev_warn(errdev, "Offline disabled.\n");
> + acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> + acpi_bus_online, NULL, NULL, NULL);
> + put_device(&device->dev);
> + return -EPERM;
> + }
> + acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
> if (errdev) {

If the target object failed with AE_SUPPORT, shouldn't we skip the 2nd
pass and return with -EPERM after rollback?

Thanks,
-Toshi


> errdev = NULL;
> acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> - NULL, acpi_bus_offline_companions,
> - (void *)true , (void **)&errdev);
> + NULL, acpi_bus_offline, (void *)true,
> + (void **)&errdev);
> if (!errdev || acpi_force_hot_remove)
> - acpi_bus_offline_companions(handle, 0, (void *)true,
> - (void **)&errdev);
> + acpi_bus_offline(handle, 0, (void *)true,
> + (void **)&errdev);
>
> if (errdev && !acpi_force_hot_remove) {
> dev_warn(errdev, "Offline failed.\n");
> - acpi_bus_online_companions(handle, 0, NULL, NULL);
> + acpi_bus_online(handle, 0, NULL, NULL);
> acpi_walk_namespace(ACPI_TYPE_ANY, handle,
> - ACPI_UINT32_MAX,
> - acpi_bus_online_companions, NULL,
> - NULL, NULL);
> + ACPI_UINT32_MAX, acpi_bus_online,
> + NULL, NULL, NULL);
> put_device(&device->dev);
> return -EBUSY;
> }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/