[4/4] gpiolib: cdev: replace locking wrappers for gpio_device with guards
Commit Message
Replace the wrapping functions that inhibit removal of the gpio_device
with equivalent guard macros.
Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
drivers/gpio/gpiolib-cdev.c | 195 ++++++++++--------------------------
1 file changed, 52 insertions(+), 143 deletions(-)
Comments
(+PeterZ)
On Wed, Dec 20, 2023 at 2:52 AM Kent Gibson <warthog618@gmail.com> wrote:
> Replace the wrapping functions that inhibit removal of the gpio_device
> with equivalent guard macros.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>
(...)
> +DEFINE_CLASS(_read_sem_guard,
> + struct rw_semaphore *,
> + up_read(_T),
> + ({
> + down_read(sem);
> + sem;
> + }),
> + struct rw_semaphore *sem);
Isn't this so generic that it should be in <linux/cleanup.h>?
Otherwise all the patches look good to me.
Yours,
Linus Walleij
On Wed, Dec 20, 2023 at 12:56 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> (+PeterZ)
>
> On Wed, Dec 20, 2023 at 2:52 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> > Replace the wrapping functions that inhibit removal of the gpio_device
> > with equivalent guard macros.
> >
> > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> (...)
> > +DEFINE_CLASS(_read_sem_guard,
> > + struct rw_semaphore *,
> > + up_read(_T),
> > + ({
> > + down_read(sem);
> > + sem;
> > + }),
> > + struct rw_semaphore *sem);
>
> Isn't this so generic that it should be in <linux/cleanup.h>?
>
> Otherwise all the patches look good to me.
>
We already have this:
DEFINE_GUARD(rwsem_read, struct rw_semaphore *, down_read(_T), up_read(_T))
DEFINE_GUARD(rwsem_write, struct rw_semaphore *, down_write(_T), up_write(_T))
DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
This can surely be used here, right?
Bart
> Yours,
> Linus Walleij
On Wed, Dec 20, 2023 at 01:05:35PM +0100, Bartosz Golaszewski wrote:
> On Wed, Dec 20, 2023 at 12:56 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > (+PeterZ)
> >
> > On Wed, Dec 20, 2023 at 2:52 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > > Replace the wrapping functions that inhibit removal of the gpio_device
> > > with equivalent guard macros.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > (...)
> > > +DEFINE_CLASS(_read_sem_guard,
> > > + struct rw_semaphore *,
> > > + up_read(_T),
> > > + ({
> > > + down_read(sem);
> > > + sem;
> > > + }),
> > > + struct rw_semaphore *sem);
> >
> > Isn't this so generic that it should be in <linux/cleanup.h>?
> >
> > Otherwise all the patches look good to me.
> >
>
> We already have this:
>
> DEFINE_GUARD(rwsem_read, struct rw_semaphore *, down_read(_T), up_read(_T))
> DEFINE_GUARD(rwsem_write, struct rw_semaphore *, down_write(_T), up_write(_T))
>
> DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
> DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
>
Ah - in rwsem.h - I missed that.
> This can surely be used here, right?
>
Don't see why not.
I would still like to move the gpio_device specific macros to gpiolib.h,
as they apply to the struct gpio_device defined there.
The naming probably needs some reworking, so open to suggestions on
that.
Cheers,
Kent.
On Wed, Dec 20, 2023 at 1:13 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Wed, Dec 20, 2023 at 01:05:35PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Dec 20, 2023 at 12:56 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > > (+PeterZ)
> > >
> > > On Wed, Dec 20, 2023 at 2:52 AM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > > Replace the wrapping functions that inhibit removal of the gpio_device
> > > > with equivalent guard macros.
> > > >
> > > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > (...)
> > > > +DEFINE_CLASS(_read_sem_guard,
> > > > + struct rw_semaphore *,
> > > > + up_read(_T),
> > > > + ({
> > > > + down_read(sem);
> > > > + sem;
> > > > + }),
> > > > + struct rw_semaphore *sem);
> > >
> > > Isn't this so generic that it should be in <linux/cleanup.h>?
> > >
> > > Otherwise all the patches look good to me.
> > >
> >
> > We already have this:
> >
> > DEFINE_GUARD(rwsem_read, struct rw_semaphore *, down_read(_T), up_read(_T))
> > DEFINE_GUARD(rwsem_write, struct rw_semaphore *, down_write(_T), up_write(_T))
> >
> > DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
> > DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
> >
>
> Ah - in rwsem.h - I missed that.
>
> > This can surely be used here, right?
> >
>
> Don't see why not.
>
> I would still like to move the gpio_device specific macros to gpiolib.h,
> as they apply to the struct gpio_device defined there.
Which ones? Because I'd rather use guard(rwsem_read)(&gdev->sem); than
some custom wrapper as this one's purpose is clearer.
Bart
> The naming probably needs some reworking, so open to suggestions on
> that.
>
> Cheers,
> Kent.
On Wed, Dec 20, 2023 at 01:16:00PM +0100, Bartosz Golaszewski wrote:
> On Wed, Dec 20, 2023 at 1:13 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Wed, Dec 20, 2023 at 01:05:35PM +0100, Bartosz Golaszewski wrote:
> > > On Wed, Dec 20, 2023 at 12:56 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > >
> > > > (+PeterZ)
> > > >
> > > > On Wed, Dec 20, 2023 at 2:52 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > > Replace the wrapping functions that inhibit removal of the gpio_device
> > > > > with equivalent guard macros.
> > > > >
> > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > > (...)
> > > > > +DEFINE_CLASS(_read_sem_guard,
> > > > > + struct rw_semaphore *,
> > > > > + up_read(_T),
> > > > > + ({
> > > > > + down_read(sem);
> > > > > + sem;
> > > > > + }),
> > > > > + struct rw_semaphore *sem);
> > > >
> > > > Isn't this so generic that it should be in <linux/cleanup.h>?
> > > >
> > > > Otherwise all the patches look good to me.
> > > >
> > >
> > > We already have this:
> > >
> > > DEFINE_GUARD(rwsem_read, struct rw_semaphore *, down_read(_T), up_read(_T))
> > > DEFINE_GUARD(rwsem_write, struct rw_semaphore *, down_write(_T), up_write(_T))
> > >
> > > DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
> > > DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
> > >
> >
> > Ah - in rwsem.h - I missed that.
> >
> > > This can surely be used here, right?
> > >
> >
> > Don't see why not.
> >
> > I would still like to move the gpio_device specific macros to gpiolib.h,
> > as they apply to the struct gpio_device defined there.
>
> Which ones? Because I'd rather use guard(rwsem_read)(&gdev->sem); than
> some custom wrapper as this one's purpose is clearer.
>
It would be read and write guards for the gpio_device.
cdev would only be using the read flavour.
And possibly named something other than read/write as the purpose is to
prevent (read) or allow (write) object removal.
I though that would be clearer than having to reference gpiolib.h to see
what gdev->sem covers, and allow you to change the locking
mechanism later and not have to update cdev.
Cheers,
Kent.
On Wed, Dec 20, 2023 at 1:23 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Wed, Dec 20, 2023 at 01:16:00PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Dec 20, 2023 at 1:13 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > On Wed, Dec 20, 2023 at 01:05:35PM +0100, Bartosz Golaszewski wrote:
> > > > On Wed, Dec 20, 2023 at 12:56 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > > >
> > > > > (+PeterZ)
> > > > >
> > > > > On Wed, Dec 20, 2023 at 2:52 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > > >
> > > > > > Replace the wrapping functions that inhibit removal of the gpio_device
> > > > > > with equivalent guard macros.
> > > > > >
> > > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > > > (...)
> > > > > > +DEFINE_CLASS(_read_sem_guard,
> > > > > > + struct rw_semaphore *,
> > > > > > + up_read(_T),
> > > > > > + ({
> > > > > > + down_read(sem);
> > > > > > + sem;
> > > > > > + }),
> > > > > > + struct rw_semaphore *sem);
> > > > >
> > > > > Isn't this so generic that it should be in <linux/cleanup.h>?
> > > > >
> > > > > Otherwise all the patches look good to me.
> > > > >
> > > >
> > > > We already have this:
> > > >
> > > > DEFINE_GUARD(rwsem_read, struct rw_semaphore *, down_read(_T), up_read(_T))
> > > > DEFINE_GUARD(rwsem_write, struct rw_semaphore *, down_write(_T), up_write(_T))
> > > >
> > > > DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
> > > > DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
> > > >
> > >
> > > Ah - in rwsem.h - I missed that.
> > >
> > > > This can surely be used here, right?
> > > >
> > >
> > > Don't see why not.
> > >
> > > I would still like to move the gpio_device specific macros to gpiolib.h,
> > > as they apply to the struct gpio_device defined there.
> >
> > Which ones? Because I'd rather use guard(rwsem_read)(&gdev->sem); than
> > some custom wrapper as this one's purpose is clearer.
> >
>
> It would be read and write guards for the gpio_device.
> cdev would only be using the read flavour.
> And possibly named something other than read/write as the purpose is to
> prevent (read) or allow (write) object removal.
>
> I though that would be clearer than having to reference gpiolib.h to see
> what gdev->sem covers, and allow you to change the locking
> mechanism later and not have to update cdev.
>
I still prefer open-coded guards here for clarity. I hope that with
SRCU in gpiolib.c, we'll get rid of locking in cdev entirely anyway.
Bart
> Cheers,
> Kent.
>
On Wed, Dec 20, 2023 at 01:30:57PM +0100, Bartosz Golaszewski wrote:
> On Wed, Dec 20, 2023 at 1:23 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > >
> >
> > It would be read and write guards for the gpio_device.
> > cdev would only be using the read flavour.
> > And possibly named something other than read/write as the purpose is to
> > prevent (read) or allow (write) object removal.
> >
> > I though that would be clearer than having to reference gpiolib.h to see
> > what gdev->sem covers, and allow you to change the locking
> > mechanism later and not have to update cdev.
> >
>
> I still prefer open-coded guards here for clarity. I hope that with
> SRCU in gpiolib.c, we'll get rid of locking in cdev entirely anyway.
>
Ok, it is your object so I should use it the way you want it used.
Btw, before I go pushing out a v2, do you have an answer on whether
gpio_ioctl() requires a guard, as mentioned in the cover letter?
Is the fact there is an active ioctl on the chardev sufficient in
itself to keep the gpio_device alive?
Cheers,
Kent.
On Wed, Dec 20, 2023 at 1:53 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Wed, Dec 20, 2023 at 01:30:57PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Dec 20, 2023 at 1:23 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > >
> > >
> > > It would be read and write guards for the gpio_device.
> > > cdev would only be using the read flavour.
> > > And possibly named something other than read/write as the purpose is to
> > > prevent (read) or allow (write) object removal.
> > >
> > > I though that would be clearer than having to reference gpiolib.h to see
> > > what gdev->sem covers, and allow you to change the locking
> > > mechanism later and not have to update cdev.
> > >
> >
> > I still prefer open-coded guards here for clarity. I hope that with
> > SRCU in gpiolib.c, we'll get rid of locking in cdev entirely anyway.
> >
>
> Ok, it is your object so I should use it the way you want it used.
>
> Btw, before I go pushing out a v2, do you have an answer on whether
> gpio_ioctl() requires a guard, as mentioned in the cover letter?
> Is the fact there is an active ioctl on the chardev sufficient in
> itself to keep the gpio_device alive?
>
AFAICT: no. I think it's a bug (good catch!). Can you extend your
series with a backportable bugfix that would come first?
Bartosz
> Cheers,
> Kent.
On Wed, Dec 20, 2023 at 02:19:37PM +0100, Bartosz Golaszewski wrote:
> On Wed, Dec 20, 2023 at 1:53 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Wed, Dec 20, 2023 at 01:30:57PM +0100, Bartosz Golaszewski wrote:
> > > On Wed, Dec 20, 2023 at 1:23 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > >
> > > >
> > > > It would be read and write guards for the gpio_device.
> > > > cdev would only be using the read flavour.
> > > > And possibly named something other than read/write as the purpose is to
> > > > prevent (read) or allow (write) object removal.
> > > >
> > > > I though that would be clearer than having to reference gpiolib.h to see
> > > > what gdev->sem covers, and allow you to change the locking
> > > > mechanism later and not have to update cdev.
> > > >
> > >
> > > I still prefer open-coded guards here for clarity. I hope that with
> > > SRCU in gpiolib.c, we'll get rid of locking in cdev entirely anyway.
> > >
> >
> > Ok, it is your object so I should use it the way you want it used.
> >
> > Btw, before I go pushing out a v2, do you have an answer on whether
> > gpio_ioctl() requires a guard, as mentioned in the cover letter?
> > Is the fact there is an active ioctl on the chardev sufficient in
> > itself to keep the gpio_device alive?
> >
>
> AFAICT: no. I think it's a bug (good catch!).
The wrappers made that harder to pick up.
It kind of stood out as the exception after changing the other ioctls
over to guards - where was the guard for that one?
> Can you extend your
> series with a backportable bugfix that would come first?
>
Sure. That would still use the guard(rwsem_read)?
I mean you don't to go adding a wrapper for the fix, just to
subsequently remove it, right?
Cheers,
Kent.
On Wed, Dec 20, 2023 at 2:28 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Wed, Dec 20, 2023 at 02:19:37PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Dec 20, 2023 at 1:53 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > On Wed, Dec 20, 2023 at 01:30:57PM +0100, Bartosz Golaszewski wrote:
> > > > On Wed, Dec 20, 2023 at 1:23 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > >
> > > > > >
> > > > >
> > > > > It would be read and write guards for the gpio_device.
> > > > > cdev would only be using the read flavour.
> > > > > And possibly named something other than read/write as the purpose is to
> > > > > prevent (read) or allow (write) object removal.
> > > > >
> > > > > I though that would be clearer than having to reference gpiolib.h to see
> > > > > what gdev->sem covers, and allow you to change the locking
> > > > > mechanism later and not have to update cdev.
> > > > >
> > > >
> > > > I still prefer open-coded guards here for clarity. I hope that with
> > > > SRCU in gpiolib.c, we'll get rid of locking in cdev entirely anyway.
> > > >
> > >
> > > Ok, it is your object so I should use it the way you want it used.
> > >
> > > Btw, before I go pushing out a v2, do you have an answer on whether
> > > gpio_ioctl() requires a guard, as mentioned in the cover letter?
> > > Is the fact there is an active ioctl on the chardev sufficient in
> > > itself to keep the gpio_device alive?
> > >
> >
> > AFAICT: no. I think it's a bug (good catch!).
>
> The wrappers made that harder to pick up.
> It kind of stood out as the exception after changing the other ioctls
> over to guards - where was the guard for that one?
>
Yeah, it makes sense. This is precisely why guards are so much better
than hand-coding locks.
> > Can you extend your
> > series with a backportable bugfix that would come first?
> >
>
> Sure. That would still use the guard(rwsem_read)?
> I mean you don't to go adding a wrapper for the fix, just to
> subsequently remove it, right?
>
In master - sure. But we definitely do want to backport that to stable
branches and for that we need to use the old wrapper.
Bart
> Cheers,
> Kent.
On Wed, Dec 20, 2023 at 02:47:45PM +0100, Bartosz Golaszewski wrote:
> On Wed, Dec 20, 2023 at 2:28 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Wed, Dec 20, 2023 at 02:19:37PM +0100, Bartosz Golaszewski wrote:
> > > On Wed, Dec 20, 2023 at 1:53 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > On Wed, Dec 20, 2023 at 01:30:57PM +0100, Bartosz Golaszewski wrote:
> > > > > On Wed, Dec 20, 2023 at 1:23 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > > >
> > > > > > >
> > > > > >
> > > > > > It would be read and write guards for the gpio_device.
> > > > > > cdev would only be using the read flavour.
> > > > > > And possibly named something other than read/write as the purpose is to
> > > > > > prevent (read) or allow (write) object removal.
> > > > > >
> > > > > > I though that would be clearer than having to reference gpiolib.h to see
> > > > > > what gdev->sem covers, and allow you to change the locking
> > > > > > mechanism later and not have to update cdev.
> > > > > >
> > > > >
> > > > > I still prefer open-coded guards here for clarity. I hope that with
> > > > > SRCU in gpiolib.c, we'll get rid of locking in cdev entirely anyway.
> > > > >
> > > >
> > > > Ok, it is your object so I should use it the way you want it used.
> > > >
> > > > Btw, before I go pushing out a v2, do you have an answer on whether
> > > > gpio_ioctl() requires a guard, as mentioned in the cover letter?
> > > > Is the fact there is an active ioctl on the chardev sufficient in
> > > > itself to keep the gpio_device alive?
> > > >
> > >
> > > AFAICT: no. I think it's a bug (good catch!).
> >
> > The wrappers made that harder to pick up.
> > It kind of stood out as the exception after changing the other ioctls
> > over to guards - where was the guard for that one?
> >
>
> Yeah, it makes sense. This is precisely why guards are so much better
> than hand-coding locks.
>
> > > Can you extend your
> > > series with a backportable bugfix that would come first?
> > >
> >
> > Sure. That would still use the guard(rwsem_read)?
> > I mean you don't to go adding a wrapper for the fix, just to
> > subsequently remove it, right?
> >
>
> In master - sure. But we definitely do want to backport that to stable
> branches and for that we need to use the old wrapper.
>
Ok, so cleanup.h is too recent for backporting.
Adding and then removing a wrapper it is then.
Cheers,
Kent.
@@ -65,44 +65,20 @@ typedef long (*ioctl_fn)(struct file *, unsigned int, unsigned long);
typedef ssize_t (*read_fn)(struct file *, char __user *,
size_t count, loff_t *);
-static __poll_t call_poll_locked(struct file *file,
- struct poll_table_struct *wait,
- struct gpio_device *gdev, poll_fn func)
-{
- __poll_t ret;
-
- down_read(&gdev->sem);
- ret = func(file, wait);
- up_read(&gdev->sem);
-
- return ret;
-}
-
-static long call_ioctl_locked(struct file *file, unsigned int cmd,
- unsigned long arg, struct gpio_device *gdev,
- ioctl_fn func)
-{
- long ret;
+DEFINE_CLASS(_read_sem_guard,
+ struct rw_semaphore *,
+ up_read(_T),
+ ({
+ down_read(sem);
+ sem;
+ }),
+ struct rw_semaphore *sem);
- down_read(&gdev->sem);
- ret = func(file, cmd, arg);
- up_read(&gdev->sem);
+/* guard that downs a rw_semaphore while in scope */
+#define read_sem_guard(sem) CLASS(_read_sem_guard, _sem)(sem)
- return ret;
-}
-
-static ssize_t call_read_locked(struct file *file, char __user *buf,
- size_t count, loff_t *f_ps,
- struct gpio_device *gdev, read_fn func)
-{
- ssize_t ret;
-
- down_read(&gdev->sem);
- ret = func(file, buf, count, f_ps);
- up_read(&gdev->sem);
-
- return ret;
-}
+/* guard on the gpio_device sem to inhibit device removal while in use */
+#define gdev_guard(gdev) read_sem_guard(gdev->sem)
/*
* GPIO line handle management
@@ -238,8 +214,8 @@ static long linehandle_set_config(struct linehandle_state *lh,
return 0;
}
-static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,
- unsigned long arg)
+static long linehandle_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
struct linehandle_state *lh = file->private_data;
void __user *ip = (void __user *)arg;
@@ -248,6 +224,8 @@ static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,
unsigned int i;
int ret;
+ gdev_guard(&lh->gdev);
+
if (!lh->gdev->chip)
return -ENODEV;
@@ -297,15 +275,6 @@ static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,
}
}
-static long linehandle_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- struct linehandle_state *lh = file->private_data;
-
- return call_ioctl_locked(file, cmd, arg, lh->gdev,
- linehandle_ioctl_unlocked);
-}
-
#ifdef CONFIG_COMPAT
static long linehandle_ioctl_compat(struct file *file, unsigned int cmd,
unsigned long arg)
@@ -1564,12 +1533,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
return 0;
}
-static long linereq_ioctl_unlocked(struct file *file, unsigned int cmd,
- unsigned long arg)
+static long linereq_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
struct linereq *lr = file->private_data;
void __user *ip = (void __user *)arg;
+ gdev_guard(&lr->gdev);
+
if (!lr->gdev->chip)
return -ENODEV;
@@ -1585,15 +1556,6 @@ static long linereq_ioctl_unlocked(struct file *file, unsigned int cmd,
}
}
-static long linereq_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- struct linereq *lr = file->private_data;
-
- return call_ioctl_locked(file, cmd, arg, lr->gdev,
- linereq_ioctl_unlocked);
-}
-
#ifdef CONFIG_COMPAT
static long linereq_ioctl_compat(struct file *file, unsigned int cmd,
unsigned long arg)
@@ -1602,12 +1564,14 @@ static long linereq_ioctl_compat(struct file *file, unsigned int cmd,
}
#endif
-static __poll_t linereq_poll_unlocked(struct file *file,
- struct poll_table_struct *wait)
+static __poll_t linereq_poll(struct file *file,
+ struct poll_table_struct *wait)
{
struct linereq *lr = file->private_data;
__poll_t events = 0;
+ gdev_guard(&lr->gdev);
+
if (!lr->gdev->chip)
return EPOLLHUP | EPOLLERR;
@@ -1620,22 +1584,16 @@ static __poll_t linereq_poll_unlocked(struct file *file,
return events;
}
-static __poll_t linereq_poll(struct file *file,
- struct poll_table_struct *wait)
-{
- struct linereq *lr = file->private_data;
-
- return call_poll_locked(file, wait, lr->gdev, linereq_poll_unlocked);
-}
-
-static ssize_t linereq_read_unlocked(struct file *file, char __user *buf,
- size_t count, loff_t *f_ps)
+static ssize_t linereq_read(struct file *file, char __user *buf,
+ size_t count, loff_t *f_ps)
{
struct linereq *lr = file->private_data;
struct gpio_v2_line_event le;
ssize_t bytes_read = 0;
int ret;
+ gdev_guard(&lr->gdev);
+
if (!lr->gdev->chip)
return -ENODEV;
@@ -1677,15 +1635,6 @@ static ssize_t linereq_read_unlocked(struct file *file, char __user *buf,
return bytes_read;
}
-static ssize_t linereq_read(struct file *file, char __user *buf,
- size_t count, loff_t *f_ps)
-{
- struct linereq *lr = file->private_data;
-
- return call_read_locked(file, buf, count, f_ps, lr->gdev,
- linereq_read_unlocked);
-}
-
static void linereq_free(struct linereq *lr)
{
struct line *line;
@@ -1938,12 +1887,14 @@ struct lineevent_state {
(GPIOEVENT_REQUEST_RISING_EDGE | \
GPIOEVENT_REQUEST_FALLING_EDGE)
-static __poll_t lineevent_poll_unlocked(struct file *file,
- struct poll_table_struct *wait)
+static __poll_t lineevent_poll(struct file *file,
+ struct poll_table_struct *wait)
{
struct lineevent_state *le = file->private_data;
__poll_t events = 0;
+ gdev_guard(&le->gdev);
+
if (!le->gdev->chip)
return EPOLLHUP | EPOLLERR;
@@ -1955,14 +1906,6 @@ static __poll_t lineevent_poll_unlocked(struct file *file,
return events;
}
-static __poll_t lineevent_poll(struct file *file,
- struct poll_table_struct *wait)
-{
- struct lineevent_state *le = file->private_data;
-
- return call_poll_locked(file, wait, le->gdev, lineevent_poll_unlocked);
-}
-
static int lineevent_unregistered_notify(struct notifier_block *nb,
unsigned long action, void *data)
{
@@ -1979,8 +1922,8 @@ struct compat_gpioeevent_data {
u32 id;
};
-static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
- size_t count, loff_t *f_ps)
+static ssize_t lineevent_read(struct file *file, char __user *buf,
+ size_t count, loff_t *f_ps)
{
struct lineevent_state *le = file->private_data;
struct gpioevent_data ge;
@@ -1988,6 +1931,8 @@ static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
ssize_t ge_size;
int ret;
+ gdev_guard(&le->gdev);
+
if (!le->gdev->chip)
return -ENODEV;
@@ -2042,15 +1987,6 @@ static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
return bytes_read;
}
-static ssize_t lineevent_read(struct file *file, char __user *buf,
- size_t count, loff_t *f_ps)
-{
- struct lineevent_state *le = file->private_data;
-
- return call_read_locked(file, buf, count, f_ps, le->gdev,
- lineevent_read_unlocked);
-}
-
static void lineevent_free(struct lineevent_state *le)
{
if (le->device_unregistered_nb.notifier_call)
@@ -2071,13 +2007,15 @@ static int lineevent_release(struct inode *inode, struct file *file)
return 0;
}
-static long lineevent_ioctl_unlocked(struct file *file, unsigned int cmd,
- unsigned long arg)
+static long lineevent_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
struct lineevent_state *le = file->private_data;
void __user *ip = (void __user *)arg;
struct gpiohandle_data ghd;
+ gdev_guard(&le->gdev);
+
if (!le->gdev->chip)
return -ENODEV;
@@ -2103,15 +2041,6 @@ static long lineevent_ioctl_unlocked(struct file *file, unsigned int cmd,
return -EINVAL;
}
-static long lineevent_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- struct lineevent_state *le = file->private_data;
-
- return call_ioctl_locked(file, cmd, arg, le->gdev,
- lineevent_ioctl_unlocked);
-}
-
#ifdef CONFIG_COMPAT
static long lineevent_ioctl_compat(struct file *file, unsigned int cmd,
unsigned long arg)
@@ -2671,12 +2600,14 @@ static int gpio_device_unregistered_notify(struct notifier_block *nb,
return NOTIFY_OK;
}
-static __poll_t lineinfo_watch_poll_unlocked(struct file *file,
- struct poll_table_struct *pollt)
+static __poll_t lineinfo_watch_poll(struct file *file,
+ struct poll_table_struct *pollt)
{
struct gpio_chardev_data *cdev = file->private_data;
__poll_t events = 0;
+ gdev_guard(&cdev->gdev);
+
if (!cdev->gdev->chip)
return EPOLLHUP | EPOLLERR;
@@ -2689,17 +2620,8 @@ static __poll_t lineinfo_watch_poll_unlocked(struct file *file,
return events;
}
-static __poll_t lineinfo_watch_poll(struct file *file,
- struct poll_table_struct *pollt)
-{
- struct gpio_chardev_data *cdev = file->private_data;
-
- return call_poll_locked(file, pollt, cdev->gdev,
- lineinfo_watch_poll_unlocked);
-}
-
-static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user *buf,
- size_t count, loff_t *off)
+static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
+ size_t count, loff_t *off)
{
struct gpio_chardev_data *cdev = file->private_data;
struct gpio_v2_line_info_changed event;
@@ -2707,6 +2629,8 @@ static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user *buf,
int ret;
size_t event_size;
+ gdev_guard(&cdev->gdev);
+
if (!cdev->gdev->chip)
return -ENODEV;
@@ -2769,15 +2693,6 @@ static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user *buf,
return bytes_read;
}
-static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
- size_t count, loff_t *off)
-{
- struct gpio_chardev_data *cdev = file->private_data;
-
- return call_read_locked(file, buf, count, off, cdev->gdev,
- lineinfo_watch_read_unlocked);
-}
-
/**
* gpio_chrdev_open() - open the chardev for ioctl operations
* @inode: inode for this chardev
@@ -2791,17 +2706,15 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
struct gpio_chardev_data *cdev;
int ret = -ENOMEM;
- down_read(&gdev->sem);
+ gdev_guard(&gdev);
/* Fail on open if the backing gpiochip is gone */
- if (!gdev->chip) {
- ret = -ENODEV;
- goto out_unlock;
- }
+ if (!gdev->chip)
+ return -ENODEV;
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
if (!cdev)
- goto out_unlock;
+ return -ENODEV;
cdev->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);
if (!cdev->watched_lines)
@@ -2830,8 +2743,6 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
if (ret)
goto out_unregister_device_notifier;
- up_read(&gdev->sem);
-
return ret;
out_unregister_device_notifier:
@@ -2845,8 +2756,6 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
bitmap_free(cdev->watched_lines);
out_free_cdev:
kfree(cdev);
-out_unlock:
- up_read(&gdev->sem);
return ret;
}