From patchwork Tue Aug 2 05:44:07 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiner Kallweit X-Patchwork-Id: 9255201 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A55196048B for ; Tue, 2 Aug 2016 05:59:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 941AA284F4 for ; Tue, 2 Aug 2016 05:59:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 88E9028506; Tue, 2 Aug 2016 05:59:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1400B284F4 for ; Tue, 2 Aug 2016 05:59:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756151AbcHBF67 (ORCPT ); Tue, 2 Aug 2016 01:58:59 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36165 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756104AbcHBF6z (ORCPT ); Tue, 2 Aug 2016 01:58:55 -0400 Received: by mail-wm0-f66.google.com with SMTP id x83so28981805wma.3 for ; Mon, 01 Aug 2016 22:58:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:subject:to:cc:message-id:date:user-agent:mime-version :content-transfer-encoding; bh=6/IH+cFpFToPoc4ObXLyeFtxMHe5Jdbgg8x/pWNgfoo=; b=gYp281QDGXTLR810P3CXpkuPNerMwXgkrI5d9XHIF0Z7hYP88NXcTvJ/f8aSXBUwHF ZJnqMfgC6Map+PEexPvMNCQij5Tshu/UdTYCZ6Ctqd1Z4bdbJ3RRmQ0aDNRic3+WJAZc 46hWfVW4S5FAL3UmW3H0MOCxuza/ZB4FDwC86IGdExs1aaXWWpP/LjWbBQuxIyYBvO47 i7lbTEnPoON1H5iXyGNYDpzx5D0pUtGOZYZyMHgt2lr0TGoXee2KPEbUXr9G7Mbdk2ac txzMXsCRz3MH6tctcdfMTthy2Br1yynRmrVqSga3HJv5HtbwyCOUIyiCJahaJCYuEfuu O9AQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:cc:message-id:date:user-agent :mime-version:content-transfer-encoding; bh=6/IH+cFpFToPoc4ObXLyeFtxMHe5Jdbgg8x/pWNgfoo=; b=DM1jJrZupE8ibyDmF6KfWSx0pHSCqVM9rg6vf5ulaMy+UIYYHLWztG+FS1/8UJluZM xwLsTC6j04GbPzX7uKeU5YnbSiXNJqs8HR6T8qu7DddqIg1gaGaDmjEov8n0KCG0B7W5 EhRsbNrNTfX/mSHFr15Rwv24kVVFNs6nKLsQso1sKrrghE2Lb4L89cmjUAm7YkD6ZM5l Vf5ycPnrcdQYibpXod29qKziLP35OtKPmnWLnkuijjb7uOLZodlZgMY0PSfKWvnqJsKP MoPkTbziHsT7Re5/kgNxLzHBm8KaP29fwT2Rvxx4BenoqH0EQjg0ZyQeYaaGcV43uGZj zf3g== X-Gm-Message-State: AEkoouuNeBBHdIxUHnMpnoXk07+AHujdgfcjtoMDB1KrY+9/+ReqTEqFhDDIpepv41EK7A== X-Received: by 10.194.103.3 with SMTP id fs3mr55392668wjb.115.1470116759707; Mon, 01 Aug 2016 22:45:59 -0700 (PDT) Received: from ?IPv6:2003:62:5f01:4400:28d9:4ab3:6068:8ae5? (p200300625F01440028D94AB360688AE5.dip0.t-ipconnect.de. [2003:62:5f01:4400:28d9:4ab3:6068:8ae5]) by smtp.googlemail.com with ESMTPSA id r127sm20183237wmf.23.2016.08.01.22.45.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 01 Aug 2016 22:45:58 -0700 (PDT) From: Heiner Kallweit Subject: [PATCH] media: rc: refactor raw handler kthread To: Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org Message-ID: Date: Tue, 2 Aug 2016 07:44:07 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I think we can get rid of the spinlock protecting the kthread from being interrupted by a wakeup in certain parts. Even with the current implementation of the kthread the only lost wakeup scenario could happen if the wakeup occurs between the kfifo_len check and setting the state to TASK_INTERRUPTIBLE. In the changed version we could lose a wakeup if it occurs between processing the fifo content and setting the state to TASK_INTERRUPTIBLE. This scenario is covered by an additional check for available events in the fifo and setting the state to TASK_RUNNING in this case. In addition the changed version flushes the kfifo before ending when the kthread is stopped. With this patch we gain: - Get rid of the spinlock - Simplify code - Don't grep / release the mutex for each individual event but just once for the complete fifo content. This reduces overhead if a driver e.g. triggers processing after writing the content of a hw fifo to the kfifo. Signed-off-by: Heiner Kallweit Tested-by: Sean Young --- drivers/media/rc/rc-core-priv.h | 2 -- drivers/media/rc/rc-ir-raw.c | 46 +++++++++++++++-------------------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h index 585d5e5..577128a 100644 --- a/drivers/media/rc/rc-core-priv.h +++ b/drivers/media/rc/rc-core-priv.h @@ -20,7 +20,6 @@ #define MAX_IR_EVENT_SIZE 512 #include -#include #include struct ir_raw_handler { @@ -37,7 +36,6 @@ struct ir_raw_handler { struct ir_raw_event_ctrl { struct list_head list; /* to keep track of raw clients */ struct task_struct *thread; - spinlock_t lock; /* fifo for the pulse/space durations */ DECLARE_KFIFO(kfifo, struct ir_raw_event, MAX_IR_EVENT_SIZE); ktime_t last_event; /* when last event occurred */ diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c index 205ecc6..71a3e17 100644 --- a/drivers/media/rc/rc-ir-raw.c +++ b/drivers/media/rc/rc-ir-raw.c @@ -17,7 +17,6 @@ #include #include #include -#include #include "rc-core-priv.h" /* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */ @@ -35,32 +34,26 @@ static int ir_raw_event_thread(void *data) struct ir_raw_handler *handler; struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl *)data; - while (!kthread_should_stop()) { - - spin_lock_irq(&raw->lock); - - if (!kfifo_len(&raw->kfifo)) { - set_current_state(TASK_INTERRUPTIBLE); - - if (kthread_should_stop()) - set_current_state(TASK_RUNNING); - - spin_unlock_irq(&raw->lock); - schedule(); - continue; + while (1) { + mutex_lock(&ir_raw_handler_lock); + while (kfifo_out(&raw->kfifo, &ev, 1)) { + list_for_each_entry(handler, &ir_raw_handler_list, list) + if (raw->dev->enabled_protocols & + handler->protocols || !handler->protocols) + handler->decode(raw->dev, ev); + raw->prev_ev = ev; } + mutex_unlock(&ir_raw_handler_lock); - if(!kfifo_out(&raw->kfifo, &ev, 1)) - dev_err(&raw->dev->dev, "IR event FIFO is empty!\n"); - spin_unlock_irq(&raw->lock); + set_current_state(TASK_INTERRUPTIBLE); - mutex_lock(&ir_raw_handler_lock); - list_for_each_entry(handler, &ir_raw_handler_list, list) - if (raw->dev->enabled_protocols & handler->protocols || - !handler->protocols) - handler->decode(raw->dev, ev); - raw->prev_ev = ev; - mutex_unlock(&ir_raw_handler_lock); + if (kthread_should_stop()) { + __set_current_state(TASK_RUNNING); + break; + } else if (!kfifo_is_empty(&raw->kfifo)) + set_current_state(TASK_RUNNING); + + schedule(); } return 0; @@ -219,14 +212,10 @@ EXPORT_SYMBOL_GPL(ir_raw_event_set_idle); */ void ir_raw_event_handle(struct rc_dev *dev) { - unsigned long flags; - if (!dev->raw) return; - spin_lock_irqsave(&dev->raw->lock, flags); wake_up_process(dev->raw->thread); - spin_unlock_irqrestore(&dev->raw->lock, flags); } EXPORT_SYMBOL_GPL(ir_raw_event_handle); @@ -274,7 +263,6 @@ int ir_raw_event_register(struct rc_dev *dev) dev->change_protocol = change_protocol; INIT_KFIFO(dev->raw->kfifo); - spin_lock_init(&dev->raw->lock); dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw, "rc%u", dev->minor);