Re: [PATCH 2/4] ACPI / PM: Expose current status of ACPI powerresources

From: Greg Kroah-Hartman
Date: Tue Jan 22 2013 - 18:49:20 EST


On Tue, Jan 22, 2013 at 03:26:29AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Since ACPI power resources are going to be used more extensively on
> new hardware platforms, it becomes necessary for user space (powertop
> in particular) to observe some properties of those resources for
> diagnostics purposes.
>
> For this reason, expose the current status of each ACPI power
> resource to user space via sysfs by adding a new resource_in_use
> attribute to the sysfs directory representing the given power
> resource.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

But, as with patch 1/4, this is racy:

> @@ -723,6 +744,9 @@ int acpi_add_power_resource(acpi_handle
> if (result)
> goto err;
>
> + if (!device_create_file(&device->dev, &dev_attr_resource_in_use))
> + device->remove = acpi_power_sysfs_remove;
> +

You are creating the file after userspace has been told that the device
is present.

As this looks like the first file this type of device has, maybe you
should just fix this up "properly" here, and not wait to do it later?

Your choice,

greg k-h
--
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/