* [PATCH 2/9] Input: serio_raw - rename serio_raw_list to serio_raw_client
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 3/9] Input: serio_raw - perform proper locking when adding clients to list Dmitry Torokhov
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
'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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/9] Input: serio_raw - perform proper locking when adding clients to list
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 2/9] Input: serio_raw - rename serio_raw_list to serio_raw_client Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 4/9] Input: serio_raw - use bool for boolean data Dmitry Torokhov
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/9] Input: serio_raw - use bool for boolean data
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 2/9] Input: serio_raw - rename serio_raw_list to serio_raw_client Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 3/9] Input: serio_raw - perform proper locking when adding clients to list Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 5/9] Input: serio_raw - use dev_*() for messages Dmitry Torokhov
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/9] Input: serio_raw - use dev_*() for messages
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
` (2 preceding siblings ...)
2011-10-06 5:08 ` [PATCH 4/9] Input: serio_raw - use bool for boolean data Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 6/9] Input: serio_raw - fix coding style issues Dmitry Torokhov
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/9] Input: serio_raw - fix coding style issues
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
` (3 preceding siblings ...)
2011-10-06 5:08 ` [PATCH 5/9] Input: serio_raw - use dev_*() for messages Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 7/9] Input: serio_raw - explicitly mark disconnected ports as dead Dmitry Torokhov
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/9] Input: serio_raw - explicitly mark disconnected ports as dead
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
` (4 preceding siblings ...)
2011-10-06 5:08 ` [PATCH 6/9] Input: serio_raw - fix coding style issues Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 8/9] Input: serio_raw - kick clients when disconnecting port Dmitry Torokhov
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 8/9] Input: serio_raw - kick clients when disconnecting port
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
` (5 preceding siblings ...)
2011-10-06 5:08 ` [PATCH 7/9] Input: serio_raw - explicitly mark disconnected ports as dead Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 5:08 ` [PATCH 9/9] Input: serio_raw - fix memory leak when closing char device Dmitry Torokhov
2011-10-06 6:05 ` [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Wanlong Gao
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 9/9] Input: serio_raw - fix memory leak when closing char device
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
` (6 preceding siblings ...)
2011-10-06 5:08 ` [PATCH 8/9] Input: serio_raw - kick clients when disconnecting port Dmitry Torokhov
@ 2011-10-06 5:08 ` Dmitry Torokhov
2011-10-06 6:05 ` [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Wanlong Gao
8 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-10-06 5:08 UTC (permalink / raw)
To: linux-input; +Cc: Thomas Tuttle
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting
2011-10-06 5:08 [PATCH 1/9] Input: serio-raw - use kref instead of rolling out its own refcounting Dmitry Torokhov
` (7 preceding siblings ...)
2011-10-06 5:08 ` [PATCH 9/9] Input: serio_raw - fix memory leak when closing char device Dmitry Torokhov
@ 2011-10-06 6:05 ` Wanlong Gao
8 siblings, 0 replies; 10+ messages in thread
From: Wanlong Gao @ 2011-10-06 6:05 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Thomas Tuttle
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);
> }
^ permalink raw reply [flat|nested] 10+ messages in thread