[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201018165822.GE231549@shinobu>
Date: Sun, 18 Oct 2020 12:58:22 -0400
From: William Breathitt Gray <vilhelm.gray@...il.com>
To: David Lechner <david@...hnology.com>
Cc: jic23@...nel.org, kamel.bouhara@...tlin.com, gwendal@...omium.org,
alexandre.belloni@...tlin.com, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, syednwaris@...il.com,
patrick.havelange@...ensium.com, fabrice.gasnier@...com,
mcoquelin.stm32@...il.com, alexandre.torgue@...com
Subject: Re: [PATCH v5 3/5] counter: Add character device interface
On Wed, Oct 14, 2020 at 05:40:44PM -0500, David Lechner wrote:
> 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.
The Counter character device interface is special in this case because
counter->events could be accessed from an interrupt context. This is
possible if counter_push_event() is called for an interrupt (as is the
case for the 104_quad_8 driver). In this case, we can't use mutex
because we can't sleep in an interrupt context, so our only option is to
use spin lock.
William Breathitt Gray
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists