From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE16AC2D0CF for ; Tue, 24 Dec 2019 12:07:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 83A9B20706 for ; Tue, 24 Dec 2019 12:07:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=bgdev-pl.20150623.gappssmtp.com header.i=@bgdev-pl.20150623.gappssmtp.com header.b="0o8HZKMS" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727150AbfLXMHy (ORCPT ); Tue, 24 Dec 2019 07:07:54 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:45364 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726262AbfLXMH2 (ORCPT ); Tue, 24 Dec 2019 07:07:28 -0500 Received: by mail-wr1-f66.google.com with SMTP id j42so19621596wrj.12 for ; Tue, 24 Dec 2019 04:07:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bgdev-pl.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=QOztRCKkDevhaR13ODjX/tyuWWrRBmljEfCTgBRtZ7Y=; b=0o8HZKMS+dnZ0sJqxTAlZdnCqE85Kypvyo4GXYiLN82NvTx6o3n/amIz6UUuyLvffQ oJQ0nHr5A3s8nc64+TK304qfcBLl/9dMU59eBg+djomRpIlVe6RD98KLGkGEB89uMojR UeTKxI8jZGqHm74cgewK2IMezO4c6zQAmWmF10vixnj0bG25/jvaNtyrrKFbKv+XaX7W WgVqQitj9j4N1PHqTaVZEehnNa9H8mSjBNFUqyGGurUiD+S/YgONspZuWJ6/kARxwvvm mMMJfA4D3N14Kg+w4f4fb+8rw0aL1nXKUBr7LWDnnfdd/dkajGzrMeF2bOMn9SkjmaiZ QGCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=QOztRCKkDevhaR13ODjX/tyuWWrRBmljEfCTgBRtZ7Y=; b=Dur2Rs+5eITspxCxHFIfvD6x7nQO0A6MpAYPgoAJSvVbcF31aNmiA3xd0GC7vuuhK6 eEOcrIORGh3n5Zj2yevhfWla8XG0NtLK9iaIugezBNycpGnC5bUdzMs4KuT5S4eEhZ40 VsHscCkB1HFUtCZ/KEZvKwrtZoCRVGB0atbMoJp1/j3uK01f4d/+gULHb6P7AR2+KDg0 9HqHSJGK1mnXJG1cJ0KUZS305VH1AXRoPymqQu5vLP0MEXBtGZgzfioM8Su+XJ/AGsWo Yw2FjEuTZknh5BR5vbj9ttLiv3mM27KQ0Wdh2JM001aGeF7+qF+FV+b1Bdm/tw9UuL8V tqSA== X-Gm-Message-State: APjAAAWMlJZJRGyJB2xyovtI0dYLN0PN9F53Is00c6J6SyFkftYj74oM pgNxvLY2c6KXiBfXkj7B6mTGeQ== X-Google-Smtp-Source: APXvYqwg9zUaDjpOwTDfb3yQWchmGT0Q/A2tJCYwl4QJemam2J77rZ8cf1RRCdP6RoEbZV/w/LVcpw== X-Received: by 2002:a5d:62c8:: with SMTP id o8mr34614320wrv.316.1577189246440; Tue, 24 Dec 2019 04:07:26 -0800 (PST) Received: from debian-brgl.home ([2a01:cb1d:af:5b00:6d6c:8493:1ab5:dad7]) by smtp.gmail.com with ESMTPSA id s10sm23829210wrw.12.2019.12.24.04.07.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Dec 2019 04:07:25 -0800 (PST) From: Bartosz Golaszewski To: Kent Gibson , Linus Walleij , Andy Shevchenko , Greg Kroah-Hartman Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, Bartosz Golaszewski Subject: [PATCH v4 09/13] gpiolib: rework the locking mechanism for lineevent kfifo Date: Tue, 24 Dec 2019 13:07:05 +0100 Message-Id: <20191224120709.18247-10-brgl@bgdev.pl> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191224120709.18247-1-brgl@bgdev.pl> References: <20191224120709.18247-1-brgl@bgdev.pl> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Bartosz Golaszewski The read_lock mutex is supposed to prevent collisions between reading and writing to the line event kfifo but it's actually only taken when the events are being read from it. Drop the mutex entirely and reuse the spinlock made available to us in the waitqueue struct. Take the lock whenever the fifo is modified or inspected. Drop the call to kfifo_to_user() and instead first extract the new element from kfifo when the lock is taken and only then pass it on to the user after the spinlock is released. Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib.c | 64 +++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 81d5eda4de7d..a859c0813e0d 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -788,8 +788,6 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) * @irq: the interrupt that trigger in response to events on this GPIO * @wait: wait queue that handles blocking reads of events * @events: KFIFO for the GPIO events - * @read_lock: mutex lock to protect reads from colliding with adding - * new events to the FIFO * @timestamp: cache for the timestamp storing it between hardirq * and IRQ thread, used to bring the timestamp close to the actual * event @@ -802,7 +800,6 @@ struct lineevent_state { int irq; wait_queue_head_t wait; DECLARE_KFIFO(events, struct gpioevent_data, 16); - struct mutex read_lock; u64 timestamp; }; @@ -818,7 +815,7 @@ static __poll_t lineevent_poll(struct file *filep, poll_wait(filep, &le->wait, wait); - if (!kfifo_is_empty(&le->events)) + if (!kfifo_is_empty_spinlocked_noirqsave(&le->events, &le->wait.lock)) events = EPOLLIN | EPOLLRDNORM; return events; @@ -831,43 +828,52 @@ static ssize_t lineevent_read(struct file *filep, loff_t *f_ps) { struct lineevent_state *le = filep->private_data; - unsigned int copied; + struct gpioevent_data event; + ssize_t bytes_read = 0; int ret; - if (count < sizeof(struct gpioevent_data)) + if (count < sizeof(event)) return -EINVAL; do { + spin_lock(&le->wait.lock); if (kfifo_is_empty(&le->events)) { - if (filep->f_flags & O_NONBLOCK) + if (bytes_read) { + spin_unlock(&le->wait.lock); + return bytes_read; + } + + if (filep->f_flags & O_NONBLOCK) { + spin_unlock(&le->wait.lock); return -EAGAIN; + } - ret = wait_event_interruptible(le->wait, + ret = wait_event_interruptible_locked(le->wait, !kfifo_is_empty(&le->events)); - if (ret) + if (ret) { + spin_unlock(&le->wait.lock); return ret; + } } - if (mutex_lock_interruptible(&le->read_lock)) - return -ERESTARTSYS; - ret = kfifo_to_user(&le->events, buf, count, &copied); - mutex_unlock(&le->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; + ret = kfifo_out(&le->events, &event, 1); + spin_unlock(&le->wait.lock); + if (ret != 1) { + /* + * This should never happen - we were holding the lock + * from the moment we learned the fifo is no longer + * empty until now. + */ + ret = -EIO; + break; + } - } while (copied == 0); + if (copy_to_user(buf + bytes_read, &event, sizeof(event))) + return -EFAULT; + bytes_read += sizeof(event); + } while (count >= bytes_read + sizeof(event)); - return copied; + return bytes_read; } static int lineevent_release(struct inode *inode, struct file *filep) @@ -969,7 +975,8 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p) return IRQ_NONE; } - ret = kfifo_put(&le->events, ge); + ret = kfifo_in_spinlocked_noirqsave(&le->events, &ge, + 1, &le->wait.lock); if (ret) wake_up_poll(&le->wait, EPOLLIN); @@ -1084,7 +1091,6 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip) INIT_KFIFO(le->events); init_waitqueue_head(&le->wait); - mutex_init(&le->read_lock); /* Request a thread to read the events */ ret = request_threaded_irq(le->irq, -- 2.23.0