On 9/26/20 9:18 PM, William Breathitt Gray wrote:
+static ssize_t counter_chrdev_read(struct file *filp, char __user *buf,
+ size_t len, loff_t *f_ps)
+{
+ struct counter_device *const counter = filp->private_data;
+ int err;
+ unsigned long flags;
+ unsigned int copied;
+
+ if (len < sizeof(struct counter_event))
+ return -EINVAL;
+
+ do {
+ if (kfifo_is_empty(&counter->events)) {
+ if (filp->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ err = wait_event_interruptible(counter->events_wait,
+ !kfifo_is_empty(&counter->events));
+ if (err)
+ return err;
+ }
+
+ raw_spin_lock_irqsave(&counter->events_lock, flags);
+ err = kfifo_to_user(&counter->events, buf, len, &copied);
+ raw_spin_unlock_irqrestore(&counter->events_lock, flags);
+ if (err)
+ return err;
+ } while (!copied);
+
+ return copied;
+}
All other uses of kfifo_to_user() I saw use a mutex instead of spin
lock. I don't see a reason for disabling interrupts here.
Example:
static ssize_t iio_event_chrdev_read(struct file *filep,
char __user *buf,
size_t count,
loff_t *f_ps)
{
struct iio_dev *indio_dev = filep->private_data;
struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
unsigned int copied;
int ret;
if (!indio_dev->info)
return -ENODEV;
if (count < sizeof(struct iio_event_data))
return -EINVAL;
do {
if (kfifo_is_empty(&ev_int->det_events)) {
if (filep->f_flags & O_NONBLOCK)
return -EAGAIN;
ret = wait_event_interruptible(ev_int->wait,
!kfifo_is_empty(&ev_int->det_events) ||
indio_dev->info == NULL);
if (ret)
return ret;
if (indio_dev->info == NULL)
return -ENODEV;
}
if (mutex_lock_interruptible(&ev_int->read_lock))
return -ERESTARTSYS;
ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
mutex_unlock(&ev_int->read_lock);
if (ret)
return ret;
/*
* If we couldn't read anything from the fifo (a different
* thread might have been faster) we either return -EAGAIN if
* the file descriptor is non-blocking, otherwise we go back to
* sleep and wait for more data to arrive.
*/
if (copied == 0 && (filep->f_flags & O_NONBLOCK))
return -EAGAIN;
} while (copied == 0);
return copied;
}
