On 17/10/2021 15:50:11+0200, Greg KH wrote:
> Note, review of this now that it has been submitted in a pull request to
> me, sorry I missed this previously...
>
> On Wed, Sep 29, 2021 at 12:15:59PM +0900, William Breathitt Gray wrote:
> > +static int counter_chrdev_open(struct inode *inode, struct file *filp)
> > +{
> > + struct counter_device *const counter = container_of(inode->i_cdev,
> > + typeof(*counter),
> > + chrdev);
> > +
> > + /* Ensure chrdev is not opened more than 1 at a time */
> > + if (!atomic_add_unless(&counter->chrdev_lock, 1, 1))
> > + return -EBUSY;
>
> I understand the feeling that you wish to stop userspace from doing
> this, but really, it does not work. Eventhough you are doing this
> correctly (you should see all the other attempts at doing this), you are
> not preventing userspace from having multiple processes access this
> device node at the same time, so please, don't even attempt to stop this
> from happening.
>
> So you can drop the atomic "lock" you have here, it's not needed at all.
>
Could you elaborate a bit here because we've had a similar thing in the
RTC subsystem:
https://elixir.bootlin.com/linux/latest/source/drivers/rtc/dev.c#L28
And it would mean I can remove rtc->flags completely.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
