Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 1 Sep 2023 02:27:28 -0700
From:   Zev Weiss <zev@bewilderbeest.net>
To:     Naresh Solanki <naresh.solanki@9elements.com>
Cc:     broonie@kernel.org, Liam Girdwood <lgirdwood@gmail.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] regulator: userspace-consumer: Add regulator event
 support
Message-ID: <5000a233-e992-455a-bca3-79aaa8713ff4@hatter.bewilderbeest.net>
References: <20230831121412.2359239-1-Naresh.Solanki@9elements.com>
 <20230831121412.2359239-2-Naresh.Solanki@9elements.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20230831121412.2359239-2-Naresh.Solanki@9elements.com>
Precedence: bulk

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