[PATCH v3] drivers/base: add sysfs entries for suppliers and consumers

[PATCH v3] drivers/base: add sysfs entries for suppliers and consumers

[Ioana Ciornei]

Instead of scraping dmesg for messages such as 'Linked as a consumer to'
or 'Dropping the link to' export two new sysfs entries in the device
folder that contain a list of the consumer and supplier devices.

Also, lower the log level of the prints since the same information
is available in the sysfs entries.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
  - lower the log level of prints
Changes in v2:
  - add documentation entries for the new sysfs files

 Documentation/ABI/testing/sysfs-devices-links | 13 +++++++
 drivers/base/core.c                           | 52 ++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-links

diff --git a/Documentation/ABI/testing/sysfs-devices-links b/Documentation/ABI/testing/sysfs-devices-links
new file mode 100644
index 0000000..d3951c5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-links
@@ -0,0 +1,13 @@
+What:		/sys/devices/.../consumers
+Date:		December 2018
+Contact:	Ioana Ciornei <ioana.ciornei@nxp.com>
+Description:
+		Read-only attribute that lists the current "consumers" of
+		a specific device.
+
+What:		/sys/devices/.../suppliers
+Date:		December 2018
+Contact:	Ioana Ciornei <ioana.ciornei@nxp.com>
+Description:
+		Read-only attribute that lists the current "suppliers" of
+		a specific device.
diff --git a/drivers/base/core.c b/drivers/base/core.c
index a4ced33..5339bcf 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -301,7 +301,7 @@ struct device_link *device_link_add(struct device *consumer,
 	list_add_tail_rcu(&link->s_node, &supplier->links.consumers);
 	list_add_tail_rcu(&link->c_node, &consumer->links.suppliers);
 
-	dev_info(consumer, "Linked as a consumer to %s\n", dev_name(supplier));
+	dev_dbg(consumer, "Linked as a consumer to %s\n", dev_name(supplier));
 
  out:
 	device_pm_unlock();
@@ -327,8 +327,8 @@ static void __device_link_del(struct kref *kref)
 {
 	struct device_link *link = container_of(kref, struct device_link, kref);
 
-	dev_info(link->consumer, "Dropping the link to %s\n",
-		 dev_name(link->supplier));
+	dev_dbg(link->consumer, "Dropping the link to %s\n",
+		dev_name(link->supplier));
 
 	if (link->flags & DL_FLAG_PM_RUNTIME)
 		pm_runtime_drop_link(link->consumer);
@@ -342,8 +342,8 @@ static void __device_link_del(struct kref *kref)
 {
 	struct device_link *link = container_of(kref, struct device_link, kref);
 
-	dev_info(link->consumer, "Dropping the link to %s\n",
-		 dev_name(link->supplier));
+	dev_dbg(link->consumer, "Dropping the link to %s\n",
+		dev_name(link->supplier));
 
 	if (link->flags & DL_FLAG_PM_RUNTIME)
 		pm_runtime_drop_link(link->consumer);
@@ -1117,6 +1117,34 @@ static ssize_t online_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(online);
 
+static ssize_t suppliers_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct device_link *link;
+	size_t count = 0;
+
+	list_for_each_entry(link, &dev->links.suppliers, c_node)
+		count += scnprintf(buf + count, PAGE_SIZE - count, "%s\n",
+				   dev_name(link->supplier));
+
+	return count;
+}
+static DEVICE_ATTR_RO(suppliers);
+
+static ssize_t consumers_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct device_link *link;
+	size_t count = 0;
+
+	list_for_each_entry(link, &dev->links.consumers, s_node)
+		count += scnprintf(buf + count, PAGE_SIZE - count, "%s\n",
+				   dev_name(link->consumer));
+
+	return count;
+}
+static DEVICE_ATTR_RO(consumers);
+
 int device_add_groups(struct device *dev, const struct attribute_group **groups)
 {
 	return sysfs_create_groups(&dev->kobj, groups);
@@ -1288,8 +1316,20 @@ static int device_add_attrs(struct device *dev)
 			goto err_remove_dev_groups;
 	}
 
+	error = device_create_file(dev, &dev_attr_suppliers);
+	if (error)
+		goto err_remove_online;
+
+	error = device_create_file(dev, &dev_attr_consumers);
+	if (error)
+		goto err_remove_suppliers;
+
 	return 0;
 
+ err_remove_suppliers:
+	device_remove_file(dev, &dev_attr_suppliers);
+ err_remove_online:
+	device_remove_file(dev, &dev_attr_online);
  err_remove_dev_groups:
 	device_remove_groups(dev, dev->groups);
  err_remove_type_groups:
@@ -1307,6 +1347,8 @@ static void device_remove_attrs(struct device *dev)
 	struct class *class = dev->class;
 	const struct device_type *type = dev->type;
 
+	device_remove_file(dev, &dev_attr_consumers);
+	device_remove_file(dev, &dev_attr_suppliers);
 	device_remove_file(dev, &dev_attr_online);
 	device_remove_groups(dev, dev->groups);
 
-- 
1.9.1

Re: [PATCH v3] drivers/base: add sysfs entries for suppliers and consumers

[gregkh]

On Thu, Dec 20, 2018 at 01:05:11PM +0000, Ioana Ciornei wrote:
> Instead of scraping dmesg for messages such as 'Linked as a consumer to'
> or 'Dropping the link to' export two new sysfs entries in the device
> folder that contain a list of the consumer and supplier devices.

What is userspace going to do with that information?  Why does it care?

> 
> Also, lower the log level of the prints since the same information
> is available in the sysfs entries.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v3:
>   - lower the log level of prints
> Changes in v2:
>   - add documentation entries for the new sysfs files
> 
>  Documentation/ABI/testing/sysfs-devices-links | 13 +++++++
>  drivers/base/core.c                           | 52 ++++++++++++++++++++++++---
>  2 files changed, 60 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-devices-links
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-links b/Documentation/ABI/testing/sysfs-devices-links
> new file mode 100644
> index 0000000..d3951c5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-links
> @@ -0,0 +1,13 @@
> +What:		/sys/devices/.../consumers
> +Date:		December 2018
> +Contact:	Ioana Ciornei <ioana.ciornei@nxp.com>
> +Description:
> +		Read-only attribute that lists the current "consumers" of
> +		a specific device.
> +
> +What:		/sys/devices/.../suppliers
> +Date:		December 2018
> +Contact:	Ioana Ciornei <ioana.ciornei@nxp.com>
> +Description:
> +		Read-only attribute that lists the current "suppliers" of
> +		a specific device.
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index a4ced33..5339bcf 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -301,7 +301,7 @@ struct device_link *device_link_add(struct device *consumer,
>  	list_add_tail_rcu(&link->s_node, &supplier->links.consumers);
>  	list_add_tail_rcu(&link->c_node, &consumer->links.suppliers);
>  
> -	dev_info(consumer, "Linked as a consumer to %s\n", dev_name(supplier));
> +	dev_dbg(consumer, "Linked as a consumer to %s\n", dev_name(supplier));
>  
>   out:
>  	device_pm_unlock();
> @@ -327,8 +327,8 @@ static void __device_link_del(struct kref *kref)
>  {
>  	struct device_link *link = container_of(kref, struct device_link, kref);
>  
> -	dev_info(link->consumer, "Dropping the link to %s\n",
> -		 dev_name(link->supplier));
> +	dev_dbg(link->consumer, "Dropping the link to %s\n",
> +		dev_name(link->supplier));
>  
>  	if (link->flags & DL_FLAG_PM_RUNTIME)
>  		pm_runtime_drop_link(link->consumer);
> @@ -342,8 +342,8 @@ static void __device_link_del(struct kref *kref)
>  {
>  	struct device_link *link = container_of(kref, struct device_link, kref);
>  
> -	dev_info(link->consumer, "Dropping the link to %s\n",
> -		 dev_name(link->supplier));
> +	dev_dbg(link->consumer, "Dropping the link to %s\n",
> +		dev_name(link->supplier));
>  
>  	if (link->flags & DL_FLAG_PM_RUNTIME)
>  		pm_runtime_drop_link(link->consumer);
> @@ -1117,6 +1117,34 @@ static ssize_t online_store(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RW(online);
>  
> +static ssize_t suppliers_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct device_link *link;
> +	size_t count = 0;
> +
> +	list_for_each_entry(link, &dev->links.suppliers, c_node)
> +		count += scnprintf(buf + count, PAGE_SIZE - count, "%s\n",
> +				   dev_name(link->supplier));
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RO(suppliers);
> +
> +static ssize_t consumers_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct device_link *link;
> +	size_t count = 0;
> +
> +	list_for_each_entry(link, &dev->links.consumers, s_node)
> +		count += scnprintf(buf + count, PAGE_SIZE - count, "%s\n",
> +				   dev_name(link->consumer));

sysfs is supposed to be one value per file, this violates that.  I don't
like that at all, now this file has to be parsed by userspace.

> +
> +	return count;
> +}
> +static DEVICE_ATTR_RO(consumers);

Turn these into an attribute group, so you can add them all at once?

And do you really need these for _EVERY_ device in the system?  That
feels like a lot of extra files for almost no users.


> +
>  int device_add_groups(struct device *dev, const struct attribute_group **groups)
>  {
>  	return sysfs_create_groups(&dev->kobj, groups);
> @@ -1288,8 +1316,20 @@ static int device_add_attrs(struct device *dev)
>  			goto err_remove_dev_groups;
>  	}
>  
> +	error = device_create_file(dev, &dev_attr_suppliers);
> +	if (error)
> +		goto err_remove_online;
> +
> +	error = device_create_file(dev, &dev_attr_consumers);
> +	if (error)
> +		goto err_remove_suppliers;

These should be added in device_add(), not burried down in
device_add_attrs, which is there to add the attributes that the
bus/class wants to have added to the device.

But I still don't know why this is needed, or who is going to use it, or
what they are going to be used for...

thanks,

greg k-h