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

Greg Kroah-Hartman
2013-01-22T23:49:38Z

On Tue, Jan 22, 2013 at 03:26:29AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> 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@intel.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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