Messages in this thread | | | Date | Fri, 13 Apr 2012 18:50:22 +0200 | Subject | Re: [PATCH] Input: serio_raw - ensure we don't block in non-blocking read | From | David Herrmann <> |
| |
Hi Dmitry
On Wed, Apr 11, 2012 at 9:23 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Tue, Apr 03, 2012 at 01:24:33PM -0700, Dmitry Torokhov wrote: >> Avoid calling wait_event_interruptible() if client requested non-blocking >> read, since it is not guaranteed that another thread will not consume >> event after we checked if serio_raw->head != serio_raw->tail. >> >> Also ensure we do not return 0 but keep waiting instead in blocking case, >> when another thread steals "our" byte. >> >> Signed-off-by: Dmitry Torokhov <dtor@mail.ru> > > Anyone? It would be nice if someone could review my patches as well, > otherwise we may end up with breakages...
I cannot test the driver as I don't have the devices, but I reviewed it and it looks fine.
> Thanks.
Regards David
>> --- >> drivers/input/serio/serio_raw.c | 43 ++++++++++++++++++++++---------------- >> 1 files changed, 25 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c >> index 4494233..094d6fb 100644 >> --- a/drivers/input/serio/serio_raw.c >> +++ b/drivers/input/serio/serio_raw.c >> @@ -165,31 +165,38 @@ static ssize_t serio_raw_read(struct file *file, char __user *buffer, >> struct serio_raw *serio_raw = client->serio_raw; >> char uninitialized_var(c); >> ssize_t read = 0; >> - int retval; >> + int error = 0; >> >> - if (serio_raw->dead) >> - return -ENODEV; >> + do { >> + if (serio_raw->dead) >> + return -ENODEV; >> >> - if (serio_raw->head == serio_raw->tail && (file->f_flags & O_NONBLOCK)) >> - return -EAGAIN; >> + if (serio_raw->head == serio_raw->tail && >> + (file->f_flags & O_NONBLOCK)) >> + return -EAGAIN; >> >> - retval = wait_event_interruptible(serio_raw->wait, >> - serio_raw->head != serio_raw->tail || serio_raw->dead); >> - if (retval) >> - return retval; >> + if (count == 0) >> + break; >> >> - if (serio_raw->dead) >> - return -ENODEV; >> + while (read < count && serio_raw_fetch_byte(serio_raw, &c)) { >> + if (put_user(c, buffer++)) { >> + error = -EFAULT; >> + goto out; >> + } >> + read++; >> + } >> >> - while (read < count && serio_raw_fetch_byte(serio_raw, &c)) { >> - if (put_user(c, buffer++)) { >> - retval = -EFAULT; >> + if (read) >> break; >> - } >> - read++; >> - } >> >> - return read ?: retval; >> + if (!(file->f_flags & O_NONBLOCK)) >> + error = wait_event_interruptible(serio_raw->wait, >> + serio_raw->head != serio_raw->tail || >> + serio_raw->dead); >> + } while (!error); >> + >> +out: >> + return read ?: error; >> } >> >> static ssize_t serio_raw_write(struct file *file, const char __user *buffer, >> -- >> 1.7.7.6 >> > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |