Message-ID: <4984ec6a.0637560a.3217.4534@mx.google.com>
Date:	Sat, 31 Jan 2009 16:27:22 -0800 (PST)
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	dmitry.torokhov@...il.com, dtor@...l.ru,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
	<fweisbec@...il.com>
Subject: [PATCH v2] psmouse: run kpsmoused only while needed

While looking at the workqueue tracer, I noticed that kpsmoused receives
rarely (if not never) events.

Currently, when a mouse has to resync, it uses the kpsmoused singlethreaded
workqueue. But resync are rare. While reading an old discussion, it seems
that usual workqueue events can't be used for that purpose because resync
can take too much time and could delay the other works in queue.

But if you have built psmouse driver, this workqueue will always be present
whether you have a ps/2 port or not. And its events are rare.

To avoid this pointless task, this patch makes the kpsmoused a kernel
thread only created on the fly when a resync is needed. Once the resync is done,
this thread will die. So you will almost never see it, and it will not be
an inactive task anymore.

This thread is created through a usual workqueue event (because we can't create
it from interrupt).

Changes in V2:

_ fix the "resync" mispelled in the patch and the changelog
_ don't schedule more than one resync in case of concurrent interrupts
_ if resync_pending is not cleared after a few time before disconnect, print a warning.

Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
---
 drivers/input/mouse/psmouse-base.c |   86 ++++++++++++++++++++++++------------
 drivers/input/mouse/psmouse.h      |    5 ++-
 2 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index f8f86de..fc4490e 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/libps2.h>
 #include <linux/mutex.h>
+#include <linux/kthread.h>
 
 #include "psmouse.h"
 #include "synaptics.h"
@@ -104,8 +105,6 @@ static struct attribute_group psmouse_attribute_group = {
  */
 static DEFINE_MUTEX(psmouse_mutex);
 
-static struct workqueue_struct *kpsmoused_wq;
-
 struct psmouse_protocol {
 	enum psmouse_type type;
 	const char *name;
@@ -203,11 +202,6 @@ static psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 	return PSMOUSE_FULL_PACKET;
 }
 
-void psmouse_queue_work(struct psmouse *psmouse, struct delayed_work *work,
-		unsigned long delay)
-{
-	queue_delayed_work(kpsmoused_wq, work, delay);
-}
 
 /*
  * __psmouse_set_state() sets new psmouse state and resets all flags.
@@ -313,7 +307,10 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
 		       psmouse->name, psmouse->phys, psmouse->pktcnt);
 		psmouse->badbyte = psmouse->packet[0];
 		__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
-		psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
+		if (atomic_inc_return(&psmouse->resync_pending) == 1)
+			schedule_work(&psmouse->resync_work);
+		else
+			atomic_dec(&psmouse->resync_pending);
 		goto out;
 	}
 
@@ -350,7 +347,11 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
 	    time_after(jiffies, psmouse->last + psmouse->resync_time * HZ)) {
 		psmouse->badbyte = psmouse->packet[0];
 		__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
-		psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
+
+		if (atomic_inc_return(&psmouse->resync_pending) == 1)
+			schedule_work(&psmouse->resync_work);
+		else
+			atomic_dec(&psmouse->resync_pending);
 		goto out;
 	}
 
@@ -979,13 +980,16 @@ static int psmouse_poll(struct psmouse *psmouse)
 
 
 /*
- * psmouse_resync() attempts to re-validate current protocol.
+ * psmouse_resync_thread() attempts to re-validate current protocol.
+ * This thread is created on the fly when needed because its job can take too
+ * much time on events workqueues, and the resync is rare enough to avoid
+ * the need of a private workqueue.
  */
 
-static void psmouse_resync(struct work_struct *work)
+static int psmouse_resync_thread(void *v)
 {
-	struct psmouse *parent = NULL, *psmouse =
-		container_of(work, struct psmouse, resync_work.work);
+	struct psmouse *psmouse = v;
+	struct psmouse *parent = NULL;
 	struct serio *serio = psmouse->ps2dev.serio;
 	psmouse_ret_t rc = PSMOUSE_GOOD_DATA;
 	int failed = 0, enabled = 0;
@@ -1072,6 +1076,30 @@ static void psmouse_resync(struct work_struct *work)
 		psmouse_activate(parent);
  out:
 	mutex_unlock(&psmouse_mutex);
+
+	/*
+	 * While disconnecting, the driver wants to be sure all resync are done
+	 */
+	atomic_dec(&psmouse->resync_pending);
+	wake_up(&psmouse->resync_pending_queue);
+	return 0;
+}
+
+/* Launch the resync thread */
+static void psmouse_resync_thread_helper(struct work_struct *work)
+{
+	struct task_struct *t;
+	struct psmouse *psmouse;
+
+	psmouse = container_of(work, struct psmouse, resync_work);
+
+	t = kthread_run(psmouse_resync_thread, psmouse, "kpsmoused");
+	if (t == ERR_PTR(-ENOMEM)) {
+		printk(KERN_WARNING "psmouse.c: failed to create kpsmoused"
+				    " thread\n");
+		atomic_dec(&psmouse->resync_pending);
+		wake_up(&psmouse->resync_pending_queue);
+	}
 }
 
 /*
@@ -1120,6 +1148,8 @@ static void psmouse_cleanup(struct serio *serio)
 static void psmouse_disconnect(struct serio *serio)
 {
 	struct psmouse *psmouse, *parent = NULL;
+	long timeout;
+	DEFINE_WAIT(wait);
 
 	psmouse = serio_get_drvdata(serio);
 
@@ -1131,7 +1161,17 @@ static void psmouse_disconnect(struct serio *serio)
 
 	/* make sure we don't have a resync in progress */
 	mutex_unlock(&psmouse_mutex);
-	flush_workqueue(kpsmoused_wq);
+
+	prepare_to_wait(&psmouse->resync_pending_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+	/* 4 seconds are more than enough to ensure a resync is completed */
+	if (atomic_read(&psmouse->resync_pending)) {
+		timeout = schedule_timeout(HZ * 4);
+		if (!timeout)
+			WARN_ON(1);
+	}
+	finish_wait(&psmouse->resync_pending_queue, &wait);
+
 	mutex_lock(&psmouse_mutex);
 
 	if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
@@ -1244,7 +1284,8 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
 		goto err_free;
 
 	ps2_init(&psmouse->ps2dev, serio);
-	INIT_DELAYED_WORK(&psmouse->resync_work, psmouse_resync);
+	INIT_WORK(&psmouse->resync_work, psmouse_resync_thread_helper);
+	init_waitqueue_head(&psmouse->resync_pending_queue);
 	psmouse->dev = input_dev;
 	snprintf(psmouse->phys, sizeof(psmouse->phys), "%s/input0", serio->phys);
 
@@ -1647,25 +1688,12 @@ static int psmouse_get_maxproto(char *buffer, struct kernel_param *kp)
 
 static int __init psmouse_init(void)
 {
-	int err;
-
-	kpsmoused_wq = create_singlethread_workqueue("kpsmoused");
-	if (!kpsmoused_wq) {
-		printk(KERN_ERR "psmouse: failed to create kpsmoused workqueue\n");
-		return -ENOMEM;
-	}
-
-	err = serio_register_driver(&psmouse_drv);
-	if (err)
-		destroy_workqueue(kpsmoused_wq);
-
-	return err;
+	return serio_register_driver(&psmouse_drv);
 }
 
 static void __exit psmouse_exit(void)
 {
 	serio_unregister_driver(&psmouse_drv);
-	destroy_workqueue(kpsmoused_wq);
 }
 
 module_init(psmouse_init);
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 54ed267..dfb9b8c 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -39,7 +39,10 @@ struct psmouse {
 	void *private;
 	struct input_dev *dev;
 	struct ps2dev ps2dev;
-	struct delayed_work resync_work;
+	struct work_struct resync_work;
+	/* help to wait for mouse resync jobs before disconnect */
+	atomic_t resync_pending;
+	wait_queue_head_t resync_pending_queue;
 	char *vendor;
 	char *name;
 	unsigned char packet[8];
-- 
1.6.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists