All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Walls <awalls@md.metrocast.net>
To: linux-media@vger.kernel.org
Subject: [PATCH 02/13] lirc_zilog: Remove broken, ineffective reference counting
Date: Thu, 17 Feb 2011 20:13:50 -0500	[thread overview]
Message-ID: <1297991630.9399.18.camel@localhost> (raw)
In-Reply-To: <1297991502.9399.16.camel@localhost>


The set_use_inc() and set_use_dec() functions tried to lock
the underlying bridge driver device instance in memory by
changing the use count on the device's i2c_clients.  This
worked for PCI devices (ivtv, cx18, bttv).  It doesn't
work for hot-pluggable usb devices (pvrusb2 and hdpvr).
With usb device instances, the driver may get locked into
memory, but the unplugged hardware is gone.

The set_use_inc() set_use_dec() functions also tried to have
lirc_zilog change its own module refernce count, which is
racy and not guaranteed to work.  The lirc_dev module does
actually perform proper module ref count manipulation on the
lirc_zilog module, so there is need for lirc_zilog to
attempt a buggy module get on itself anyway.

lirc_zilog also errantly called these functions on itself
in open() and close(), but lirc_dev did that already too.

So let's just gut the bodies of the set_use_*() functions,
and remove the extra calls to them from within lirc_zilog.

Proper reference counting of the struct IR, IR_rx, and IR_tx
objects -- to handle the case when the underlying
bttv, ivtv, cx18, hdpvr, or pvrusb2 bridge driver module or
device instance goes away -- will be added in subsequent
patches.

Signed-off-by: Andy Walls <awalls@md.metrocast.net>
---
 drivers/staging/lirc/lirc_zilog.c |   32 +-------------------------------
 1 files changed, 1 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c
index 7389b77..3a91257 100644
--- a/drivers/staging/lirc/lirc_zilog.c
+++ b/drivers/staging/lirc/lirc_zilog.c
@@ -305,34 +305,12 @@ static int lirc_thread(void *arg)
 
 static int set_use_inc(void *data)
 {
-	struct IR *ir = data;
-
-	if (ir->l.owner == NULL || try_module_get(ir->l.owner) == 0)
-		return -ENODEV;
-
-	/* lock bttv in memory while /dev/lirc is in use  */
-	/*
-	 * this is completely broken code. lirc_unregister_driver()
-	 * must be possible even when the device is open
-	 */
-	if (ir->rx != NULL)
-		i2c_use_client(ir->rx->c);
-	if (ir->tx != NULL)
-		i2c_use_client(ir->tx->c);
-
 	return 0;
 }
 
 static void set_use_dec(void *data)
 {
-	struct IR *ir = data;
-
-	if (ir->rx)
-		i2c_release_client(ir->rx->c);
-	if (ir->tx)
-		i2c_release_client(ir->tx->c);
-	if (ir->l.owner != NULL)
-		module_put(ir->l.owner);
+	return;
 }
 
 /* safe read of a uint32 (always network byte order) */
@@ -1098,7 +1076,6 @@ static struct IR *find_ir_device_by_minor(unsigned int minor)
 static int open(struct inode *node, struct file *filep)
 {
 	struct IR *ir;
-	int ret;
 	unsigned int minor = MINOR(node->i_rdev);
 
 	/* find our IR struct */
@@ -1112,12 +1089,6 @@ static int open(struct inode *node, struct file *filep)
 	/* increment in use count */
 	mutex_lock(&ir->ir_lock);
 	++ir->open;
-	ret = set_use_inc(ir);
-	if (ret != 0) {
-		--ir->open;
-		mutex_unlock(&ir->ir_lock);
-		return ret;
-	}
 	mutex_unlock(&ir->ir_lock);
 
 	/* stash our IR struct */
@@ -1139,7 +1110,6 @@ static int close(struct inode *node, struct file *filep)
 	/* decrement in use count */
 	mutex_lock(&ir->ir_lock);
 	--ir->open;
-	set_use_dec(ir);
 	mutex_unlock(&ir->ir_lock);
 
 	return 0;
-- 
1.7.2.1




  parent reply	other threads:[~2011-02-18  1:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-18  1:11 [PATCH 0/13] lirc_zilog: Ref-counting and locking cleanup Andy Walls
2011-02-18  1:12 ` [PATCH 01/13] lirc_zilog: Restore checks for existence of the IR_tx object Andy Walls
2011-02-18  1:13 ` Andy Walls [this message]
2011-02-18  1:14 ` [PATCH 03/13] lirc_zilog: Convert ir_device instance array to a linked list Andy Walls
2011-02-18  1:15 ` [PATCH 04/13] lirc_zilog: Convert the instance open count to an atomic_t Andy Walls
2011-02-18  1:15 ` [PATCH 05/13] lirc_zilog: Use kernel standard methods for marking device non-seekable Andy Walls
2011-02-18  1:16 ` [PATCH 06/13] lirc_zilog: Don't acquire the rx->buf_lock in the poll() function Andy Walls
2011-02-18  1:17 ` [PATCH 07/13] lirc_zilog: Remove unneeded rx->buf_lock Andy Walls
2011-02-18  1:18 ` [PATCH 08/13] lirc_zilog: Always allocate a Rx lirc_buffer object Andy Walls
2011-02-18  1:19 ` [PATCH 09/13] lirc_zilog: Move constants from ir_probe() into the lirc_driver template Andy Walls
2011-02-18  1:20 ` [PATCH 10/13] lirc_zilog: Add ref counting of struct IR, IR_tx, and IR_rx Andy Walls
2011-02-18  1:20 ` [PATCH 11/13] lirc_zilog: Add locking of the i2c_clients when in use Andy Walls
2011-02-18  1:21 ` [PATCH 12/13] lirc_zilog: Fix somewhat confusing information messages in ir_probe() Andy Walls
2011-02-18  1:22 ` [PATCH 13/13] lirc_zilog: Update TODO list based on work completed and revised plans Andy Walls
2011-03-06  4:00 ` [PATCH 0/13] lirc_zilog: Ref-counting and locking cleanup Jarod Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1297991630.9399.18.camel@localhost \
    --to=awalls@md.metrocast.net \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.