From patchwork Thu Apr 3 23:34:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?RGF2aWQgSMODwqRyZGVtYW4=?= X-Patchwork-Id: 3935051 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B20589F334 for ; Thu, 3 Apr 2014 23:34:53 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B80BA20320 for ; Thu, 3 Apr 2014 23:34:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 937E820306 for ; Thu, 3 Apr 2014 23:34:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753636AbaDCXer (ORCPT ); Thu, 3 Apr 2014 19:34:47 -0400 Received: from hardeman.nu ([95.142.160.32]:40343 "EHLO hardeman.nu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753909AbaDCXek (ORCPT ); Thu, 3 Apr 2014 19:34:40 -0400 Received: from zeus.hardeman.nu (ppp-188-174-176-74.dynamic.mnet-online.de [188.174.176.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by vader.hardeman.nu (Postfix) with ESMTPSA id C2449273; Fri, 4 Apr 2014 01:34:38 +0200 (CEST) Received: by zeus.hardeman.nu (Postfix, from userid 1000) id 78376C600FA; Fri, 4 Apr 2014 01:34:38 +0200 (CEST) Subject: [PATCH 40/49] rc-ir-raw: simplify locking From: David =?utf-8?b?SMOkcmRlbWFu?= To: linux-media@vger.kernel.org Cc: m.chehab@samsung.com Date: Fri, 04 Apr 2014 01:34:38 +0200 Message-ID: <20140403233438.27099.1183.stgit@zeus.muc.hardeman.nu> In-Reply-To: <20140403232420.27099.94872.stgit@zeus.muc.hardeman.nu> References: <20140403232420.27099.94872.stgit@zeus.muc.hardeman.nu> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Simplify and improve the locking in rc-ir-raw by making better use of the existing kfifo functionality and by using RCU where possible. Signed-off-by: David Härdeman --- drivers/media/rc/rc-core-priv.h | 6 +- drivers/media/rc/rc-ir-raw.c | 124 ++++++++++++++++----------------------- 2 files changed, 55 insertions(+), 75 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h index 0b32ef8..c3de26b 100644 --- a/drivers/media/rc/rc-core-priv.h +++ b/drivers/media/rc/rc-core-priv.h @@ -37,11 +37,13 @@ struct ir_raw_handler { int (*raw_unregister)(struct rc_dev *dev); }; +/* max number of pulse/space transitions to buffer */ +#define RC_MAX_IR_EVENTS 512 + struct ir_raw_event_ctrl { struct list_head list; /* to keep track of raw clients */ struct task_struct *thread; - spinlock_t lock; - struct kfifo_rec_ptr_1 kfifo; /* fifo for the pulse/space durations */ + DECLARE_KFIFO(kfifo, struct ir_raw_event, RC_MAX_IR_EVENTS); /* for pulse/space durations */ ktime_t last_event; /* when last event occurred */ enum raw_event_type last_type; /* last event type */ struct rc_dev *dev; /* pointer to the parent rc_dev */ diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c index 86f5aa7..9631825 100644 --- a/drivers/media/rc/rc-ir-raw.c +++ b/drivers/media/rc/rc-ir-raw.c @@ -21,15 +21,12 @@ #include #include "rc-core-priv.h" -/* Define the max number of pulse/space transitions to buffer */ -#define MAX_IR_EVENT_SIZE 512 - -/* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */ +/* IR raw clients/handlers, writers synchronize with ir_raw_mutex */ +static DEFINE_MUTEX(ir_raw_mutex); static LIST_HEAD(ir_raw_client_list); - -/* Used to handle IR raw handler extensions */ -static DEFINE_MUTEX(ir_raw_handler_lock); static LIST_HEAD(ir_raw_handler_list); + +/* protocols supported by the currently loaded decoders */ static u64 available_protocols; static int ir_raw_event_thread(void *data) @@ -37,32 +34,19 @@ static int ir_raw_event_thread(void *data) struct ir_raw_event ev; struct ir_raw_handler *handler; struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl *)data; - int retval; while (!kthread_should_stop()) { - - spin_lock_irq(&raw->lock); - retval = kfifo_len(&raw->kfifo); - - if (retval < sizeof(ev)) { + if (kfifo_out(&raw->kfifo, &ev, 1) == 0) { set_current_state(TASK_INTERRUPTIBLE); - - if (kthread_should_stop()) - set_current_state(TASK_RUNNING); - - spin_unlock_irq(&raw->lock); schedule(); continue; } - retval = kfifo_out(&raw->kfifo, &ev, sizeof(ev)); - spin_unlock_irq(&raw->lock); - - mutex_lock(&ir_raw_handler_lock); - list_for_each_entry(handler, &ir_raw_handler_list, list) + rcu_read_lock(); + list_for_each_entry_rcu(handler, &ir_raw_handler_list, list) handler->decode(raw->dev, ev); + rcu_read_unlock(); raw->prev_ev = ev; - mutex_unlock(&ir_raw_handler_lock); } return 0; @@ -76,7 +60,8 @@ static int ir_raw_event_thread(void *data) * This routine (which may be called from an interrupt context) stores a * pulse/space duration for the raw ir decoding state machines. Pulses are * signalled as positive values and spaces as negative values. A zero value - * will reset the decoding state machines. + * will reset the decoding state machines. Drivers are responsible for + * synchronizing calls to this function. */ int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev) { @@ -86,7 +71,7 @@ int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev) IR_dprintk(2, "sample: (%05dus %s)\n", TO_US(ev->duration), TO_STR(ev->pulse)); - if (kfifo_in(&dev->raw->kfifo, ev, sizeof(*ev)) != sizeof(*ev)) + if (kfifo_in(&dev->raw->kfifo, ev, 1) != 1) return -ENOMEM; return 0; @@ -258,14 +243,8 @@ EXPORT_SYMBOL_GPL(ir_raw_get_tx_event); */ 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); + if (dev->raw) + wake_up_process(dev->raw->thread); } EXPORT_SYMBOL_GPL(ir_raw_event_handle); @@ -273,9 +252,9 @@ EXPORT_SYMBOL_GPL(ir_raw_event_handle); static u64 ir_raw_get_allowed_protocols(struct rc_dev *dev) { u64 protocols; - mutex_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_mutex); protocols = available_protocols; - mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_mutex); return protocols; } @@ -290,52 +269,49 @@ static int change_protocol(struct rc_dev *dev, u64 *rc_type) { int rc_register_ir_raw_device(struct rc_dev *dev) { int rc; + struct ir_raw_event_ctrl *raw; struct ir_raw_handler *handler; if (!dev) return -EINVAL; - dev->raw = kzalloc(sizeof(*dev->raw), GFP_KERNEL); - if (!dev->raw) + raw = kzalloc(sizeof(*raw), GFP_KERNEL); + if (!raw) return -ENOMEM; - dev->raw->dev = dev; - dev->enabled_protocols = ~0; - dev->get_protocols = ir_raw_get_allowed_protocols; - dev->driver_type = RC_DRIVER_IR_RAW; - dev->change_protocol = change_protocol; - spin_lock_init(&dev->raw->lock); - rc = kfifo_alloc(&dev->raw->kfifo, - sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE, - GFP_KERNEL); - if (rc < 0) - goto out; - - dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw, - dev_name(&dev->dev)); + raw->dev = dev; + INIT_KFIFO(raw->kfifo); + raw->thread = kthread_run(ir_raw_event_thread, dev->raw, + dev_name(&dev->dev)); - if (IS_ERR(dev->raw->thread)) { - rc = PTR_ERR(dev->raw->thread); + if (IS_ERR(raw->thread)) { + rc = PTR_ERR(raw->thread); goto out; } + dev->raw = raw; + dev->enabled_protocols = ~0; + dev->get_protocols = ir_raw_get_allowed_protocols; + dev->driver_type = RC_DRIVER_IR_RAW; + dev->change_protocol = change_protocol; rc = rc_register_device(dev); if (rc < 0) goto out_thread; - mutex_lock(&ir_raw_handler_lock); - list_add_tail(&dev->raw->list, &ir_raw_client_list); - list_for_each_entry(handler, &ir_raw_handler_list, list) + mutex_lock(&ir_raw_mutex); + list_add_tail_rcu(&dev->raw->list, &ir_raw_client_list); + list_for_each_entry_rcu(handler, &ir_raw_handler_list, list) if (handler->raw_register) handler->raw_register(dev); - mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_mutex); + synchronize_rcu(); return 0; out_thread: - kthread_stop(dev->raw->thread); + kthread_stop(raw->thread); out: - kfree(dev->raw); + kfree(raw); dev->raw = NULL; return rc; } @@ -350,14 +326,14 @@ void rc_unregister_ir_raw_device(struct rc_dev *dev) kthread_stop(dev->raw->thread); - mutex_lock(&ir_raw_handler_lock); - list_del(&dev->raw->list); - list_for_each_entry(handler, &ir_raw_handler_list, list) + mutex_lock(&ir_raw_mutex); + list_del_rcu(&dev->raw->list); + list_for_each_entry_rcu(handler, &ir_raw_handler_list, list) if (handler->raw_unregister) handler->raw_unregister(dev); - mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_mutex); + synchronize_rcu(); - kfifo_free(&dev->raw->kfifo); kfree(dev->raw); dev->raw = NULL; rc_unregister_device(dev); @@ -372,13 +348,14 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler) { struct ir_raw_event_ctrl *raw; - mutex_lock(&ir_raw_handler_lock); - list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list); + mutex_lock(&ir_raw_mutex); + list_add_tail_rcu(&ir_raw_handler->list, &ir_raw_handler_list); if (ir_raw_handler->raw_register) - list_for_each_entry(raw, &ir_raw_client_list, list) + list_for_each_entry_rcu(raw, &ir_raw_client_list, list) ir_raw_handler->raw_register(raw->dev); available_protocols |= ir_raw_handler->protocols; - mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_mutex); + synchronize_rcu(); return 0; } @@ -388,13 +365,14 @@ void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler) { struct ir_raw_event_ctrl *raw; - mutex_lock(&ir_raw_handler_lock); - list_del(&ir_raw_handler->list); + mutex_lock(&ir_raw_mutex); + list_del_rcu(&ir_raw_handler->list); if (ir_raw_handler->raw_unregister) - list_for_each_entry(raw, &ir_raw_client_list, list) + list_for_each_entry_rcu(raw, &ir_raw_client_list, list) ir_raw_handler->raw_unregister(raw->dev); available_protocols &= ~ir_raw_handler->protocols; - mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_mutex); + synchronize_rcu(); } EXPORT_SYMBOL(ir_raw_handler_unregister);