[PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting

[PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   28 +++++++++++++---------------
 1 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index b7ba459..ef3a69c 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -9,6 +9,7 @@
  * the Free Software Foundation.
  */
 
+#include <linux/kref.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/poll.h>
@@ -33,7 +34,7 @@ struct serio_raw {
 	unsigned int tail, head;
 
 	char name[16];
-	unsigned int refcnt;
+	struct kref kref;
 	struct serio *serio;
 	struct miscdevice dev;
 	wait_queue_head_t wait;
@@ -104,7 +105,7 @@ static int serio_raw_open(struct inode *inode, struct file *file)
 	list->serio_raw = serio_raw;
 	file->private_data = list;
 
-	serio_raw->refcnt++;
+	kref_get(&serio_raw->kref);
 	list_add_tail(&list->node, &serio_raw->list);
 
 out:
@@ -112,17 +113,14 @@ out:
 	return retval;
 }
 
-static int serio_raw_cleanup(struct serio_raw *serio_raw)
+static void serio_raw_cleanup(struct kref *kref)
 {
-	if (--serio_raw->refcnt == 0) {
-		misc_deregister(&serio_raw->dev);
-		list_del_init(&serio_raw->node);
-		kfree(serio_raw);
+	struct serio_raw *serio_raw =
+			container_of(kref, struct serio_raw, kref);
 
-		return 1;
-	}
-
-	return 0;
+	misc_deregister(&serio_raw->dev);
+	list_del_init(&serio_raw->node);
+	kfree(serio_raw);
 }
 
 static int serio_raw_release(struct inode *inode, struct file *file)
@@ -132,7 +130,7 @@ static int serio_raw_release(struct inode *inode, struct file *file)
 
 	mutex_lock(&serio_raw_mutex);
 
-	serio_raw_cleanup(serio_raw);
+	kref_put(&serio_raw->kref, serio_raw_cleanup);
 
 	mutex_unlock(&serio_raw_mutex);
 	return 0;
@@ -283,7 +281,7 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 	mutex_lock(&serio_raw_mutex);
 
 	snprintf(serio_raw->name, sizeof(serio_raw->name), "serio_raw%d", serio_raw_no++);
-	serio_raw->refcnt = 1;
+	kref_init(&serio_raw->kref);
 	serio_raw->serio = serio;
 	INIT_LIST_HEAD(&serio_raw->list);
 	init_waitqueue_head(&serio_raw->wait);
@@ -357,8 +355,8 @@ static void serio_raw_disconnect(struct serio *serio)
 	serio_set_drvdata(serio, NULL);
 
 	serio_raw->serio = NULL;
-	if (!serio_raw_cleanup(serio_raw))
-		wake_up_interruptible(&serio_raw->wait);
+	wake_up_interruptible(&serio_raw->wait);
+	kref_put(&serio_raw->kref, serio_raw_cleanup);
 
 	mutex_unlock(&serio_raw_mutex);
 }
-- 
1.7.6.4

[PATCH 2/9] Input: serio_raw - rename serio_raw_list to serio_raw_client

[Dmitry Torokhov]

'serio_raw_list' and 'list' names do not accurately represent their objects
and are extremely confusing when reading the code. Let's use better suited
names.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   75 ++++++++++++++++++++------------------
 1 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index ef3a69c..6b57ee3 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -38,11 +38,11 @@ struct serio_raw {
 	struct serio *serio;
 	struct miscdevice dev;
 	wait_queue_head_t wait;
-	struct list_head list;
+	struct list_head client_list;
 	struct list_head node;
 };
 
-struct serio_raw_list {
+struct serio_raw_client {
 	struct fasync_struct *fasync;
 	struct serio_raw *serio_raw;
 	struct list_head node;
@@ -58,9 +58,9 @@ static unsigned int serio_raw_no;
 
 static int serio_raw_fasync(int fd, struct file *file, int on)
 {
-	struct serio_raw_list *list = file->private_data;
+	struct serio_raw_client *client = file->private_data;
 
-	return fasync_helper(fd, file, on, &list->fasync);
+	return fasync_helper(fd, file, on, &client->fasync);
 }
 
 static struct serio_raw *serio_raw_locate(int minor)
@@ -78,8 +78,8 @@ static struct serio_raw *serio_raw_locate(int minor)
 static int serio_raw_open(struct inode *inode, struct file *file)
 {
 	struct serio_raw *serio_raw;
-	struct serio_raw_list *list;
-	int retval = 0;
+	struct serio_raw_client *client;
+	int retval;
 
 	retval = mutex_lock_interruptible(&serio_raw_mutex);
 	if (retval)
@@ -96,17 +96,17 @@ static int serio_raw_open(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	list = kzalloc(sizeof(struct serio_raw_list), GFP_KERNEL);
-	if (!list) {
+	client = kzalloc(sizeof(struct serio_raw_client), GFP_KERNEL);
+	if (!client) {
 		retval = -ENOMEM;
 		goto out;
 	}
 
-	list->serio_raw = serio_raw;
-	file->private_data = list;
+	client->serio_raw = serio_raw;
+	file->private_data = client;
 
 	kref_get(&serio_raw->kref);
-	list_add_tail(&list->node, &serio_raw->list);
+	list_add_tail(&client->node, &serio_raw->client_list);
 
 out:
 	mutex_unlock(&serio_raw_mutex);
@@ -125,8 +125,8 @@ static void serio_raw_cleanup(struct kref *kref)
 
 static int serio_raw_release(struct inode *inode, struct file *file)
 {
-	struct serio_raw_list *list = file->private_data;
-	struct serio_raw *serio_raw = list->serio_raw;
+	struct serio_raw_client *client = file->private_data;
+	struct serio_raw *serio_raw = client->serio_raw;
 
 	mutex_lock(&serio_raw_mutex);
 
@@ -156,8 +156,8 @@ static int serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
 
 static ssize_t serio_raw_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
 {
-	struct serio_raw_list *list = file->private_data;
-	struct serio_raw *serio_raw = list->serio_raw;
+	struct serio_raw_client *client = file->private_data;
+	struct serio_raw *serio_raw = client->serio_raw;
 	char uninitialized_var(c);
 	ssize_t retval = 0;
 
@@ -167,8 +167,9 @@ static ssize_t serio_raw_read(struct file *file, char __user *buffer, size_t cou
 	if (serio_raw->head == serio_raw->tail && (file->f_flags & O_NONBLOCK))
 		return -EAGAIN;
 
-	retval = wait_event_interruptible(list->serio_raw->wait,
-					  serio_raw->head != serio_raw->tail || !serio_raw->serio);
+	retval = wait_event_interruptible(serio_raw->wait,
+					  serio_raw->head != serio_raw->tail ||
+						!serio_raw->serio);
 	if (retval)
 		return retval;
 
@@ -186,7 +187,8 @@ static ssize_t serio_raw_read(struct file *file, char __user *buffer, size_t cou
 
 static ssize_t serio_raw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
 {
-	struct serio_raw_list *list = file->private_data;
+	struct serio_raw_client *client = file->private_data;
+	struct serio_raw *serio_raw = client->serio_raw;
 	ssize_t written = 0;
 	int retval;
 	unsigned char c;
@@ -195,7 +197,7 @@ static ssize_t serio_raw_write(struct file *file, const char __user *buffer, siz
 	if (retval)
 		return retval;
 
-	if (!list->serio_raw->serio) {
+	if (!serio_raw->serio) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -208,7 +210,7 @@ static ssize_t serio_raw_write(struct file *file, const char __user *buffer, siz
 			retval = -EFAULT;
 			goto out;
 		}
-		if (serio_write(list->serio_raw->serio, c)) {
+		if (serio_write(serio_raw->serio, c)) {
 			retval = -EIO;
 			goto out;
 		}
@@ -222,25 +224,26 @@ out:
 
 static unsigned int serio_raw_poll(struct file *file, poll_table *wait)
 {
-	struct serio_raw_list *list = file->private_data;
+	struct serio_raw_client *client = file->private_data;
+	struct serio_raw *serio_raw = client->serio_raw;
 
-	poll_wait(file, &list->serio_raw->wait, wait);
+	poll_wait(file, &serio_raw->wait, wait);
 
-	if (list->serio_raw->head != list->serio_raw->tail)
+	if (serio_raw->head != serio_raw->tail)
 		return POLLIN | POLLRDNORM;
 
 	return 0;
 }
 
 static const struct file_operations serio_raw_fops = {
-	.owner =	THIS_MODULE,
-	.open =		serio_raw_open,
-	.release =	serio_raw_release,
-	.read =		serio_raw_read,
-	.write =	serio_raw_write,
-	.poll =		serio_raw_poll,
-	.fasync =	serio_raw_fasync,
-	.llseek = noop_llseek,
+	.owner		= THIS_MODULE,
+	.open		= serio_raw_open,
+	.release	= serio_raw_release,
+	.read		= serio_raw_read,
+	.write		= serio_raw_write,
+	.poll		= serio_raw_poll,
+	.fasync		= serio_raw_fasync,
+	.llseek		= noop_llseek,
 };
 
 
@@ -252,16 +255,16 @@ static irqreturn_t serio_raw_interrupt(struct serio *serio, unsigned char data,
 					unsigned int dfl)
 {
 	struct serio_raw *serio_raw = serio_get_drvdata(serio);
-	struct serio_raw_list *list;
+	struct serio_raw_client *client;
 	unsigned int head = serio_raw->head;
 
-	/* we are holding serio->lock here so we are prootected */
+	/* we are holding serio->lock here so we are protected */
 	serio_raw->queue[head] = data;
 	head = (head + 1) % SERIO_RAW_QUEUE_LEN;
 	if (likely(head != serio_raw->tail)) {
 		serio_raw->head = head;
-		list_for_each_entry(list, &serio_raw->list, node)
-			kill_fasync(&list->fasync, SIGIO, POLL_IN);
+		list_for_each_entry(client, &serio_raw->client_list, node)
+			kill_fasync(&client->fasync, SIGIO, POLL_IN);
 		wake_up_interruptible(&serio_raw->wait);
 	}
 
@@ -283,7 +286,7 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 	snprintf(serio_raw->name, sizeof(serio_raw->name), "serio_raw%d", serio_raw_no++);
 	kref_init(&serio_raw->kref);
 	serio_raw->serio = serio;
-	INIT_LIST_HEAD(&serio_raw->list);
+	INIT_LIST_HEAD(&serio_raw->client_list);
 	init_waitqueue_head(&serio_raw->wait);
 
 	serio_set_drvdata(serio, serio_raw);
-- 
1.7.6.4

[PATCH 3/9] Input: serio_raw - perform proper locking when adding clients to list

[Dmitry Torokhov]

Make sure we hold serio lock when adding clients to client list so that
we do not race with serio_raw_release() removing clients from the same
list.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 6b57ee3..77ce3a6 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -106,7 +106,10 @@ static int serio_raw_open(struct inode *inode, struct file *file)
 	file->private_data = client;
 
 	kref_get(&serio_raw->kref);
+
+	serio_pause_rx(serio_raw->serio);
 	list_add_tail(&client->node, &serio_raw->client_list);
+	serio_continue_rx(serio_raw->serio);
 
 out:
 	mutex_unlock(&serio_raw_mutex);
@@ -138,10 +141,9 @@ static int serio_raw_release(struct inode *inode, struct file *file)
 
 static int serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
 {
-	unsigned long flags;
 	int empty;
 
-	spin_lock_irqsave(&serio_raw->serio->lock, flags);
+	serio_pause_rx(serio_raw->serio);
 
 	empty = serio_raw->head == serio_raw->tail;
 	if (!empty) {
@@ -149,7 +151,7 @@ static int serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
 		serio_raw->tail = (serio_raw->tail + 1) % SERIO_RAW_QUEUE_LEN;
 	}
 
-	spin_unlock_irqrestore(&serio_raw->serio->lock, flags);
+	serio_continue_rx(serio_raw->serio);
 
 	return !empty;
 }
-- 
1.7.6.4

[PATCH 4/9] Input: serio_raw - use bool for boolean data

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 77ce3a6..a5afc7c 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -139,9 +139,9 @@ static int serio_raw_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
+static bool serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
 {
-	int empty;
+	bool empty;
 
 	serio_pause_rx(serio_raw->serio);
 
@@ -394,7 +394,7 @@ static struct serio_driver serio_raw_drv = {
 	.connect	= serio_raw_connect,
 	.reconnect	= serio_raw_reconnect,
 	.disconnect	= serio_raw_disconnect,
-	.manual_bind	= 1,
+	.manual_bind	= true,
 };
 
 static int __init serio_raw_init(void)
-- 
1.7.6.4

[PATCH 5/9] Input: serio_raw - use dev_*() for messages

[Dmitry Torokhov]

This will ensure our reporting is consistent with the rest of the system.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index a5afc7c..396a17f 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -279,7 +279,7 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 	int err;
 
 	if (!(serio_raw = kzalloc(sizeof(struct serio_raw), GFP_KERNEL))) {
-		printk(KERN_ERR "serio_raw.c: can't allocate memory for a device\n");
+		dev_dbg(&serio->dev, "can't allocate memory for a device\n");
 		return -ENOMEM;
 	}
 
@@ -311,13 +311,14 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 	}
 
 	if (err) {
-		printk(KERN_INFO "serio_raw: failed to register raw access device for %s\n",
+		dev_err(&serio->dev,
+			"failed to register raw access device for %s\n",
 			serio->phys);
 		goto out_close;
 	}
 
-	printk(KERN_INFO "serio_raw: raw access enabled on %s (%s, minor %d)\n",
-		serio->phys, serio_raw->name, serio_raw->dev.minor);
+	dev_info(&serio->dev, "raw access enabled on %s (%s, minor %d)\n",
+		 serio->phys, serio_raw->name, serio_raw->dev.minor);
 	goto out;
 
 out_close:
@@ -337,7 +338,8 @@ static int serio_raw_reconnect(struct serio *serio)
 	struct serio_driver *drv = serio->drv;
 
 	if (!drv || !serio_raw) {
-		printk(KERN_DEBUG "serio_raw: reconnect request, but serio is disconnected, ignoring...\n");
+		dev_dbg(&serio->dev,
+			"reconnect request, but serio is disconnected, ignoring...\n");
 		return -1;
 	}
 
-- 
1.7.6.4

[PATCH 6/9] Input: serio_raw - fix coding style issues

[Dmitry Torokhov]

This makes checkpatch.pl happy with the driver

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 396a17f..64fcefb 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -156,7 +156,8 @@ static bool serio_raw_fetch_byte(struct serio_raw *serio_raw, char *c)
 	return !empty;
 }
 
-static ssize_t serio_raw_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t serio_raw_read(struct file *file, char __user *buffer,
+			      size_t count, loff_t *ppos)
 {
 	struct serio_raw_client *client = file->private_data;
 	struct serio_raw *serio_raw = client->serio_raw;
@@ -187,7 +188,8 @@ static ssize_t serio_raw_read(struct file *file, char __user *buffer, size_t cou
 	return retval;
 }
 
-static ssize_t serio_raw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t serio_raw_write(struct file *file, const char __user *buffer,
+			       size_t count, loff_t *ppos)
 {
 	struct serio_raw_client *client = file->private_data;
 	struct serio_raw *serio_raw = client->serio_raw;
@@ -250,7 +252,7 @@ static const struct file_operations serio_raw_fops = {
 
 
 /*********************************************************************
- *                   Interface with serio port   	             *
+ *                   Interface with serio port                       *
  *********************************************************************/
 
 static irqreturn_t serio_raw_interrupt(struct serio *serio, unsigned char data,
@@ -278,14 +280,16 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 	struct serio_raw *serio_raw;
 	int err;
 
-	if (!(serio_raw = kzalloc(sizeof(struct serio_raw), GFP_KERNEL))) {
+	serio_raw = kzalloc(sizeof(struct serio_raw), GFP_KERNEL);
+	if (!serio_raw) {
 		dev_dbg(&serio->dev, "can't allocate memory for a device\n");
 		return -ENOMEM;
 	}
 
 	mutex_lock(&serio_raw_mutex);
 
-	snprintf(serio_raw->name, sizeof(serio_raw->name), "serio_raw%d", serio_raw_no++);
+	snprintf(serio_raw->name, sizeof(serio_raw->name),
+		 "serio_raw%d", serio_raw_no++);
 	kref_init(&serio_raw->kref);
 	serio_raw->serio = serio;
 	INIT_LIST_HEAD(&serio_raw->client_list);
-- 
1.7.6.4

[PATCH 7/9] Input: serio_raw - explicitly mark disconnected ports as dead

[Dmitry Torokhov]

Instead of relying on setting serio_raw->serio to NULL upon disconnecting
ports mark them explicitly as "dead". Also take and carry reference to
underlying serio port to make sure it does not go away until we are done
with it.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 64fcefb..30ff963 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -40,6 +40,7 @@ struct serio_raw {
 	wait_queue_head_t wait;
 	struct list_head client_list;
 	struct list_head node;
+	bool dead;
 };
 
 struct serio_raw_client {
@@ -91,7 +92,7 @@ static int serio_raw_open(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	if (!serio_raw->serio) {
+	if (serio_raw->dead) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -123,6 +124,8 @@ static void serio_raw_cleanup(struct kref *kref)
 
 	misc_deregister(&serio_raw->dev);
 	list_del_init(&serio_raw->node);
+
+	put_device(&serio_raw->serio->dev);
 	kfree(serio_raw);
 }
 
@@ -164,19 +167,18 @@ static ssize_t serio_raw_read(struct file *file, char __user *buffer,
 	char uninitialized_var(c);
 	ssize_t retval = 0;
 
-	if (!serio_raw->serio)
+	if (serio_raw->dead)
 		return -ENODEV;
 
 	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->serio);
+			serio_raw->head != serio_raw->tail || serio_raw->dead);
 	if (retval)
 		return retval;
 
-	if (!serio_raw->serio)
+	if (serio_raw->dead)
 		return -ENODEV;
 
 	while (retval < count && serio_raw_fetch_byte(serio_raw, &c)) {
@@ -201,7 +203,7 @@ static ssize_t serio_raw_write(struct file *file, const char __user *buffer,
 	if (retval)
 		return retval;
 
-	if (!serio_raw->serio) {
+	if (serio_raw->dead) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -291,10 +293,12 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 	snprintf(serio_raw->name, sizeof(serio_raw->name),
 		 "serio_raw%d", serio_raw_no++);
 	kref_init(&serio_raw->kref);
-	serio_raw->serio = serio;
 	INIT_LIST_HEAD(&serio_raw->client_list);
 	init_waitqueue_head(&serio_raw->wait);
 
+	serio_raw->serio = serio;
+	get_device(&serio->dev);
+
 	serio_set_drvdata(serio, serio_raw);
 
 	err = serio_open(serio, drv);
@@ -330,6 +334,7 @@ out_close:
 	list_del_init(&serio_raw->node);
 out_free:
 	serio_set_drvdata(serio, NULL);
+	put_device(&serio->dev);
 	kfree(serio_raw);
 out:
 	mutex_unlock(&serio_raw_mutex);
@@ -365,7 +370,7 @@ static void serio_raw_disconnect(struct serio *serio)
 	serio_close(serio);
 	serio_set_drvdata(serio, NULL);
 
-	serio_raw->serio = NULL;
+	serio_raw->dead = true;
 	wake_up_interruptible(&serio_raw->wait);
 	kref_put(&serio_raw->kref, serio_raw_cleanup);
 
-- 
1.7.6.4

[PATCH 8/9] Input: serio_raw - kick clients when disconnecting port

[Dmitry Torokhov]

Send SIGIO/POLL_HUP and otherwise wake up waiters when corresponding serio
port is being disconnected. Also check if port is dead in serio_raw_poll
and signal POLLHUP|POLLERR.

This should speed up process of releasing dead devices by userspace
applications.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 30ff963..830e2fe 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -232,9 +232,11 @@ static unsigned int serio_raw_poll(struct file *file, poll_table *wait)
 {
 	struct serio_raw_client *client = file->private_data;
 	struct serio_raw *serio_raw = client->serio_raw;
+	unsigned int mask;
 
 	poll_wait(file, &serio_raw->wait, wait);
 
+	mask = serio_raw->dead ? POLLHUP | POLLERR : POLLOUT | POLLWRNORM;
 	if (serio_raw->head != serio_raw->tail)
 		return POLLIN | POLLRDNORM;
 
@@ -359,22 +361,37 @@ static int serio_raw_reconnect(struct serio *serio)
 	return 0;
 }
 
+/*
+ * Wake up users waiting for IO so they can disconnect from
+ * dead device.
+ */
+static void serio_raw_hangup(struct serio_raw *serio_raw)
+{
+	struct serio_raw_client *client;
+
+	serio_pause_rx(serio_raw->serio);
+	list_for_each_entry(client, &serio_raw->client_list, node)
+		kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+	serio_continue_rx(serio_raw->serio);
+
+	wake_up_interruptible(&serio_raw->wait);
+}
+
+
 static void serio_raw_disconnect(struct serio *serio)
 {
-	struct serio_raw *serio_raw;
+	struct serio_raw *serio_raw = serio_get_drvdata(serio);
 
 	mutex_lock(&serio_raw_mutex);
 
-	serio_raw = serio_get_drvdata(serio);
-
 	serio_close(serio);
-	serio_set_drvdata(serio, NULL);
-
 	serio_raw->dead = true;
-	wake_up_interruptible(&serio_raw->wait);
+	serio_raw_hangup(serio_raw);
 	kref_put(&serio_raw->kref, serio_raw_cleanup);
 
 	mutex_unlock(&serio_raw_mutex);
+
+	serio_set_drvdata(serio, NULL);
 }
 
 static struct serio_device_id serio_raw_serio_ids[] = {
-- 
1.7.6.4

[PATCH 9/9] Input: serio_raw - fix memory leak when closing char device

[Dmitry Torokhov]

Apparently we never freed memory allocated when users open our char
devices nor removed old users from the list of connected clients.

Also unregister misc device immediately upon disconnecting the port
instead of waiting until last user drops off (refcounting in misc
device code will make sure needed pieces stay around while they
are needed) and make sure we are not holing holding serio_raw_mutex
when registering/unregistering misc device. This should fix potential
deadlock between serio_raw and misc device code uncovered by lockdep
and reported by Thomas Tuttle.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/serio_raw.c |   54 +++++++++++++++++++++------------------
 1 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
index 830e2fe..4d4cd14 100644
--- a/drivers/input/serio/serio_raw.c
+++ b/drivers/input/serio/serio_raw.c
@@ -51,7 +51,6 @@ struct serio_raw_client {
 
 static DEFINE_MUTEX(serio_raw_mutex);
 static LIST_HEAD(serio_raw_list);
-static unsigned int serio_raw_no;
 
 /*********************************************************************
  *             Interface with userspace (file operations)            *
@@ -117,14 +116,11 @@ out:
 	return retval;
 }
 
-static void serio_raw_cleanup(struct kref *kref)
+static void serio_raw_free(struct kref *kref)
 {
 	struct serio_raw *serio_raw =
 			container_of(kref, struct serio_raw, kref);
 
-	misc_deregister(&serio_raw->dev);
-	list_del_init(&serio_raw->node);
-
 	put_device(&serio_raw->serio->dev);
 	kfree(serio_raw);
 }
@@ -134,11 +130,14 @@ static int serio_raw_release(struct inode *inode, struct file *file)
 	struct serio_raw_client *client = file->private_data;
 	struct serio_raw *serio_raw = client->serio_raw;
 
-	mutex_lock(&serio_raw_mutex);
+	serio_pause_rx(serio_raw->serio);
+	list_del(&client->node);
+	serio_continue_rx(serio_raw->serio);
 
-	kref_put(&serio_raw->kref, serio_raw_cleanup);
+	kfree(client);
+
+	kref_put(&serio_raw->kref, serio_raw_free);
 
-	mutex_unlock(&serio_raw_mutex);
 	return 0;
 }
 
@@ -281,6 +280,7 @@ static irqreturn_t serio_raw_interrupt(struct serio *serio, unsigned char data,
 
 static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 {
+	static atomic_t serio_raw_no = ATOMIC_INIT(0);
 	struct serio_raw *serio_raw;
 	int err;
 
@@ -290,10 +290,8 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 		return -ENOMEM;
 	}
 
-	mutex_lock(&serio_raw_mutex);
-
 	snprintf(serio_raw->name, sizeof(serio_raw->name),
-		 "serio_raw%d", serio_raw_no++);
+		 "serio_raw%ld", (long)atomic_inc_return(&serio_raw_no) - 1);
 	kref_init(&serio_raw->kref);
 	INIT_LIST_HEAD(&serio_raw->client_list);
 	init_waitqueue_head(&serio_raw->wait);
@@ -305,9 +303,14 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 
 	err = serio_open(serio, drv);
 	if (err)
-		goto out_free;
+		goto err_free;
+
+	err = mutex_lock_killable(&serio_raw_mutex);
+	if (err)
+		goto err_close;
 
 	list_add_tail(&serio_raw->node, &serio_raw_list);
+	mutex_unlock(&serio_raw_mutex);
 
 	serio_raw->dev.minor = PSMOUSE_MINOR;
 	serio_raw->dev.name = serio_raw->name;
@@ -324,22 +327,20 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
 		dev_err(&serio->dev,
 			"failed to register raw access device for %s\n",
 			serio->phys);
-		goto out_close;
+		goto err_unlink;
 	}
 
 	dev_info(&serio->dev, "raw access enabled on %s (%s, minor %d)\n",
 		 serio->phys, serio_raw->name, serio_raw->dev.minor);
-	goto out;
+	return 0;
 
-out_close:
-	serio_close(serio);
+err_unlink:
 	list_del_init(&serio_raw->node);
-out_free:
+err_close:
+	serio_close(serio);
+err_free:
 	serio_set_drvdata(serio, NULL);
-	put_device(&serio->dev);
-	kfree(serio_raw);
-out:
-	mutex_unlock(&serio_raw_mutex);
+	kref_put(&serio_raw->kref, serio_raw_free);
 	return err;
 }
 
@@ -382,14 +383,17 @@ static void serio_raw_disconnect(struct serio *serio)
 {
 	struct serio_raw *serio_raw = serio_get_drvdata(serio);
 
-	mutex_lock(&serio_raw_mutex);
+	misc_deregister(&serio_raw->dev);
 
-	serio_close(serio);
+	mutex_lock(&serio_raw_mutex);
 	serio_raw->dead = true;
+	list_del_init(&serio_raw->node);
+	mutex_unlock(&serio_raw_mutex);
+
 	serio_raw_hangup(serio_raw);
-	kref_put(&serio_raw->kref, serio_raw_cleanup);
 
-	mutex_unlock(&serio_raw_mutex);
+	serio_close(serio);
+	kref_put(&serio_raw->kref, serio_raw_free);
 
 	serio_set_drvdata(serio, NULL);
 }
-- 
1.7.6.4

Re: [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting

[Wanlong Gao]

On 10/06/2011 01:08 PM, Dmitry Torokhov wrote:

> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>

All the 9 patches.

Thanks

> ---
>  drivers/input/serio/serio_raw.c |   28 +++++++++++++---------------
>  1 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
> index b7ba459..ef3a69c 100644
> --- a/drivers/input/serio/serio_raw.c
> +++ b/drivers/input/serio/serio_raw.c
> @@ -9,6 +9,7 @@
>   * the Free Software Foundation.
>   */
>  
> +#include <linux/kref.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/poll.h>
> @@ -33,7 +34,7 @@ struct serio_raw {
>  	unsigned int tail, head;
>  
>  	char name[16];
> -	unsigned int refcnt;
> +	struct kref kref;
>  	struct serio *serio;
>  	struct miscdevice dev;
>  	wait_queue_head_t wait;
> @@ -104,7 +105,7 @@ static int serio_raw_open(struct inode *inode, struct file *file)
>  	list->serio_raw = serio_raw;
>  	file->private_data = list;
>  
> -	serio_raw->refcnt++;
> +	kref_get(&serio_raw->kref);
>  	list_add_tail(&list->node, &serio_raw->list);
>  
>  out:
> @@ -112,17 +113,14 @@ out:
>  	return retval;
>  }
>  
> -static int serio_raw_cleanup(struct serio_raw *serio_raw)
> +static void serio_raw_cleanup(struct kref *kref)
>  {
> -	if (--serio_raw->refcnt == 0) {
> -		misc_deregister(&serio_raw->dev);
> -		list_del_init(&serio_raw->node);
> -		kfree(serio_raw);
> +	struct serio_raw *serio_raw =
> +			container_of(kref, struct serio_raw, kref);
>  
> -		return 1;
> -	}
> -
> -	return 0;
> +	misc_deregister(&serio_raw->dev);
> +	list_del_init(&serio_raw->node);
> +	kfree(serio_raw);
>  }
>  
>  static int serio_raw_release(struct inode *inode, struct file *file)
> @@ -132,7 +130,7 @@ static int serio_raw_release(struct inode *inode, struct file *file)
>  
>  	mutex_lock(&serio_raw_mutex);
>  
> -	serio_raw_cleanup(serio_raw);
> +	kref_put(&serio_raw->kref, serio_raw_cleanup);
>  
>  	mutex_unlock(&serio_raw_mutex);
>  	return 0;
> @@ -283,7 +281,7 @@ static int serio_raw_connect(struct serio *serio, struct serio_driver *drv)
>  	mutex_lock(&serio_raw_mutex);
>  
>  	snprintf(serio_raw->name, sizeof(serio_raw->name), "serio_raw%d", serio_raw_no++);
> -	serio_raw->refcnt = 1;
> +	kref_init(&serio_raw->kref);
>  	serio_raw->serio = serio;
>  	INIT_LIST_HEAD(&serio_raw->list);
>  	init_waitqueue_head(&serio_raw->wait);
> @@ -357,8 +355,8 @@ static void serio_raw_disconnect(struct serio *serio)
>  	serio_set_drvdata(serio, NULL);
>  
>  	serio_raw->serio = NULL;
> -	if (!serio_raw_cleanup(serio_raw))
> -		wake_up_interruptible(&serio_raw->wait);
> +	wake_up_interruptible(&serio_raw->wait);
> +	kref_put(&serio_raw->kref, serio_raw_cleanup);
>  
>  	mutex_unlock(&serio_raw_mutex);
>  }