[PATCH] PM / wakeup: Avoid dev_name collisions in wakeup class

[PATCH] PM / wakeup: Avoid dev_name collisions in wakeup class

[Stephen Boyd]

If a device is wakeup capable and the driver calls device_wakeup_init()
on it during probe and then userspace writes 'enabled' to that device's
power/wakeup file in sysfs we'll try to create the same named wakeup
device in sysfs. The kernel will complain about duplicate file names.

sysfs: cannot create duplicate filename '/devices/virtual/wakeup/1-1.1'
kobject_add_internal failed for 1-1.1 with -EEXIST, don't try to register things with the same name in the same directory.

It may be advantageous to not write 'enabled' to the wakeup file (see
wakeup_store()) from userspace for these devices because we allocate
devices and register them and then throw them all away later on if the
device driver has already initialized the wakeup attribute. The
implementation currently tries to avoid taking locks here so it seems
best to optimize that path in a separate patch.

Let's rename the wakeup class devices as 'wakeupN' with an IDA that's
simple enough to just return some sort of number. In addition, let's
make the device registering the wakeup the parent and include a 'name'
attribute in case userspace wants to figure out the type of wakeup it is
(in the case of virtual wakeups) or the device associated with the
wakeup. This makes it easier for userspace to go from /sys/class/wakeup
to a place in the device hierarchy where the wakeup is generated from
like an input device.

Cc: Tri Vo <trong@android.com>
Cc: Kalesh Singh <kaleshsingh@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/acpi/device_pm.c          |  2 +-
 drivers/base/power/wakeup.c       |  8 +++++---
 drivers/base/power/wakeup_stats.c | 31 ++++++++++++++++++++++++++-----
 fs/eventpoll.c                    |  4 ++--
 include/linux/pm_wakeup.h         | 12 ++++++++----
 kernel/power/autosleep.c          |  2 +-
 kernel/power/wakelock.c           |  2 +-
 kernel/time/alarmtimer.c          |  2 +-
 8 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 28cffaaf9d82..0863be1e42d6 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -495,7 +495,7 @@ acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
 		goto out;
 
 	mutex_lock(&acpi_pm_notifier_lock);
-	adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
+	adev->wakeup.ws = wakeup_source_register(&adev->dev, dev_name(&adev->dev));
 	adev->wakeup.context.dev = dev;
 	adev->wakeup.context.func = func;
 	adev->wakeup.flags.notifier_present = true;
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 2b8def0ea59f..7ba242b49831 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -201,15 +201,17 @@ EXPORT_SYMBOL_GPL(wakeup_source_remove);
 /**
  * wakeup_source_register - Create wakeup source and add it to the list.
  * @name: Name of the wakeup source to register.
+ * @dev: Device wakeup source is associated with (or NULL if virtual)
  */
-struct wakeup_source *wakeup_source_register(const char *name)
+struct wakeup_source *wakeup_source_register(struct device *dev,
+					     const char *name)
 {
 	struct wakeup_source *ws;
 	int ret;
 
 	ws = wakeup_source_create(name);
 	if (ws) {
-		ret = wakeup_source_sysfs_add(ws);
+		ret = wakeup_source_sysfs_add(dev, ws);
 		if (ret) {
 			kfree_const(ws->name);
 			kfree(ws);
@@ -273,7 +275,7 @@ int device_wakeup_enable(struct device *dev)
 	if (pm_suspend_target_state != PM_SUSPEND_ON)
 		dev_dbg(dev, "Suspicious %s() during system transition!\n", __func__);
 
-	ws = wakeup_source_register(dev_name(dev));
+	ws = wakeup_source_register(dev, dev_name(dev));
 	if (!ws)
 		return -ENOMEM;
 
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
index 9c01150f1213..927cc84d3392 100644
--- a/drivers/base/power/wakeup_stats.c
+++ b/drivers/base/power/wakeup_stats.c
@@ -7,8 +7,9 @@
  * Copyright (c) 2019 Google Inc.
  */
 
-#include <linux/slab.h>
+#include <linux/idr.h>
 #include <linux/kdev_t.h>
+#include <linux/slab.h>
 
 #include "power.h"
 
@@ -80,6 +81,15 @@ static ssize_t last_change_ms_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(last_change_ms);
 
+static ssize_t name_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct wakeup_source *ws = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", ws->name);
+}
+static DEVICE_ATTR_RO(name);
+
 static ssize_t prevent_suspend_time_ms_show(struct device *dev,
 					    struct device_attribute *attr,
 					    char *buf)
@@ -96,6 +106,7 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev,
 static DEVICE_ATTR_RO(prevent_suspend_time_ms);
 
 static struct attribute *wakeup_source_attrs[] = {
+	&dev_attr_name.attr,
 	&dev_attr_active_count.attr,
 	&dev_attr_event_count.attr,
 	&dev_attr_wakeup_count.attr,
@@ -109,18 +120,27 @@ static struct attribute *wakeup_source_attrs[] = {
 };
 ATTRIBUTE_GROUPS(wakeup_source);
 
+static DEFINE_IDA(wakeup_ida);
+
 /**
  * wakeup_source_sysfs_add - Add wakeup_source attributes to sysfs.
  * @ws: Wakeup source to be added in sysfs.
+ * @parent: Device wakeup source is associated with (or NULL if virtual)
  */
-int wakeup_source_sysfs_add(struct wakeup_source *ws)
+int wakeup_source_sysfs_add(struct device *parent, struct wakeup_source *ws)
 {
 	struct device *dev;
 
-	dev = device_create_with_groups(wakeup_class, NULL, MKDEV(0, 0), ws,
-					wakeup_source_groups, "%s", ws->name);
-	if (IS_ERR(dev))
+	ws->id = ida_simple_get(&wakeup_ida, 0, 0, GFP_KERNEL);
+	if (ws->id < 0)
+		return ws->id;
+
+	dev = device_create_with_groups(wakeup_class, parent, MKDEV(0, 0), ws,
+					wakeup_source_groups, "wakeup%d", ws->id);
+	if (IS_ERR(dev)) {
+		ida_simple_remove(&wakeup_ida, ws->id);
 		return PTR_ERR(dev);
+	}
 
 	ws->dev = dev;
 	return 0;
@@ -134,6 +154,7 @@ EXPORT_SYMBOL_GPL(wakeup_source_sysfs_add);
 void wakeup_source_sysfs_remove(struct wakeup_source *ws)
 {
 	device_unregister(ws->dev);
+	ida_simple_remove(&wakeup_ida, ws->id);
 }
 EXPORT_SYMBOL_GPL(wakeup_source_sysfs_remove);
 
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index d7f1f5011fac..c4159bcc05d9 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1459,13 +1459,13 @@ static int ep_create_wakeup_source(struct epitem *epi)
 	struct wakeup_source *ws;
 
 	if (!epi->ep->ws) {
-		epi->ep->ws = wakeup_source_register("eventpoll");
+		epi->ep->ws = wakeup_source_register(NULL, "eventpoll");
 		if (!epi->ep->ws)
 			return -ENOMEM;
 	}
 
 	name = epi->ffd.file->f_path.dentry->d_name.name;
-	ws = wakeup_source_register(name);
+	ws = wakeup_source_register(NULL, name);
 
 	if (!ws)
 		return -ENOMEM;
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index 500f9cfe2db8..822e74f45384 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -41,6 +41,7 @@ struct wake_irq;
  */
 struct wakeup_source {
 	const char 		*name;
+	int			id;
 	struct list_head	entry;
 	spinlock_t		lock;
 	struct wake_irq		*wakeirq;
@@ -88,7 +89,8 @@ extern struct wakeup_source *wakeup_source_create(const char *name);
 extern void wakeup_source_destroy(struct wakeup_source *ws);
 extern void wakeup_source_add(struct wakeup_source *ws);
 extern void wakeup_source_remove(struct wakeup_source *ws);
-extern struct wakeup_source *wakeup_source_register(const char *name);
+extern struct wakeup_source *wakeup_source_register(struct device *dev,
+						    const char *name);
 extern void wakeup_source_unregister(struct wakeup_source *ws);
 extern int device_wakeup_enable(struct device *dev);
 extern int device_wakeup_disable(struct device *dev);
@@ -128,7 +130,8 @@ static inline void wakeup_source_add(struct wakeup_source *ws) {}
 
 static inline void wakeup_source_remove(struct wakeup_source *ws) {}
 
-static inline struct wakeup_source *wakeup_source_register(const char *name)
+static inline struct wakeup_source *wakeup_source_register(struct device *dev,
+							   const char *name)
 {
 	return NULL;
 }
@@ -186,12 +189,13 @@ static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
 #ifdef CONFIG_PM_SLEEP_STATS
 
 /* drivers/base/power/wakeup_stats.c */
-int wakeup_source_sysfs_add(struct wakeup_source *ws);
+int wakeup_source_sysfs_add(struct device *parent, struct wakeup_source *ws);
 void wakeup_source_sysfs_remove(struct wakeup_source *ws);
 
 #else /* !CONFIG_PM_SLEEP_STATS */
 
-static inline int wakeup_source_sysfs_add(struct wakeup_source *ws)
+static inline int wakeup_source_sysfs_add(struct device *parent,
+					  struct wakeup_source *ws)
 {
 	return 0;
 }
diff --git a/kernel/power/autosleep.c b/kernel/power/autosleep.c
index 41e83a779e19..9af5a50d3489 100644
--- a/kernel/power/autosleep.c
+++ b/kernel/power/autosleep.c
@@ -116,7 +116,7 @@ int pm_autosleep_set_state(suspend_state_t state)
 
 int __init pm_autosleep_init(void)
 {
-	autosleep_ws = wakeup_source_register("autosleep");
+	autosleep_ws = wakeup_source_register(NULL, "autosleep");
 	if (!autosleep_ws)
 		return -ENOMEM;
 
diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
index 32726da3d6e6..826fcd97647a 100644
--- a/kernel/power/wakelock.c
+++ b/kernel/power/wakelock.c
@@ -192,7 +192,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
 	wl->ws.name = wl->name;
 	wl->ws.last_time = ktime_get();
 
-	ret = wakeup_source_sysfs_add(&wl->ws);
+	ret = wakeup_source_sysfs_add(NULL, &wl->ws);
 	if (ret) {
 		kfree(wl->name);
 		kfree(wl);
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 57518efc3810..93b382d9701c 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -97,7 +97,7 @@ static int alarmtimer_rtc_add_device(struct device *dev,
 	if (!device_may_wakeup(rtc->dev.parent))
 		return -1;
 
-	__ws = wakeup_source_register("alarmtimer");
+	__ws = wakeup_source_register(dev, "alarmtimer");
 
 	spin_lock_irqsave(&rtcdev_lock, flags);
 	if (!rtcdev) {

base-commit: 0c826a07dd696d99784c68ec1e8def4399cc4a0b
-- 
Sent by a computer through tubes

Re: [PATCH] PM / wakeup: Avoid dev_name collisions in wakeup class

[Rafael J. Wysocki]

On Sat, Jul 27, 2019 at 3:11 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> If a device is wakeup capable and the driver calls device_wakeup_init()
> on it during probe and then userspace writes 'enabled' to that device's
> power/wakeup file in sysfs we'll try to create the same named wakeup
> device in sysfs. The kernel will complain about duplicate file names.
>
> sysfs: cannot create duplicate filename '/devices/virtual/wakeup/1-1.1'
> kobject_add_internal failed for 1-1.1 with -EEXIST, don't try to register things with the same name in the same directory.
>
> It may be advantageous to not write 'enabled' to the wakeup file (see
> wakeup_store()) from userspace for these devices because we allocate
> devices and register them and then throw them all away later on if the
> device driver has already initialized the wakeup attribute. The
> implementation currently tries to avoid taking locks here so it seems
> best to optimize that path in a separate patch.
>
> Let's rename the wakeup class devices as 'wakeupN' with an IDA that's
> simple enough to just return some sort of number. In addition, let's
> make the device registering the wakeup the parent and include a 'name'
> attribute in case userspace wants to figure out the type of wakeup it is
> (in the case of virtual wakeups) or the device associated with the
> wakeup. This makes it easier for userspace to go from /sys/class/wakeup
> to a place in the device hierarchy where the wakeup is generated from
> like an input device.
>
> Cc: Tri Vo <trong@android.com>
> Cc: Kalesh Singh <kaleshsingh@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>

I'd rather change the commit that introduced this issue which is only
in linux-next for now.

> ---
>  drivers/acpi/device_pm.c          |  2 +-
>  drivers/base/power/wakeup.c       |  8 +++++---
>  drivers/base/power/wakeup_stats.c | 31 ++++++++++++++++++++++++++-----
>  fs/eventpoll.c                    |  4 ++--
>  include/linux/pm_wakeup.h         | 12 ++++++++----
>  kernel/power/autosleep.c          |  2 +-
>  kernel/power/wakelock.c           |  2 +-
>  kernel/time/alarmtimer.c          |  2 +-
>  8 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index 28cffaaf9d82..0863be1e42d6 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
> @@ -495,7 +495,7 @@ acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
>                 goto out;
>
>         mutex_lock(&acpi_pm_notifier_lock);
> -       adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
> +       adev->wakeup.ws = wakeup_source_register(&adev->dev, dev_name(&adev->dev));
>         adev->wakeup.context.dev = dev;
>         adev->wakeup.context.func = func;
>         adev->wakeup.flags.notifier_present = true;
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 2b8def0ea59f..7ba242b49831 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -201,15 +201,17 @@ EXPORT_SYMBOL_GPL(wakeup_source_remove);
>  /**
>   * wakeup_source_register - Create wakeup source and add it to the list.
>   * @name: Name of the wakeup source to register.
> + * @dev: Device wakeup source is associated with (or NULL if virtual)
>   */
> -struct wakeup_source *wakeup_source_register(const char *name)
> +struct wakeup_source *wakeup_source_register(struct device *dev,
> +                                            const char *name)
>  {
>         struct wakeup_source *ws;
>         int ret;
>
>         ws = wakeup_source_create(name);
>         if (ws) {
> -               ret = wakeup_source_sysfs_add(ws);
> +               ret = wakeup_source_sysfs_add(dev, ws);
>                 if (ret) {
>                         kfree_const(ws->name);
>                         kfree(ws);
> @@ -273,7 +275,7 @@ int device_wakeup_enable(struct device *dev)
>         if (pm_suspend_target_state != PM_SUSPEND_ON)
>                 dev_dbg(dev, "Suspicious %s() during system transition!\n", __func__);
>
> -       ws = wakeup_source_register(dev_name(dev));
> +       ws = wakeup_source_register(dev, dev_name(dev));
>         if (!ws)
>                 return -ENOMEM;
>
> diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
> index 9c01150f1213..927cc84d3392 100644
> --- a/drivers/base/power/wakeup_stats.c
> +++ b/drivers/base/power/wakeup_stats.c
> @@ -7,8 +7,9 @@
>   * Copyright (c) 2019 Google Inc.
>   */
>
> -#include <linux/slab.h>
> +#include <linux/idr.h>
>  #include <linux/kdev_t.h>
> +#include <linux/slab.h>
>
>  #include "power.h"
>
> @@ -80,6 +81,15 @@ static ssize_t last_change_ms_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(last_change_ms);
>
> +static ssize_t name_show(struct device *dev,
> +                                  struct device_attribute *attr, char *buf)
> +{
> +       struct wakeup_source *ws = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%s\n", ws->name);
> +}
> +static DEVICE_ATTR_RO(name);
> +
>  static ssize_t prevent_suspend_time_ms_show(struct device *dev,
>                                             struct device_attribute *attr,
>                                             char *buf)
> @@ -96,6 +106,7 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev,
>  static DEVICE_ATTR_RO(prevent_suspend_time_ms);
>
>  static struct attribute *wakeup_source_attrs[] = {
> +       &dev_attr_name.attr,
>         &dev_attr_active_count.attr,
>         &dev_attr_event_count.attr,
>         &dev_attr_wakeup_count.attr,
> @@ -109,18 +120,27 @@ static struct attribute *wakeup_source_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(wakeup_source);
>
> +static DEFINE_IDA(wakeup_ida);
> +
>  /**
>   * wakeup_source_sysfs_add - Add wakeup_source attributes to sysfs.
>   * @ws: Wakeup source to be added in sysfs.
> + * @parent: Device wakeup source is associated with (or NULL if virtual)
>   */
> -int wakeup_source_sysfs_add(struct wakeup_source *ws)
> +int wakeup_source_sysfs_add(struct device *parent, struct wakeup_source *ws)
>  {
>         struct device *dev;
>
> -       dev = device_create_with_groups(wakeup_class, NULL, MKDEV(0, 0), ws,
> -                                       wakeup_source_groups, "%s", ws->name);
> -       if (IS_ERR(dev))
> +       ws->id = ida_simple_get(&wakeup_ida, 0, 0, GFP_KERNEL);
> +       if (ws->id < 0)
> +               return ws->id;
> +
> +       dev = device_create_with_groups(wakeup_class, parent, MKDEV(0, 0), ws,
> +                                       wakeup_source_groups, "wakeup%d", ws->id);
> +       if (IS_ERR(dev)) {
> +               ida_simple_remove(&wakeup_ida, ws->id);
>                 return PTR_ERR(dev);
> +       }
>
>         ws->dev = dev;
>         return 0;
> @@ -134,6 +154,7 @@ EXPORT_SYMBOL_GPL(wakeup_source_sysfs_add);
>  void wakeup_source_sysfs_remove(struct wakeup_source *ws)
>  {
>         device_unregister(ws->dev);
> +       ida_simple_remove(&wakeup_ida, ws->id);
>  }
>  EXPORT_SYMBOL_GPL(wakeup_source_sysfs_remove);
>
> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index d7f1f5011fac..c4159bcc05d9 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -1459,13 +1459,13 @@ static int ep_create_wakeup_source(struct epitem *epi)
>         struct wakeup_source *ws;
>
>         if (!epi->ep->ws) {
> -               epi->ep->ws = wakeup_source_register("eventpoll");
> +               epi->ep->ws = wakeup_source_register(NULL, "eventpoll");
>                 if (!epi->ep->ws)
>                         return -ENOMEM;
>         }
>
>         name = epi->ffd.file->f_path.dentry->d_name.name;
> -       ws = wakeup_source_register(name);
> +       ws = wakeup_source_register(NULL, name);
>
>         if (!ws)
>                 return -ENOMEM;
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index 500f9cfe2db8..822e74f45384 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -41,6 +41,7 @@ struct wake_irq;
>   */
>  struct wakeup_source {
>         const char              *name;
> +       int                     id;
>         struct list_head        entry;
>         spinlock_t              lock;
>         struct wake_irq         *wakeirq;
> @@ -88,7 +89,8 @@ extern struct wakeup_source *wakeup_source_create(const char *name);
>  extern void wakeup_source_destroy(struct wakeup_source *ws);
>  extern void wakeup_source_add(struct wakeup_source *ws);
>  extern void wakeup_source_remove(struct wakeup_source *ws);
> -extern struct wakeup_source *wakeup_source_register(const char *name);
> +extern struct wakeup_source *wakeup_source_register(struct device *dev,
> +                                                   const char *name);
>  extern void wakeup_source_unregister(struct wakeup_source *ws);
>  extern int device_wakeup_enable(struct device *dev);
>  extern int device_wakeup_disable(struct device *dev);
> @@ -128,7 +130,8 @@ static inline void wakeup_source_add(struct wakeup_source *ws) {}
>
>  static inline void wakeup_source_remove(struct wakeup_source *ws) {}
>
> -static inline struct wakeup_source *wakeup_source_register(const char *name)
> +static inline struct wakeup_source *wakeup_source_register(struct device *dev,
> +                                                          const char *name)
>  {
>         return NULL;
>  }
> @@ -186,12 +189,13 @@ static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
>  #ifdef CONFIG_PM_SLEEP_STATS
>
>  /* drivers/base/power/wakeup_stats.c */
> -int wakeup_source_sysfs_add(struct wakeup_source *ws);
> +int wakeup_source_sysfs_add(struct device *parent, struct wakeup_source *ws);
>  void wakeup_source_sysfs_remove(struct wakeup_source *ws);
>
>  #else /* !CONFIG_PM_SLEEP_STATS */
>
> -static inline int wakeup_source_sysfs_add(struct wakeup_source *ws)
> +static inline int wakeup_source_sysfs_add(struct device *parent,
> +                                         struct wakeup_source *ws)
>  {
>         return 0;
>  }
> diff --git a/kernel/power/autosleep.c b/kernel/power/autosleep.c
> index 41e83a779e19..9af5a50d3489 100644
> --- a/kernel/power/autosleep.c
> +++ b/kernel/power/autosleep.c
> @@ -116,7 +116,7 @@ int pm_autosleep_set_state(suspend_state_t state)
>
>  int __init pm_autosleep_init(void)
>  {
> -       autosleep_ws = wakeup_source_register("autosleep");
> +       autosleep_ws = wakeup_source_register(NULL, "autosleep");
>         if (!autosleep_ws)
>                 return -ENOMEM;
>
> diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
> index 32726da3d6e6..826fcd97647a 100644
> --- a/kernel/power/wakelock.c
> +++ b/kernel/power/wakelock.c
> @@ -192,7 +192,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
>         wl->ws.name = wl->name;
>         wl->ws.last_time = ktime_get();
>
> -       ret = wakeup_source_sysfs_add(&wl->ws);
> +       ret = wakeup_source_sysfs_add(NULL, &wl->ws);
>         if (ret) {
>                 kfree(wl->name);
>                 kfree(wl);
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index 57518efc3810..93b382d9701c 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -97,7 +97,7 @@ static int alarmtimer_rtc_add_device(struct device *dev,
>         if (!device_may_wakeup(rtc->dev.parent))
>                 return -1;
>
> -       __ws = wakeup_source_register("alarmtimer");
> +       __ws = wakeup_source_register(dev, "alarmtimer");
>
>         spin_lock_irqsave(&rtcdev_lock, flags);
>         if (!rtcdev) {
>
> base-commit: 0c826a07dd696d99784c68ec1e8def4399cc4a0b
> --
> Sent by a computer through tubes
>

Re: [PATCH] PM / wakeup: Avoid dev_name collisions in wakeup class

[Tri Vo]

On Sat, Jul 27, 2019 at 6:10 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, Jul 27, 2019 at 3:11 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > If a device is wakeup capable and the driver calls device_wakeup_init()
> > on it during probe and then userspace writes 'enabled' to that device's
> > power/wakeup file in sysfs we'll try to create the same named wakeup
> > device in sysfs. The kernel will complain about duplicate file names.

Thanks for reporting the issue, Stephen!
> >
> > sysfs: cannot create duplicate filename '/devices/virtual/wakeup/1-1.1'
> > kobject_add_internal failed for 1-1.1 with -EEXIST, don't try to register things with the same name in the same directory.
> >
> > It may be advantageous to not write 'enabled' to the wakeup file (see
> > wakeup_store()) from userspace for these devices because we allocate
> > devices and register them and then throw them all away later on if the
> > device driver has already initialized the wakeup attribute. The
> > implementation currently tries to avoid taking locks here so it seems
> > best to optimize that path in a separate patch.
> >
> > Let's rename the wakeup class devices as 'wakeupN' with an IDA that's
> > simple enough to just return some sort of number. In addition, let's
> > make the device registering the wakeup the parent and include a 'name'
> > attribute in case userspace wants to figure out the type of wakeup it is
> > (in the case of virtual wakeups) or the device associated with the
> > wakeup. This makes it easier for userspace to go from /sys/class/wakeup
> > to a place in the device hierarchy where the wakeup is generated from
> > like an input device.
> >
> > Cc: Tri Vo <trong@android.com>
> > Cc: Kalesh Singh <kaleshsingh@google.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>
> I'd rather change the commit that introduced this issue which is only
> in linux-next for now.

Raphael, could you roll back my patch? I'll work with Stephen to fix it.

Re: [PATCH] PM / wakeup: Avoid dev_name collisions in wakeup class

[Rafael J. Wysocki]

On Saturday, July 27, 2019 7:37:07 PM CEST Tri Vo wrote:
> On Sat, Jul 27, 2019 at 6:10 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Sat, Jul 27, 2019 at 3:11 AM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > If a device is wakeup capable and the driver calls device_wakeup_init()
> > > on it during probe and then userspace writes 'enabled' to that device's
> > > power/wakeup file in sysfs we'll try to create the same named wakeup
> > > device in sysfs. The kernel will complain about duplicate file names.
> 
> Thanks for reporting the issue, Stephen!
> > >
> > > sysfs: cannot create duplicate filename '/devices/virtual/wakeup/1-1.1'
> > > kobject_add_internal failed for 1-1.1 with -EEXIST, don't try to register things with the same name in the same directory.
> > >
> > > It may be advantageous to not write 'enabled' to the wakeup file (see
> > > wakeup_store()) from userspace for these devices because we allocate
> > > devices and register them and then throw them all away later on if the
> > > device driver has already initialized the wakeup attribute. The
> > > implementation currently tries to avoid taking locks here so it seems
> > > best to optimize that path in a separate patch.
> > >
> > > Let's rename the wakeup class devices as 'wakeupN' with an IDA that's
> > > simple enough to just return some sort of number. In addition, let's
> > > make the device registering the wakeup the parent and include a 'name'
> > > attribute in case userspace wants to figure out the type of wakeup it is
> > > (in the case of virtual wakeups) or the device associated with the
> > > wakeup. This makes it easier for userspace to go from /sys/class/wakeup
> > > to a place in the device hierarchy where the wakeup is generated from
> > > like an input device.
> > >
> > > Cc: Tri Vo <trong@android.com>
> > > Cc: Kalesh Singh <kaleshsingh@google.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >
> > I'd rather change the commit that introduced this issue which is only
> > in linux-next for now.
> 
> Raphael, could you roll back my patch? I'll work with Stephen to fix it.

I'll drop it, thanks!

Re: [PATCH] PM / wakeup: Avoid dev_name collisions in wakeup class

[Stephen Boyd]

Quoting Rafael J. Wysocki (2019-07-27 06:10:00)
> On Sat, Jul 27, 2019 at 3:11 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > If a device is wakeup capable and the driver calls device_wakeup_init()
> > on it during probe and then userspace writes 'enabled' to that device's
> > power/wakeup file in sysfs we'll try to create the same named wakeup
> > device in sysfs. The kernel will complain about duplicate file names.
> >
> > sysfs: cannot create duplicate filename '/devices/virtual/wakeup/1-1.1'
> > kobject_add_internal failed for 1-1.1 with -EEXIST, don't try to register things with the same name in the same directory.
> >
> > It may be advantageous to not write 'enabled' to the wakeup file (see
> > wakeup_store()) from userspace for these devices because we allocate
> > devices and register them and then throw them all away later on if the
> > device driver has already initialized the wakeup attribute. The
> > implementation currently tries to avoid taking locks here so it seems
> > best to optimize that path in a separate patch.
> >
> > Let's rename the wakeup class devices as 'wakeupN' with an IDA that's
> > simple enough to just return some sort of number. In addition, let's
> > make the device registering the wakeup the parent and include a 'name'
> > attribute in case userspace wants to figure out the type of wakeup it is
> > (in the case of virtual wakeups) or the device associated with the
> > wakeup. This makes it easier for userspace to go from /sys/class/wakeup
> > to a place in the device hierarchy where the wakeup is generated from
> > like an input device.
> >
> > Cc: Tri Vo <trong@android.com>
> > Cc: Kalesh Singh <kaleshsingh@google.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> 
> I'd rather change the commit that introduced this issue which is only
> in linux-next for now.

Feel free to squash the two patches together and throw my signed-off-by
on it. I forgot to add 'name' to the Documentation directory. Here's
something to that effect.

-----8<-----
diff --git a/Documentation/ABI/testing/sysfs-class-wakeup b/Documentation/ABI/testing/sysfs-class-wakeup
index 30fb23eb9112..b83a87380d2c 100644
--- a/Documentation/ABI/testing/sysfs-class-wakeup
+++ b/Documentation/ABI/testing/sysfs-class-wakeup
@@ -5,6 +5,13 @@ Description:
 		The /sys/class/wakeup/ directory contains pointers to all
 		wakeup sources in the kernel at that moment in time.
 
+What:		/sys/class/wakeup/.../name
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the name of the wakeup source when
+		it was created.
+
 What:		/sys/class/wakeup/.../active_count
 Date:		June 2019
 Contact:	Tri Vo <trong@android.com>