Re: [PATCH 2/3] regulator: userspace-consumer: Add regulator event support

[Zev Weiss <zev@bewilderbeest.net>]

On Thu, Aug 31, 2023 at 05:14:09AM PDT, Naresh Solanki wrote:
>Add sysfs attribute to track regulator events received from regulator
>notifier block handler.
>
>Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>---
> drivers/regulator/userspace-consumer.c | 54 +++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/regulator/userspace-consumer.c b/drivers/regulator/userspace-consumer.c
>index 97f075ed68c9..a936661d99cd 100644
>--- a/drivers/regulator/userspace-consumer.c
>+++ b/drivers/regulator/userspace-consumer.c
>@@ -29,6 +29,10 @@ struct userspace_consumer_data {
>
> 	int num_supplies;
> 	struct regulator_bulk_data *supplies;
>+
>+	struct kobject *kobj;
>+	struct notifier_block nb;
>+	unsigned long events;
> };
>
> static ssize_t name_show(struct device *dev,
>@@ -89,12 +93,30 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
> 	return count;
> }
>
>+static DEFINE_SPINLOCK(events_lock);
>+
>+static ssize_t events_show(struct device *dev,
>+			   struct device_attribute *attr, char *buf)
>+{
>+	struct userspace_consumer_data *data = dev_get_drvdata(dev);
>+	unsigned long e;
>+
>+	spin_lock(&events_lock);
>+	e = data->events;
>+	data->events = 0;

I still don't think this is a good solution for the problem.

I for one frequently examine things in sysfs using shell commands like 
'cat' and 'grep' and such, and I suspect I'm (very, very) far from alone 
in that.  With this design a user doing that could cause a monitoring 
daemon to miss events that it was expecting to receive via this file.  

I don't think we should be creating sysfs files that are secretly land 
mines that allow a curious user innocently peeking around in sysfs doing 
(they think) read-only operations to break things for other programs 
using those files.


Zev