* [RFC/RFT 1/5] Input: implement proper locking in input core
2007-07-24 4:45 [RFC/RFT 0/5] Input locking patches Dmitry Torokhov
@ 2007-07-24 4:45 ` Dmitry Torokhov
2007-07-24 5:35 ` Jeff Garzik
2007-07-27 23:28 ` Indan Zupancic
2007-07-24 4:45 ` [RFC/RFT 2/5] evdev - implement proper locking Dmitry Torokhov
` (4 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-24 4:45 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
[-- Attachment #1: input-locking.patch --]
[-- Type: text/plain, Size: 33650 bytes --]
Input: implement proper locking in input core
Also add some kerneldoc documentation to input.h
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/input.c | 656 ++++++++++++++++++++++++++++++++++++--------------
include/linux/input.h | 112 +++++++-
2 files changed, 585 insertions(+), 183 deletions(-)
Index: work/drivers/input/input.c
===================================================================
--- work.orig/drivers/input/input.c
+++ work/drivers/input/input.c
@@ -17,10 +17,10 @@
#include <linux/major.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
-#include <linux/interrupt.h>
#include <linux/poll.h>
#include <linux/device.h>
#include <linux/mutex.h>
+#include <linux/rcupdate.h>
MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
MODULE_DESCRIPTION("Input core");
@@ -31,167 +31,242 @@ MODULE_LICENSE("GPL");
static LIST_HEAD(input_dev_list);
static LIST_HEAD(input_handler_list);
+/*
+ * input_mutex protects access to both input_dev_list and input_handler_list.
+ * This also causes input_[un]register_device and input_[un]register_handler
+ * be mutually exclusive which simplifies locking in drivers implementing
+ * input handlers.
+ */
+static DEFINE_MUTEX(input_mutex);
+
static struct input_handler *input_table[8];
-/**
- * input_event() - report new input event
- * @dev: device that generated the event
- * @type: type of the event
- * @code: event code
- * @value: value of the event
- *
- * This function should be used by drivers implementing various input devices
- * See also input_inject_event()
- */
-void input_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
+static inline int is_event_supported(unsigned int code,
+ unsigned long *bm, unsigned int max)
{
- struct input_handle *handle;
+ return code <= max && test_bit(code, bm);
+}
- if (type > EV_MAX || !test_bit(type, dev->evbit))
- return;
+static int input_defuzz_abs_event(int value, int old_val, int fuzz)
+{
+ if (fuzz) {
+ if (value > old_val - fuzz / 2 && value < old_val + fuzz / 2)
+ return value;
- add_input_randomness(type, code, value);
+ if (value > old_val - fuzz && value < old_val + fuzz)
+ return (old_val * 3 + value) / 4;
- switch (type) {
+ if (value > old_val - fuzz * 2 && value < old_val + fuzz * 2)
+ return (old_val + value) / 2;
+ }
- case EV_SYN:
- switch (code) {
- case SYN_CONFIG:
- if (dev->event)
- dev->event(dev, type, code, value);
- break;
-
- case SYN_REPORT:
- if (dev->sync)
- return;
- dev->sync = 1;
- break;
- }
- break;
+ return value;
+}
- case EV_KEY:
+/*
+ * Pass event through all open handles. This function is called with
+ * dev->event_lock held and interrupts disabled. Because of that we
+ * do not need to use rcu_read_lock() here although we are using RCU
+ * to access handle list.
+ */
+static void input_pass_event(struct input_dev *dev,
+ unsigned int type, unsigned int code, int value)
+{
+ struct input_handle *handle = rcu_dereference(dev->grab);
- if (code > KEY_MAX || !test_bit(code, dev->keybit) || !!test_bit(code, dev->key) == value)
- return;
+ if (handle)
+ handle->handler->event(handle, type, code, value);
+ else
+ list_for_each_entry_rcu(handle, &dev->h_list, d_node)
+ if (handle->open)
+ handle->handler->event(handle,
+ type, code, value);
+}
- if (value == 2)
- break;
+/*
+ * Generate software autorepeat event. Note that we take
+ * dev->event_lock here to avoid racing with input_event
+ * which may cause keys get "stuck".
+ */
+static void input_repeat_key(unsigned long data)
+{
+ struct input_dev *dev = (void *) data;
- change_bit(code, dev->key);
+ spin_lock_irq(&dev->event_lock);
- if (test_bit(EV_REP, dev->evbit) && dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] && dev->timer.data && value) {
- dev->repeat_key = code;
- mod_timer(&dev->timer, jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
- }
+ if (test_bit(dev->repeat_key, dev->key) &&
+ is_event_supported(dev->repeat_key, dev->keybit, KEY_MAX)) {
- break;
+ input_pass_event(dev, EV_KEY, dev->repeat_key, 2);
- case EV_SW:
+ if (dev->sync) {
+ /*
+ * Only send SYN_REPORT if we are not in a middle
+ * of driver parsing a new hardware packet.
+ * Otherwise assume that the driver will send
+ * SYN_REPORT once it's done.
+ */
+ input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+ }
- if (code > SW_MAX || !test_bit(code, dev->swbit) || !!test_bit(code, dev->sw) == value)
- return;
+ if (dev->rep[REP_PERIOD])
+ mod_timer(&dev->timer, jiffies +
+ msecs_to_jiffies(dev->rep[REP_PERIOD]));
+ }
- change_bit(code, dev->sw);
+ spin_unlock_irq(&dev->event_lock);
+}
- break;
+static void input_start_autorepeat(struct input_dev *dev, int code)
+{
+ if (test_bit(EV_REP, dev->evbit) &&
+ dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
+ dev->timer.data) {
+ dev->repeat_key = code;
+ mod_timer(&dev->timer,
+ jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
+ }
+}
- case EV_ABS:
+#define INPUT_IGNORE_EVENT 0
+#define INPUT_PASS_TO_HANDLERS 1
+#define INPUT_PASS_TO_DEVICE 2
+#define INPUT_PASS_TO_ALL (INPUT_PASS_TO_HANDLERS | INPUT_PASS_TO_DEVICE)
- if (code > ABS_MAX || !test_bit(code, dev->absbit))
- return;
+static void input_handle_event(struct input_dev *dev,
+ unsigned int type, unsigned int code, int value)
+{
+ int disposition = INPUT_IGNORE_EVENT;
- if (dev->absfuzz[code]) {
- if ((value > dev->abs[code] - (dev->absfuzz[code] >> 1)) &&
- (value < dev->abs[code] + (dev->absfuzz[code] >> 1)))
- return;
-
- if ((value > dev->abs[code] - dev->absfuzz[code]) &&
- (value < dev->abs[code] + dev->absfuzz[code]))
- value = (dev->abs[code] * 3 + value) >> 2;
-
- if ((value > dev->abs[code] - (dev->absfuzz[code] << 1)) &&
- (value < dev->abs[code] + (dev->absfuzz[code] << 1)))
- value = (dev->abs[code] + value) >> 1;
- }
+ switch (type) {
- if (dev->abs[code] == value)
- return;
+ case EV_SYN:
+ switch (code) {
+ case SYN_CONFIG:
+ disposition = INPUT_PASS_TO_ALL;
+ break;
- dev->abs[code] = value;
+ case SYN_REPORT:
+ if (!dev->sync) {
+ dev->sync = 1;
+ disposition = INPUT_PASS_TO_HANDLERS;
+ }
break;
+ }
+ break;
- case EV_REL:
+ case EV_KEY:
+ if (is_event_supported(code, dev->keybit, KEY_MAX) &&
+ !!test_bit(code, dev->key) != value) {
- if (code > REL_MAX || !test_bit(code, dev->relbit) || (value == 0))
- return;
+ if (value != 2) {
+ __change_bit(code, dev->key);
+ if (value)
+ input_start_autorepeat(dev, code);
+ }
- break;
+ disposition = INPUT_PASS_TO_HANDLERS;
+ }
+ break;
- case EV_MSC:
+ case EV_SW:
+ if (is_event_supported(code, dev->swbit, SW_MAX) &&
+ !!test_bit(code, dev->sw) != value) {
- if (code > MSC_MAX || !test_bit(code, dev->mscbit))
- return;
+ __change_bit(code, dev->sw);
+ disposition = INPUT_PASS_TO_HANDLERS;
+ }
+ break;
- if (dev->event)
- dev->event(dev, type, code, value);
+ case EV_ABS:
+ if (is_event_supported(code, dev->absbit, ABS_MAX)) {
- break;
+ value = input_defuzz_abs_event(value,
+ dev->abs[code], dev->absfuzz[code]);
+
+ if (dev->abs[code] != value) {
+ dev->abs[code] = value;
+ disposition = INPUT_PASS_TO_HANDLERS;
+ }
+ }
+ break;
- case EV_LED:
+ case EV_REL:
+ if (is_event_supported(code, dev->relbit, REL_MAX) && value)
+ disposition = INPUT_PASS_TO_HANDLERS;
- if (code > LED_MAX || !test_bit(code, dev->ledbit) || !!test_bit(code, dev->led) == value)
- return;
+ break;
- change_bit(code, dev->led);
+ case EV_MSC:
+ if (is_event_supported(code, dev->mscbit, MSC_MAX))
+ disposition = INPUT_PASS_TO_ALL;
- if (dev->event)
- dev->event(dev, type, code, value);
+ break;
- break;
+ case EV_LED:
+ if (is_event_supported(code, dev->ledbit, LED_MAX) &&
+ !!test_bit(code, dev->led) != value) {
- case EV_SND:
+ __change_bit(code, dev->led);
+ disposition = INPUT_PASS_TO_ALL;
+ }
+ break;
- if (code > SND_MAX || !test_bit(code, dev->sndbit))
- return;
+ case EV_SND:
+ if (is_event_supported(code, dev->sndbit, SND_MAX)) {
if (!!test_bit(code, dev->snd) != !!value)
- change_bit(code, dev->snd);
+ __change_bit(code, dev->snd);
+ disposition = INPUT_PASS_TO_ALL;
+ }
+ break;
- if (dev->event)
- dev->event(dev, type, code, value);
+ case EV_REP:
+ if (code <= REP_MAX && value >= 0 && dev->rep[code] != value) {
+ dev->rep[code] = value;
+ disposition = INPUT_PASS_TO_ALL;
+ }
+ break;
- break;
+ case EV_FF:
+ if (value >= 0)
+ disposition = INPUT_PASS_TO_ALL;
+ break;
+ }
- case EV_REP:
+ if (type != EV_SYN)
+ dev->sync = 0;
- if (code > REP_MAX || value < 0 || dev->rep[code] == value)
- return;
+ if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
+ dev->event(dev, type, code, value);
- dev->rep[code] = value;
- if (dev->event)
- dev->event(dev, type, code, value);
+ if (disposition & INPUT_PASS_TO_HANDLERS)
+ input_pass_event(dev, type, code, value);
+}
- break;
+/**
+ * input_event() - report new input event
+ * @dev: device that generated the event
+ * @type: type of the event
+ * @code: event code
+ * @value: value of the event
+ *
+ * This function should be used by drivers implementing various input
+ * devices. See also input_inject_event().
+ */
- case EV_FF:
+void input_event(struct input_dev *dev,
+ unsigned int type, unsigned int code, int value)
+{
+ unsigned long flags;
- if (value < 0)
- return;
+ if (is_event_supported(type, dev->evbit, EV_MAX)) {
- if (dev->event)
- dev->event(dev, type, code, value);
- break;
+ spin_lock_irqsave(&dev->event_lock, flags);
+ add_input_randomness(type, code, value);
+ input_handle_event(dev, type, code, value);
+ spin_unlock_irqrestore(&dev->event_lock, flags);
}
-
- if (type != EV_SYN)
- dev->sync = 0;
-
- if (dev->grab)
- dev->grab->handler->event(dev->grab, type, code, value);
- else
- list_for_each_entry(handle, &dev->h_list, d_node)
- if (handle->open)
- handle->handler->event(handle, type, code, value);
}
EXPORT_SYMBOL(input_event);
@@ -202,102 +277,222 @@ EXPORT_SYMBOL(input_event);
* @code: event code
* @value: value of the event
*
- * Similar to input_event() but will ignore event if device is "grabbed" and handle
- * injecting event is not the one that owns the device.
+ * Similar to input_event() but will ignore event if device is
+ * "grabbed" and handle injecting event is not the one that owns
+ * the device.
*/
-void input_inject_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
-{
- if (!handle->dev->grab || handle->dev->grab == handle)
- input_event(handle->dev, type, code, value);
-}
-EXPORT_SYMBOL(input_inject_event);
-
-static void input_repeat_key(unsigned long data)
+void input_inject_event(struct input_handle *handle,
+ unsigned int type, unsigned int code, int value)
{
- struct input_dev *dev = (void *) data;
+ struct input_dev *dev = handle->dev;
+ struct input_handle *grab;
- if (!test_bit(dev->repeat_key, dev->key))
- return;
+ if (is_event_supported(type, dev->evbit, EV_MAX)) {
+ spin_lock_irq(&dev->event_lock);
- input_event(dev, EV_KEY, dev->repeat_key, 2);
- input_sync(dev);
+ grab = rcu_dereference(dev->grab);
+ if (!grab || grab == handle)
+ input_handle_event(dev, type, code, value);
- if (dev->rep[REP_PERIOD])
- mod_timer(&dev->timer, jiffies + msecs_to_jiffies(dev->rep[REP_PERIOD]));
+ spin_unlock_irq(&dev->event_lock);
+ }
}
+EXPORT_SYMBOL(input_inject_event);
+/**
+ * input_grab_device - grabs device for exclusive use
+ * @handle: input handle that wants to own the device
+ *
+ * When a device is grabbed by an input handle all events generated by
+ * the device are delivered only to this handle. Also events injected
+ * by other input handles are ignored while device is grabbed.
+ */
int input_grab_device(struct input_handle *handle)
{
- if (handle->dev->grab)
- return -EBUSY;
+ struct input_dev *dev = handle->dev;
+ int retval;
- handle->dev->grab = handle;
- return 0;
+ retval = mutex_lock_interruptible(&dev->mutex);
+ if (retval)
+ return retval;
+
+ if (dev->grab) {
+ retval = -EBUSY;
+ goto out;
+ }
+
+ rcu_assign_pointer(dev->grab, handle);
+ synchronize_sched();
+
+ out:
+ mutex_unlock(&dev->mutex);
+ return retval;
}
EXPORT_SYMBOL(input_grab_device);
-void input_release_device(struct input_handle *handle)
+static void __input_release_device(struct input_handle *handle)
{
struct input_dev *dev = handle->dev;
if (dev->grab == handle) {
- dev->grab = NULL;
+ rcu_assign_pointer(dev->grab, NULL);
+ /* Make sure input_pass_event() notices that grab is gone */
+ synchronize_sched();
list_for_each_entry(handle, &dev->h_list, d_node)
- if (handle->handler->start)
+ if (handle->open && handle->handler->start)
handle->handler->start(handle);
}
}
+
+/**
+ * input_release_device - release previously grabbed device
+ * @handle: input handle that owns the device
+ *
+ * Releases previously grabbed device so that other input handles can
+ * start receiving input events. Upon release all handlers attached
+ * to the device have their start() method called so they have a change
+ * to synchronize device state with the rest of the system.
+ */
+void input_release_device(struct input_handle *handle)
+{
+ struct input_dev *dev = handle->dev;
+
+ mutex_lock(&dev->mutex);
+ __input_release_device(handle);
+ mutex_unlock(&dev->mutex);
+}
EXPORT_SYMBOL(input_release_device);
+/**
+ * input_open_device - open input device
+ * @handle: handle through which device is being accessed
+ *
+ * This function should be called by input handlers when they
+ * want to start receive events from given input device.
+ */
int input_open_device(struct input_handle *handle)
{
struct input_dev *dev = handle->dev;
- int err;
+ int retval;
- err = mutex_lock_interruptible(&dev->mutex);
- if (err)
- return err;
+ retval = mutex_lock_interruptible(&dev->mutex);
+ if (retval)
+ return retval;
+
+ if (dev->going_away) {
+ retval = -ENODEV;
+ goto out;
+ }
handle->open++;
if (!dev->users++ && dev->open)
- err = dev->open(dev);
+ retval = dev->open(dev);
- if (err)
- handle->open--;
+ if (retval && !--handle->open) {
+ /*
+ * Make sure we are not delivering any more events
+ * through this handle
+ */
+ synchronize_sched();
+ }
+ out:
mutex_unlock(&dev->mutex);
-
- return err;
+ return retval;
}
EXPORT_SYMBOL(input_open_device);
-int input_flush_device(struct input_handle* handle, struct file* file)
+int input_flush_device(struct input_handle *handle, struct file *file)
{
- if (handle->dev->flush)
- return handle->dev->flush(handle->dev, file);
+ struct input_dev *dev = handle->dev;
+ int retval;
- return 0;
+ retval = mutex_lock_interruptible(&dev->mutex);
+ if (retval)
+ return retval;
+
+ if (dev->flush)
+ retval = dev->flush(dev, file);
+
+ mutex_unlock(&dev->mutex);
+ return retval;
}
EXPORT_SYMBOL(input_flush_device);
+/**
+ * input_close_device - close input device
+ * @handle: handle through which device is being accessed
+ *
+ * This function should be called by input handlers when they
+ * want to stop receive events from given input device.
+ */
void input_close_device(struct input_handle *handle)
{
struct input_dev *dev = handle->dev;
- input_release_device(handle);
-
mutex_lock(&dev->mutex);
+ __input_release_device(handle);
+
if (!--dev->users && dev->close)
dev->close(dev);
- handle->open--;
+
+ if (!--handle->open) {
+ /*
+ * synchronize_sched() makes sure that input_pass_event()
+ * completed and that no more input events are delivered
+ * through this handle
+ */
+ synchronize_sched();
+ }
mutex_unlock(&dev->mutex);
}
EXPORT_SYMBOL(input_close_device);
+/*
+ * Prepare device for unregistering
+ */
+static void input_disconnect_device(struct input_dev *dev)
+{
+ struct input_handle *handle;
+ int code;
+
+ /*
+ * Mark device as going away. Note that we take dev->mutex here
+ * not to protect access to dev->going_away but rather to ensure
+ * that there are no threads in the middle of input_open_device()
+ */
+ mutex_lock(&dev->mutex);
+ dev->going_away = 1;
+ mutex_unlock(&dev->mutex);
+
+ spin_lock_irq(&dev->event_lock);
+
+ /*
+ * Simulate keyup events for all pressed keys so that handlers
+ * are not left with "stuck" keys. The driver may continue
+ * generate events even after we done here but they will not
+ * reach any handlers.
+ */
+ if (is_event_supported(EV_KEY, dev->evbit, EV_MAX)) {
+ for (code = 0; code <= KEY_MAX; code++) {
+ if (is_event_supported(code, dev->keybit, KEY_MAX) &&
+ test_bit(code, dev->key)) {
+ input_pass_event(dev, EV_KEY, code, 0);
+ }
+ }
+ input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+ }
+
+ list_for_each_entry(handle, &dev->h_list, d_node)
+ handle->open = 0;
+
+ spin_unlock_irq(&dev->event_lock);
+}
+
static int input_fetch_keycode(struct input_dev *dev, int scancode)
{
switch (dev->keycodesize) {
@@ -473,7 +668,8 @@ static unsigned int input_proc_devices_p
static void *input_devices_seq_start(struct seq_file *seq, loff_t *pos)
{
- /* acquire lock here ... Yes, we do need locking, I knowi, I know... */
+ if (mutex_lock_interruptible(&input_mutex))
+ return NULL;
return seq_list_start(&input_dev_list, *pos);
}
@@ -485,7 +681,7 @@ static void *input_devices_seq_next(stru
static void input_devices_seq_stop(struct seq_file *seq, void *v)
{
- /* release lock here */
+ mutex_unlock(&input_mutex);
}
static void input_seq_print_bitmap(struct seq_file *seq, const char *name,
@@ -569,7 +765,9 @@ static const struct file_operations inpu
static void *input_handlers_seq_start(struct seq_file *seq, loff_t *pos)
{
- /* acquire lock here ... Yes, we do need locking, I knowi, I know... */
+ if (mutex_lock_interruptible(&input_mutex))
+ return NULL;
+
seq->private = (void *)(unsigned long)*pos;
return seq_list_start(&input_handler_list, *pos);
}
@@ -582,7 +780,7 @@ static void *input_handlers_seq_next(str
static void input_handlers_seq_stop(struct seq_file *seq, void *v)
{
- /* release lock here */
+ mutex_unlock(&input_mutex);
}
static int input_handlers_seq_show(struct seq_file *seq, void *v)
@@ -1005,6 +1203,7 @@ struct input_dev *input_allocate_device(
dev->dev.class = &input_class;
device_initialize(&dev->dev);
mutex_init(&dev->mutex);
+ spin_lock_init(&dev->event_lock);
INIT_LIST_HEAD(&dev->h_list);
INIT_LIST_HEAD(&dev->node);
@@ -1022,7 +1221,7 @@ EXPORT_SYMBOL(input_allocate_device);
* This function should only be used if input_register_device()
* was not called yet or if it failed. Once device was registered
* use input_unregister_device() and memory will be freed once last
- * refrence to the device is dropped.
+ * reference to the device is dropped.
*
* Device should be allocated by input_allocate_device().
*
@@ -1092,6 +1291,18 @@ void input_set_capability(struct input_d
}
EXPORT_SYMBOL(input_set_capability);
+/**
+ * input_register_device - register device with input core
+ * @dev: device to be registered
+ *
+ * This function registers device with input core. The device must be
+ * allocated with input_allocate_device() and all it's capabilities
+ * set up before registering.
+ * If function fails the device must be freed with input_free_device().
+ * Once device has been successfully registered it can be unregistered
+ * with input_unregister_device(); input_free_device() should not be
+ * called in this case.
+ */
int input_register_device(struct input_dev *dev)
{
static atomic_t input_no = ATOMIC_INIT(0);
@@ -1099,7 +1310,7 @@ int input_register_device(struct input_d
const char *path;
int error;
- set_bit(EV_SYN, dev->evbit);
+ __set_bit(EV_SYN, dev->evbit);
/*
* If delay and period are pre-set by the driver, then autorepeating
@@ -1120,8 +1331,6 @@ int input_register_device(struct input_d
if (!dev->setkeycode)
dev->setkeycode = input_default_setkeycode;
- list_add_tail(&dev->node, &input_dev_list);
-
snprintf(dev->dev.bus_id, sizeof(dev->dev.bus_id),
"input%ld", (unsigned long) atomic_inc_return(&input_no) - 1);
@@ -1137,49 +1346,79 @@ int input_register_device(struct input_d
dev->name ? dev->name : "Unspecified device", path ? path : "N/A");
kfree(path);
+ error = mutex_lock_interruptible(&input_mutex);
+ if (error) {
+ device_del(&dev->dev);
+ return error;
+ }
+
+ list_add_tail(&dev->node, &input_dev_list);
+
list_for_each_entry(handler, &input_handler_list, node)
input_attach_handler(dev, handler);
input_wakeup_procfs_readers();
+ mutex_unlock(&input_mutex);
+
return 0;
}
EXPORT_SYMBOL(input_register_device);
+/**
+ * input_unregister_device - unregister previously registered device
+ * @dev: device to be unregistered
+ *
+ * This function unregisters an input device. Once device is unregistered
+ * the caller should not try to access it as it may get freed at any moment.
+ */
void input_unregister_device(struct input_dev *dev)
{
struct input_handle *handle, *next;
- int code;
- for (code = 0; code <= KEY_MAX; code++)
- if (test_bit(code, dev->key))
- input_report_key(dev, code, 0);
- input_sync(dev);
+ input_disconnect_device(dev);
- del_timer_sync(&dev->timer);
+ mutex_lock(&input_mutex);
list_for_each_entry_safe(handle, next, &dev->h_list, d_node)
handle->handler->disconnect(handle);
WARN_ON(!list_empty(&dev->h_list));
+ del_timer_sync(&dev->timer);
list_del_init(&dev->node);
- device_unregister(&dev->dev);
-
input_wakeup_procfs_readers();
+
+ mutex_unlock(&input_mutex);
+
+ device_unregister(&dev->dev);
}
EXPORT_SYMBOL(input_unregister_device);
+/**
+ * input_register_handler - register a new input handler
+ * @handler: handler to be registered
+ *
+ * This function registers a new input handler (interface) for input
+ * devices in the system and attaches it to all input devices that
+ * are compatible with the handler.
+ */
int input_register_handler(struct input_handler *handler)
{
struct input_dev *dev;
+ int retval;
+
+ retval = mutex_lock_interruptible(&input_mutex);
+ if (retval)
+ return retval;
INIT_LIST_HEAD(&handler->h_list);
if (handler->fops != NULL) {
- if (input_table[handler->minor >> 5])
- return -EBUSY;
-
+ if (input_table[handler->minor >> 5]) {
+ retval = -EBUSY;
+ goto out;
+ }
input_table[handler->minor >> 5] = handler;
}
@@ -1189,14 +1428,26 @@ int input_register_handler(struct input_
input_attach_handler(dev, handler);
input_wakeup_procfs_readers();
- return 0;
+
+ out:
+ mutex_unlock(&input_mutex);
+ return retval;
}
EXPORT_SYMBOL(input_register_handler);
+/**
+ * input_unregister_handler - unregisters an input handler
+ * @handler: handler to be unregistered
+ *
+ * This function disconnects a handler from its input devices and
+ * removes it from lists of known handlers.
+ */
void input_unregister_handler(struct input_handler *handler)
{
struct input_handle *handle, *next;
+ mutex_lock(&input_mutex);
+
list_for_each_entry_safe(handle, next, &handler->h_list, h_node)
handler->disconnect(handle);
WARN_ON(!list_empty(&handler->h_list));
@@ -1207,14 +1458,50 @@ void input_unregister_handler(struct inp
input_table[handler->minor >> 5] = NULL;
input_wakeup_procfs_readers();
+
+ mutex_unlock(&input_mutex);
}
EXPORT_SYMBOL(input_unregister_handler);
+/**
+ * input_register_handle - register a new input handle
+ * @handle: handle to register
+ *
+ * This function puts a new input handle onto device's
+ * and handler's lists so that events can flow through
+ * it once it is opened using input_open_device().
+ *
+ * This function is supposed to be called from handler's
+ * connect() method.
+ */
int input_register_handle(struct input_handle *handle)
{
struct input_handler *handler = handle->handler;
+ struct input_dev *dev = handle->dev;
+ int error;
+
+ /*
+ * We take dev->mutex here to prevent race with
+ * input_release_device().
+ */
+ error = mutex_lock_interruptible(&dev->mutex);
+ if (error)
+ return error;
+ list_add_tail_rcu(&handle->d_node, &dev->h_list);
+ mutex_unlock(&dev->mutex);
+ /*
+ * We don't use synchronize_rcu() here because we rely
+ * on dev->event_lock to protect read-side critical
+ * section in input_pass_event().
+ */
+ synchronize_sched();
- list_add_tail(&handle->d_node, &handle->dev->h_list);
+ /*
+ * Since we are supposed to be called from ->connect()
+ * which is mutually exclusive with ->disconnect()
+ * we can't be racing with input_unregister_handle()
+ * and so separate lock is not needed here.
+ */
list_add_tail(&handle->h_node, &handler->h_list);
if (handler->start)
@@ -1224,10 +1511,29 @@ int input_register_handle(struct input_h
}
EXPORT_SYMBOL(input_register_handle);
+/**
+ * input_unregister_handle - unregister an input handle
+ * @handle: handle to unregister
+ *
+ * This function removes input handle from device's
+ * and handler's lists.
+ *
+ * This function is supposed to be called from handler's
+ * disconnect() method.
+ */
void input_unregister_handle(struct input_handle *handle)
{
+ struct input_dev *dev = handle->dev;
+
list_del_init(&handle->h_node);
- list_del_init(&handle->d_node);
+
+ /*
+ * Take dev->mutex to prevent race with input_release_device().
+ */
+ mutex_lock(&dev->mutex);
+ list_del_rcu(&handle->d_node);
+ mutex_unlock(&dev->mutex);
+ synchronize_sched();
}
EXPORT_SYMBOL(input_unregister_handle);
Index: work/include/linux/input.h
===================================================================
--- work.orig/include/linux/input.h
+++ work/include/linux/input.h
@@ -845,7 +845,7 @@ struct ff_rumble_effect {
* defining effect parameters
*
* This structure is sent through ioctl from the application to the driver.
- * To create a new effect aplication should set its @id to -1; the kernel
+ * To create a new effect application should set its @id to -1; the kernel
* will return assigned @id which can later be used to update or delete
* this effect.
*
@@ -925,9 +925,82 @@ struct ff_effect {
#define BIT(x) (1UL<<((x)%BITS_PER_LONG))
#define LONG(x) ((x)/BITS_PER_LONG)
+/**
+ * struct input_dev - represents an input device
+ * @name: name of the device
+ * @phys: physical path to the device in the system hierarchy
+ * @uniq: unique identification code for the device (if device has it)
+ * @id: id of the device (struct input_id)
+ * @evbit: bitmap of types of events supported by the device (EV_KEY,
+ * EV_REL, etc.)
+ * @keybit: bitmap of keys/buttons this device has
+ * @relbit: bitmap of relative axes for the device
+ * @absbit: bitmap of absolute axes for the device
+ * @mscbit: bitmap of miscellaneous events supported by the device
+ * @ledbit: bitmap of leds present on the device
+ * @sndbit: bitmap of sound effects supported by the device
+ * @ffbit: bitmap of force feedback effects supported by the device
+ * @swbit: bitmap of switches present on the device
+ * @keycodemax: size of keycode table
+ * @keycodesize: size of elements in keycode table
+ * @keycode: map of scancodes to keycodes for this device
+ * @setkeycode: optional method to alter current keymap, used to implement
+ * sparse keymaps. If not supplied default mechanism will be used
+ * @getkeycode: optional method to retrieve current keymap. If not supplied
+ * default mechanism will be used
+ * @ff: force feedback structure associated with the device if device
+ * supports force feedback effects
+ * @repeat_key: stores key code of the last key pressed; used to implement
+ * software autorepeat
+ * @timer: timer for software autorepeat
+ * @sync: set to 1 when there were no new events since last EV_SYNC
+ * @abs: current values for reports from absolute axes
+ * @rep: current values for autorepeat parameters (delay, rate)
+ * @key: reflects current state of device's keys/buttons
+ * @led: reflects current state of device's LEDs
+ * @snd: reflects current state of sound effects
+ * @sw: reflects current state of device's switches
+ * @absmax: maximum values for events coming from absolute axes
+ * @absmin: minimum values for events coming from absolute axes
+ * @absfuzz: describes noisiness for axes
+ * @absflat: size of the center flat position (used by joydev)
+ * @open: this method is called when the very first user calls
+ * input_open_device(). The driver must prepare the device
+ * to start generating events (start polling thread,
+ * request an IRQ, submit URB, etc.)
+ * @close: this method is called when the very last user calls
+ * input_close_device().
+ * @flush: purges the device. Most commonly used to get rid of force
+ * feedback effects loaded into the device when disconnecting
+ * from it
+ * @event: event handler for events sent _to_ the device, like EV_LED
+ * or EV_SND. The device is expected to carry out the requested
+ * action (turn on a LED, play sound, etc.) The call is protected
+ * by @event_lock and must not sleep
+ * @grab: input handle that currently has the device grabbed (via
+ * EVIOCGRAB ioctl). When a handle grabs a device it becomes sole
+ * recipient for all input events coming from the device
+ * @event_lock: this spinlock is is taken when input core receives
+ * and processes a new event for the device (in input_event()).
+ * Code that accesses and/or modifies parameters of a device
+ * (such as keymap or absmin, absmax, absfuzz, etc.) after device
+ * has been registered with input core must take this lock.
+ * @mutex: serializes calls to open(), close() and flush() methods
+ * @users: stores number of users (input handlers) that opened this
+ * device. It is used by input_open_device() and input_close_device()
+ * to make sure that dev->open() is only called when the first
+ * user opens device and dev->close() is called when the very
+ * last user closes the device
+ * @going_away: marks devices that are in a middle of unregistering and
+ * causes input_open_device*() fail with -ENODEV.
+ * @dev: driver model's view of this device
+ * @h_list: list of input handles associated with the device. When
+ * accessing the list dev->mutex must be held
+ * @node: used to place the device onto input_dev_list
+ */
struct input_dev {
- void *private;
+ void *private; /* do not use */
const char *name;
const char *phys;
@@ -955,8 +1028,6 @@ struct input_dev {
unsigned int repeat_key;
struct timer_list timer;
- int state;
-
int sync;
int abs[ABS_MAX + 1];
@@ -979,8 +1050,11 @@ struct input_dev {
struct input_handle *grab;
- struct mutex mutex; /* serializes open and close operations */
+ spinlock_t event_lock;
+ struct mutex mutex;
+
unsigned int users;
+ int going_away;
struct device dev;
union { /* temporarily so while we switching to struct device */
@@ -1046,7 +1120,9 @@ struct input_handle;
/**
* struct input_handler - implements one of interfaces for input devices
* @private: driver-specific data
- * @event: event handler
+ * @event: event handler. This method is being called by input core with
+ * interrupts disabled and dev->event_lock spinlock held and so
+ * it may not sleep
* @connect: called when attaching a handler to an input device
* @disconnect: disconnects a handler from input device
* @start: starts handler for given handle. This function is called by
@@ -1058,10 +1134,18 @@ struct input_handle;
* @name: name of the handler, to be shown in /proc/bus/input/handlers
* @id_table: pointer to a table of input_device_ids this driver can
* handle
- * @blacklist: prointer to a table of input_device_ids this driver should
+ * @blacklist: pointer to a table of input_device_ids this driver should
* ignore even if they match @id_table
* @h_list: list of input handles associated with the handler
* @node: for placing the driver onto input_handler_list
+ *
+ * Input handlers attach to input devices and create input handles. There
+ * are likely several handlers attached to any given input device at the
+ * same time. All of them will get their copy of input event generated by
+ * the device.
+ *
+ * Note that input core serializes calls to connect() and disconnect()
+ * methods.
*/
struct input_handler {
@@ -1083,6 +1167,18 @@ struct input_handler {
struct list_head node;
};
+/**
+ * struct input_handle - links input device with an input handler
+ * @private: handler-specific data
+ * @open: counter showing whether the handle is 'open', i.e. should deliver
+ * events from its device
+ * @name: name given to the handle by handler that created it
+ * @dev: input device the handle is attached to
+ * @handler: handler that works with the device through this handle
+ * @d_node: used to put the handle on device's list of attached handles
+ * @h_node: used to put the handle on handler's list of handles from which
+ * it gets events
+ */
struct input_handle {
void *private;
@@ -1205,7 +1301,7 @@ extern struct class input_class;
* @max_effects: maximum number of effects supported by device
* @effects: pointer to an array of effects currently loaded into device
* @effect_owners: array of effect owners; when file handle owning
- * an effect gets closed the effcet is automatically erased
+ * an effect gets closed the effect is automatically erased
*
* Every force-feedback device must implement upload() and playback()
* methods; erase() is optional. set_gain() and set_autocenter() need
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC/RFT 1/5] Input: implement proper locking in input core
2007-07-24 4:45 ` [RFC/RFT 1/5] Input: implement proper locking in input core Dmitry Torokhov
@ 2007-07-24 5:35 ` Jeff Garzik
2007-07-24 5:52 ` Dmitry Torokhov
2007-07-27 23:28 ` Indan Zupancic
1 sibling, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2007-07-24 5:35 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
Dmitry Torokhov wrote:
> +static void input_repeat_key(unsigned long data)
> +{
> + struct input_dev *dev = (void *) data;
>
> - change_bit(code, dev->key);
> + spin_lock_irq(&dev->event_lock);
[...]
> +void input_inject_event(struct input_handle *handle,
> + unsigned int type, unsigned int code, int value)
> {
> - struct input_dev *dev = (void *) data;
> + struct input_dev *dev = handle->dev;
> + struct input_handle *grab;
>
> - if (!test_bit(dev->repeat_key, dev->key))
> - return;
> + if (is_event_supported(type, dev->evbit, EV_MAX)) {
> + spin_lock_irq(&dev->event_lock);
>
> - input_event(dev, EV_KEY, dev->repeat_key, 2);
> - input_sync(dev);
> + grab = rcu_dereference(dev->grab);
> + if (!grab || grab == handle)
> + input_handle_event(dev, type, code, value);
>
> - if (dev->rep[REP_PERIOD])
> - mod_timer(&dev->timer, jiffies + msecs_to_jiffies(dev->rep[REP_PERIOD]));
> + spin_unlock_irq(&dev->event_lock);
> + }
> }
> +EXPORT_SYMBOL(input_inject_event);
[...]
> + spin_lock_irq(&dev->event_lock);
> +
> + /*
> + * Simulate keyup events for all pressed keys so that handlers
> + * are not left with "stuck" keys. The driver may continue
> + * generate events even after we done here but they will not
> + * reach any handlers.
> + */
> + if (is_event_supported(EV_KEY, dev->evbit, EV_MAX)) {
> + for (code = 0; code <= KEY_MAX; code++) {
> + if (is_event_supported(code, dev->keybit, KEY_MAX) &&
> + test_bit(code, dev->key)) {
> + input_pass_event(dev, EV_KEY, code, 0);
> + }
> + }
> + input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
> + }
> +
> + list_for_each_entry(handle, &dev->h_list, d_node)
> + handle->open = 0;
> +
> + spin_unlock_irq(&dev->event_lock);
spin_lock_irq() should generally be avoided.
In cases like the first case -- input_repeat_key() -- you are making
incorrect assumptions about the state of interrupts. The other cases
are probably ok, but in general spin_lock_irq() has a long history of
being very fragile and quite often wrong.
Use spin_lock_irqsave() to be safe. Definitely in input_repeat_key(),
but I strongly recommend removing spin_lock_irq() from all your patches
here.
Jeff
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC/RFT 1/5] Input: implement proper locking in input core
2007-07-24 5:35 ` Jeff Garzik
@ 2007-07-24 5:52 ` Dmitry Torokhov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-24 5:52 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-input, linux-kernel
Hi Jeff,
On Tuesday 24 July 2007 01:35, Jeff Garzik wrote:
>
> spin_lock_irq() should generally be avoided.
>
> In cases like the first case -- input_repeat_key() -- you are making
> incorrect assumptions about the state of interrupts. The other cases
> are probably ok, but in general spin_lock_irq() has a long history of
> being very fragile and quite often wrong.
>
> Use spin_lock_irqsave() to be safe. Definitely in input_repeat_key(),
> but I strongly recommend removing spin_lock_irq() from all your patches
> here.
>
Thasnk you for looking at the patches. Actually I went back and forth
between spin_lock_irq and spin_lock_irqsave.. I will change back to
irqsave version, it is indeed safer.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC/RFT 1/5] Input: implement proper locking in input core
2007-07-24 4:45 ` [RFC/RFT 1/5] Input: implement proper locking in input core Dmitry Torokhov
2007-07-24 5:35 ` Jeff Garzik
@ 2007-07-27 23:28 ` Indan Zupancic
2007-07-29 3:50 ` Dmitry Torokhov
1 sibling, 1 reply; 15+ messages in thread
From: Indan Zupancic @ 2007-07-27 23:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
Hi,
Not real feedback, just some nitpicks.
On Tue, July 24, 2007 06:45, Dmitry Torokhov wrote:
> +static int input_defuzz_abs_event(int value, int old_val, int fuzz)
> +{
> + if (fuzz) {
> + if (value > old_val - fuzz / 2 && value < old_val + fuzz / 2)
> + return value;
>
> - add_input_randomness(type, code, value);
> + if (value > old_val - fuzz && value < old_val + fuzz)
> + return (old_val * 3 + value) / 4;
>
> - switch (type) {
> + if (value > old_val - fuzz * 2 && value < old_val + fuzz * 2)
> + return (old_val + value) / 2;
> + }
Shouldn't the return values of the second and third case be reversed?
In the 2nd check the new values is weighted for 1/4, while in the 3rd
case it counts for 1/2, which breaks the "account new value more when
it is closer to the old one" logic that I thought I saw here. So to sum up,
should the second return be "return (old_val + value * 3) / 4"?
> +/*
> + * Generate software autorepeat event. Note that we take
> + * dev->event_lock here to avoid racing with input_event
> + * which may cause keys get "stuck".
> + */
Hurray. :-)
> - if (code > SW_MAX || !test_bit(code, dev->swbit) || !!test_bit(code, dev->sw) == value)
> - return;
> + if (dev->rep[REP_PERIOD])
> + mod_timer(&dev->timer, jiffies +
> + msecs_to_jiffies(dev->rep[REP_PERIOD]));
> + }
Perhaps use a local var for the "msecs_to_jiffies(dev->rep[REP_PERIOD])" part.
> +static void input_start_autorepeat(struct input_dev *dev, int code)
> +{
> + if (test_bit(EV_REP, dev->evbit) &&
> + dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
> + dev->timer.data) {
> + dev->repeat_key = code;
> + mod_timer(&dev->timer,
> + jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
> + }
> +}
Same here.
> + case EV_KEY:
> + if (is_event_supported(code, dev->keybit, KEY_MAX) &&
> + !!test_bit(code, dev->key) != value) {
A bit confusing, test_bit(0 only returns 0 or 1 anyway, doesn't it?
So "test_bit(code, dev->key) != value" should be all right.
I noticed that the old code did it too, but still.
> - case EV_MSC:
> + case EV_SW:
> + if (is_event_supported(code, dev->swbit, SW_MAX) &&
> + !!test_bit(code, dev->sw) != value) {
Same.
> - break;
> + case EV_LED:
> + if (is_event_supported(code, dev->ledbit, LED_MAX) &&
> + !!test_bit(code, dev->led) != value) {
And here.
> +void input_inject_event(struct input_handle *handle,
> + unsigned int type, unsigned int code, int value)
> {
> - struct input_dev *dev = (void *) data;
> + struct input_dev *dev = handle->dev;
> + struct input_handle *grab;
>
> - if (!test_bit(dev->repeat_key, dev->key))
> - return;
> + if (is_event_supported(type, dev->evbit, EV_MAX)) {
> + spin_lock_irq(&dev->event_lock);
>
> - input_event(dev, EV_KEY, dev->repeat_key, 2);
> - input_sync(dev);
> + grab = rcu_dereference(dev->grab);
> + if (!grab || grab == handle)
> + input_handle_event(dev, type, code, value);
'handle' can't be NULL, so can drop the "!grab" check, as checking
"grab == handle" should be sufficient.
> +/**
> + * input_open_device - open input device
> + * @handle: handle through which device is being accessed
> + *
> + * This function should be called by input handlers when they
> + * want to start receive events from given input device.
> + */
> int input_open_device(struct input_handle *handle)
> {
> struct input_dev *dev = handle->dev;
> - int err;
> + int retval;
>
> - err = mutex_lock_interruptible(&dev->mutex);
> - if (err)
> - return err;
> + retval = mutex_lock_interruptible(&dev->mutex);
> + if (retval)
> + return retval;
> +
> + if (dev->going_away) {
> + retval = -ENODEV;
> + goto out;
> + }
>
> handle->open++;
>
> if (!dev->users++ && dev->open)
Ugh, not your code, and perhaps it's me, but that looks weird.
The ++ hidden inthe if check is ugly, and would mean that "users"
can be negative, which is strange.
> - err = dev->open(dev);
> + retval = dev->open(dev);
>
> - if (err)
> - handle->open--;
> + if (retval && !--handle->open) {
Eek! That -- is hidden well there. Would it hurt to call synchronize_sched()
unconditionally? Something like:
if (retval) {
handle->open--;
It's a rare case anyway.
> + /*
> + * Make sure we are not delivering any more events
> + * through this handle
> + */
> + synchronize_sched();
> + }
>
> +/**
> + * input_close_device - close input device
> + * @handle: handle through which device is being accessed
> + *
> + * This function should be called by input handlers when they
> + * want to stop receive events from given input device.
> + */
> void input_close_device(struct input_handle *handle)
> {
> struct input_dev *dev = handle->dev;
>
> - input_release_device(handle);
> -
> mutex_lock(&dev->mutex);
>
> + __input_release_device(handle);
> +
> if (!--dev->users && dev->close)
> dev->close(dev);
> - handle->open--;
> +
> + if (!--handle->open) {
> + /*
> + * synchronize_sched() makes sure that input_pass_event()
> + * completed and that no more input events are delivered
> + * through this handle
> + */
> + synchronize_sched();
> + }
Same here, though just leaving the original "handle->open--;" there and
merely adding the if check would be better too I think. Or just get rid of
the whole if thing.
> static void input_seq_print_bitmap(struct seq_file *seq, const char *name,
> @@ -569,7 +765,9 @@ static const struct file_operations inpu
>
> static void *input_handlers_seq_start(struct seq_file *seq, loff_t *pos)
> {
> - /* acquire lock here ... Yes, we do need locking, I knowi, I know... */
;-)
> + if (mutex_lock_interruptible(&input_mutex))
> + return NULL;
> +
> seq->private = (void *)(unsigned long)*pos;
> return seq_list_start(&input_handler_list, *pos);
> }
Greetings,
Indan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC/RFT 1/5] Input: implement proper locking in input core
2007-07-27 23:28 ` Indan Zupancic
@ 2007-07-29 3:50 ` Dmitry Torokhov
2007-07-29 12:50 ` Indan Zupancic
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-29 3:50 UTC (permalink / raw)
To: Indan Zupancic; +Cc: linux-input, linux-kernel
Hi Indan,
On Friday 27 July 2007 19:28, Indan Zupancic wrote:
> Hi,
>
> Not real feedback, just some nitpicks.
>
> On Tue, July 24, 2007 06:45, Dmitry Torokhov wrote:
> > +static int input_defuzz_abs_event(int value, int old_val, int fuzz)
> > +{
> > + if (fuzz) {
> > + if (value > old_val - fuzz / 2 && value < old_val + fuzz / 2)
> > + return value;
> >
> > - add_input_randomness(type, code, value);
> > + if (value > old_val - fuzz && value < old_val + fuzz)
> > + return (old_val * 3 + value) / 4;
> >
> > - switch (type) {
> > + if (value > old_val - fuzz * 2 && value < old_val + fuzz * 2)
> > + return (old_val + value) / 2;
> > + }
>
> Shouldn't the return values of the second and third case be reversed?
> In the 2nd check the new values is weighted for 1/4, while in the 3rd
> case it counts for 1/2, which breaks the "account new value more when
> it is closer to the old one" logic that I thought I saw here. So to sum up,
> should the second return be "return (old_val + value * 3) / 4"?
Thank you for bringing this up. Actually the 1st return valus should be
"old_val", not value. The logic is to "gravitate towards old" when
difference is small.
>
>
> > +/*
> > + * Generate software autorepeat event. Note that we take
> > + * dev->event_lock here to avoid racing with input_event
> > + * which may cause keys get "stuck".
> > + */
>
> Hurray. :-)
>
> > - if (code > SW_MAX || !test_bit(code, dev->swbit) || !!test_bit(code, dev->sw) == value)
> > - return;
> > + if (dev->rep[REP_PERIOD])
> > + mod_timer(&dev->timer, jiffies +
> > + msecs_to_jiffies(dev->rep[REP_PERIOD]));
> > + }
>
> Perhaps use a local var for the "msecs_to_jiffies(dev->rep[REP_PERIOD])" part.
>
What would be the benefit of doing so?
>
> > +static void input_start_autorepeat(struct input_dev *dev, int code)
> > +{
> > + if (test_bit(EV_REP, dev->evbit) &&
> > + dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
> > + dev->timer.data) {
> > + dev->repeat_key = code;
> > + mod_timer(&dev->timer,
> > + jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
> > + }
> > +}
>
> Same here.
>
>
> > + case EV_KEY:
> > + if (is_event_supported(code, dev->keybit, KEY_MAX) &&
> > + !!test_bit(code, dev->key) != value) {
>
> A bit confusing, test_bit(0 only returns 0 or 1 anyway, doesn't it?
> So "test_bit(code, dev->key) != value" should be all right.
> I noticed that the old code did it too, but still.
Is it guaranteed? I only expect it to return 0/non-0 values, not necessarily
0 and 1.
>
> > - case EV_MSC:
> > + case EV_SW:
> > + if (is_event_supported(code, dev->swbit, SW_MAX) &&
> > + !!test_bit(code, dev->sw) != value) {
>
> Same.
>
> > - break;
> > + case EV_LED:
> > + if (is_event_supported(code, dev->ledbit, LED_MAX) &&
> > + !!test_bit(code, dev->led) != value) {
>
> And here.
>
>
> > +void input_inject_event(struct input_handle *handle,
> > + unsigned int type, unsigned int code, int value)
> > {
> > - struct input_dev *dev = (void *) data;
> > + struct input_dev *dev = handle->dev;
> > + struct input_handle *grab;
> >
> > - if (!test_bit(dev->repeat_key, dev->key))
> > - return;
> > + if (is_event_supported(type, dev->evbit, EV_MAX)) {
> > + spin_lock_irq(&dev->event_lock);
> >
> > - input_event(dev, EV_KEY, dev->repeat_key, 2);
> > - input_sync(dev);
> > + grab = rcu_dereference(dev->grab);
> > + if (!grab || grab == handle)
> > + input_handle_event(dev, type, code, value);
>
> 'handle' can't be NULL, so can drop the "!grab" check, as checking
> "grab == handle" should be sufficient.
>
It is "or", not "and". The idea is to pass the event if device is not
grabbed by anyone _or_ if source of event is handle that grabbed the
device.
>
> > +/**
> > + * input_open_device - open input device
> > + * @handle: handle through which device is being accessed
> > + *
> > + * This function should be called by input handlers when they
> > + * want to start receive events from given input device.
> > + */
> > int input_open_device(struct input_handle *handle)
> > {
> > struct input_dev *dev = handle->dev;
> > - int err;
> > + int retval;
> >
> > - err = mutex_lock_interruptible(&dev->mutex);
> > - if (err)
> > - return err;
> > + retval = mutex_lock_interruptible(&dev->mutex);
> > + if (retval)
> > + return retval;
> > +
> > + if (dev->going_away) {
> > + retval = -ENODEV;
> > + goto out;
> > + }
> >
> > handle->open++;
> >
> > if (!dev->users++ && dev->open)
>
> Ugh, not your code, and perhaps it's me, but that looks weird.
> The ++ hidden inthe if check is ugly, and would mean that "users"
> can be negative, which is strange.
>
Why would it mean that?
> > - err = dev->open(dev);
> > + retval = dev->open(dev);
> >
> > - if (err)
> > - handle->open--;
> > + if (retval && !--handle->open) {
>
> Eek! That -- is hidden well there. Would it hurt to call synchronize_sched()
> unconditionally? Something like:
>
> if (retval) {
> handle->open--;
>
> It's a rare case anyway.
>
Because it would not be needed and the follwing comment would be false.
> > + /*
> > + * Make sure we are not delivering any more events
> > + * through this handle
> > + */
> > + synchronize_sched();
> > + }
> >
>
> > +/**
> > + * input_close_device - close input device
> > + * @handle: handle through which device is being accessed
> > + *
> > + * This function should be called by input handlers when they
> > + * want to stop receive events from given input device.
> > + */
> > void input_close_device(struct input_handle *handle)
> > {
> > struct input_dev *dev = handle->dev;
> >
> > - input_release_device(handle);
> > -
> > mutex_lock(&dev->mutex);
> >
> > + __input_release_device(handle);
> > +
> > if (!--dev->users && dev->close)
> > dev->close(dev);
> > - handle->open--;
> > +
> > + if (!--handle->open) {
> > + /*
> > + * synchronize_sched() makes sure that input_pass_event()
> > + * completed and that no more input events are delivered
> > + * through this handle
> > + */
> > + synchronize_sched();
> > + }
>
> Same here, though just leaving the original "handle->open--;" there and
> merely adding the if check would be better too I think. Or just get rid of
> the whole if thing.
>
No, we do not want to do synchronize_sched when there are more users.
>
> > static void input_seq_print_bitmap(struct seq_file *seq, const char *name,
> > @@ -569,7 +765,9 @@ static const struct file_operations inpu
> >
> > static void *input_handlers_seq_start(struct seq_file *seq, loff_t *pos)
> > {
> > - /* acquire lock here ... Yes, we do need locking, I knowi, I know... */
>
> ;-)
>
> > + if (mutex_lock_interruptible(&input_mutex))
> > + return NULL;
> > +
> > seq->private = (void *)(unsigned long)*pos;
> > return seq_list_start(&input_handler_list, *pos);
> > }
>
> Greetings,
>
> Indan
>
>
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC/RFT 1/5] Input: implement proper locking in input core
2007-07-29 3:50 ` Dmitry Torokhov
@ 2007-07-29 12:50 ` Indan Zupancic
0 siblings, 0 replies; 15+ messages in thread
From: Indan Zupancic @ 2007-07-29 12:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
On Sun, July 29, 2007 05:50, Dmitry Torokhov wrote:
> Hi Indan,
>
> On Friday 27 July 2007 19:28, Indan Zupancic wrote:
>> Hi,
>>
>> Not real feedback, just some nitpicks.
>>
>> On Tue, July 24, 2007 06:45, Dmitry Torokhov wrote:
>> > +static int input_defuzz_abs_event(int value, int old_val, int fuzz)
>> > +{
>> > + if (fuzz) {
>> > + if (value > old_val - fuzz / 2 && value < old_val + fuzz / 2)
>> > + return value;
>> >
>> > - add_input_randomness(type, code, value);
>> > + if (value > old_val - fuzz && value < old_val + fuzz)
>> > + return (old_val * 3 + value) / 4;
>> >
>> > - switch (type) {
>> > + if (value > old_val - fuzz * 2 && value < old_val + fuzz * 2)
>> > + return (old_val + value) / 2;
>> > + }
>>
>> Shouldn't the return values of the second and third case be reversed?
>> In the 2nd check the new values is weighted for 1/4, while in the 3rd
>> case it counts for 1/2, which breaks the "account new value more when
>> it is closer to the old one" logic that I thought I saw here. So to sum up,
>> should the second return be "return (old_val + value * 3) / 4"?
>
> Thank you for bringing this up. Actually the 1st return valus should be
> "old_val", not value. The logic is to "gravitate towards old" when
> difference is small.
>
>>
>>
>> > +/*
>> > + * Generate software autorepeat event. Note that we take
>> > + * dev->event_lock here to avoid racing with input_event
>> > + * which may cause keys get "stuck".
>> > + */
>>
>> Hurray. :-)
>>
>> > - if (code > SW_MAX || !test_bit(code, dev->swbit) || !!test_bit(code, dev->sw) == value)
>> > - return;
>> > + if (dev->rep[REP_PERIOD])
>> > + mod_timer(&dev->timer, jiffies +
>> > + msecs_to_jiffies(dev->rep[REP_PERIOD]));
>> > + }
>>
>> Perhaps use a local var for the "msecs_to_jiffies(dev->rep[REP_PERIOD])" part.
>>
>
> What would be the benefit of doing so?
I was hoping it would make things more readable, and less staircase like.
>
>>
>> > +static void input_start_autorepeat(struct input_dev *dev, int code)
>> > +{
>> > + if (test_bit(EV_REP, dev->evbit) &&
>> > + dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
>> > + dev->timer.data) {
>> > + dev->repeat_key = code;
>> > + mod_timer(&dev->timer,
>> > + jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
>> > + }
>> > +}
>>
>> Same here.
>>
>>
>> > + case EV_KEY:
>> > + if (is_event_supported(code, dev->keybit, KEY_MAX) &&
>> > + !!test_bit(code, dev->key) != value) {
>>
>> A bit confusing, test_bit(0 only returns 0 or 1 anyway, doesn't it?
>> So "test_bit(code, dev->key) != value" should be all right.
>> I noticed that the old code did it too, but still.
>
> Is it guaranteed? I only expect it to return 0/non-0 values, not necessarily
> 0 and 1.
Not sure, but it seems so. All the test_bit implementations I checked
returned either 0 or 1. No idea where to get that guarantee though.
>
>>
>> > - case EV_MSC:
>> > + case EV_SW:
>> > + if (is_event_supported(code, dev->swbit, SW_MAX) &&
>> > + !!test_bit(code, dev->sw) != value) {
>>
>> Same.
>>
>> > - break;
>> > + case EV_LED:
>> > + if (is_event_supported(code, dev->ledbit, LED_MAX) &&
>> > + !!test_bit(code, dev->led) != value) {
>>
>> And here.
>>
>>
>> > +void input_inject_event(struct input_handle *handle,
>> > + unsigned int type, unsigned int code, int value)
>> > {
>> > - struct input_dev *dev = (void *) data;
>> > + struct input_dev *dev = handle->dev;
>> > + struct input_handle *grab;
>> >
>> > - if (!test_bit(dev->repeat_key, dev->key))
>> > - return;
>> > + if (is_event_supported(type, dev->evbit, EV_MAX)) {
>> > + spin_lock_irq(&dev->event_lock);
>> >
>> > - input_event(dev, EV_KEY, dev->repeat_key, 2);
>> > - input_sync(dev);
>> > + grab = rcu_dereference(dev->grab);
>> > + if (!grab || grab == handle)
>> > + input_handle_event(dev, type, code, value);
>>
>> 'handle' can't be NULL, so can drop the "!grab" check, as checking
>> "grab == handle" should be sufficient.
>>
>
> It is "or", not "and". The idea is to pass the event if device is not
> grabbed by anyone _or_ if source of event is handle that grabbed the
> device.
Ah, oops.
>
>>
>> > +/**
>> > + * input_open_device - open input device
>> > + * @handle: handle through which device is being accessed
>> > + *
>> > + * This function should be called by input handlers when they
>> > + * want to start receive events from given input device.
>> > + */
>> > int input_open_device(struct input_handle *handle)
>> > {
>> > struct input_dev *dev = handle->dev;
>> > - int err;
>> > + int retval;
>> >
>> > - err = mutex_lock_interruptible(&dev->mutex);
>> > - if (err)
>> > - return err;
>> > + retval = mutex_lock_interruptible(&dev->mutex);
>> > + if (retval)
>> > + return retval;
>> > +
>> > + if (dev->going_away) {
>> > + retval = -ENODEV;
>> > + goto out;
>> > + }
>> >
>> > handle->open++;
>> >
>> > if (!dev->users++ && dev->open)
>>
>> Ugh, not your code, and perhaps it's me, but that looks weird.
>> The ++ hidden inthe if check is ugly, and would mean that "users"
>> can be negative, which is strange.
>>
>
> Why would it mean that?
I was confused by those pre-decrements, this one is post-increment,
so it doesn't.
>
>> > - err = dev->open(dev);
>> > + retval = dev->open(dev);
>> >
>> > - if (err)
>> > - handle->open--;
>> > + if (retval && !--handle->open) {
>>
>> Eek! That -- is hidden well there. Would it hurt to call synchronize_sched()
>> unconditionally? Something like:
>>
>> if (retval) {
>> handle->open--;
>>
>> It's a rare case anyway.
>>
>
> Because it would not be needed and the follwing comment would be false.
>
>> > + /*
>> > + * Make sure we are not delivering any more events
>> > + * through this handle
>> > + */
>> > + synchronize_sched();
>> > + }
>> >
>>
>> > +/**
>> > + * input_close_device - close input device
>> > + * @handle: handle through which device is being accessed
>> > + *
>> > + * This function should be called by input handlers when they
>> > + * want to stop receive events from given input device.
>> > + */
>> > void input_close_device(struct input_handle *handle)
>> > {
>> > struct input_dev *dev = handle->dev;
>> >
>> > - input_release_device(handle);
>> > -
>> > mutex_lock(&dev->mutex);
>> >
>> > + __input_release_device(handle);
>> > +
>> > if (!--dev->users && dev->close)
>> > dev->close(dev);
>> > - handle->open--;
>> > +
>> > + if (!--handle->open) {
>> > + /*
>> > + * synchronize_sched() makes sure that input_pass_event()
>> > + * completed and that no more input events are delivered
>> > + * through this handle
>> > + */
>> > + synchronize_sched();
>> > + }
>>
>> Same here, though just leaving the original "handle->open--;" there and
>> merely adding the if check would be better too I think. Or just get rid of
>> the whole if thing.
>>
>
> No, we do not want to do synchronize_sched when there are more users.
Yes, missed that, for some reason I thought it was the other way round.
Greetings,
Indan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC/RFT 2/5] evdev - implement proper locking
2007-07-24 4:45 [RFC/RFT 0/5] Input locking patches Dmitry Torokhov
2007-07-24 4:45 ` [RFC/RFT 1/5] Input: implement proper locking in input core Dmitry Torokhov
@ 2007-07-24 4:45 ` Dmitry Torokhov
2007-07-24 4:45 ` [RFC/RFT 3/5] Input: tsdev " Dmitry Torokhov
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-24 4:45 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
[-- Attachment #1: evdev-locking.patch --]
[-- Type: text/plain, Size: 28126 bytes --]
Input: evdev - implement proper locking
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/evdev.c | 719 +++++++++++++++++++++++++++++++++-----------------
1 files changed, 476 insertions(+), 243 deletions(-)
Index: work/drivers/input/evdev.c
===================================================================
--- work.orig/drivers/input/evdev.c
+++ work/drivers/input/evdev.c
@@ -30,6 +30,8 @@ struct evdev {
wait_queue_head_t wait;
struct evdev_client *grab;
struct list_head client_list;
+ spinlock_t client_lock; /* protects client_list */
+ struct mutex mutex;
struct device dev;
};
@@ -37,39 +39,53 @@ struct evdev_client {
struct input_event buffer[EVDEV_BUFFER_SIZE];
int head;
int tail;
+ spinlock_t buffer_lock; /* protects access to buffer, head and tail */
struct fasync_struct *fasync;
struct evdev *evdev;
struct list_head node;
};
static struct evdev *evdev_table[EVDEV_MINORS];
+static DEFINE_MUTEX(evdev_table_mutex);
-static void evdev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
+static void evdev_pass_event(struct evdev_client *client,
+ struct input_event *event)
+{
+ /*
+ * Interrupts are disabled, just acquire the lock
+ */
+ spin_lock(&client->buffer_lock);
+ client->buffer[client->head++] = *event;
+ client->head &= EVDEV_BUFFER_SIZE - 1;
+ spin_unlock(&client->buffer_lock);
+
+ kill_fasync(&client->fasync, SIGIO, POLL_IN);
+}
+
+/*
+ * Pass incoming event to all connected clients. Note that we are
+ * caleld under a spinlock with interrupts off so we don't need
+ * to use rcu_read_lock() here. Writers will be using syncronize_sched()
+ * instead of synchrnoize_rcu().
+ */
+static void evdev_event(struct input_handle *handle,
+ unsigned int type, unsigned int code, int value)
{
struct evdev *evdev = handle->private;
struct evdev_client *client;
+ struct input_event event;
- if (evdev->grab) {
- client = evdev->grab;
-
- do_gettimeofday(&client->buffer[client->head].time);
- client->buffer[client->head].type = type;
- client->buffer[client->head].code = code;
- client->buffer[client->head].value = value;
- client->head = (client->head + 1) & (EVDEV_BUFFER_SIZE - 1);
-
- kill_fasync(&client->fasync, SIGIO, POLL_IN);
- } else
- list_for_each_entry(client, &evdev->client_list, node) {
-
- do_gettimeofday(&client->buffer[client->head].time);
- client->buffer[client->head].type = type;
- client->buffer[client->head].code = code;
- client->buffer[client->head].value = value;
- client->head = (client->head + 1) & (EVDEV_BUFFER_SIZE - 1);
-
- kill_fasync(&client->fasync, SIGIO, POLL_IN);
- }
+ do_gettimeofday(&event.time);
+ event.type = type;
+ event.code = code;
+ event.value = value;
+
+ client = rcu_dereference(evdev->grab);
+ if (client)
+ evdev_pass_event(client, &event);
+ else
+ list_for_each_entry_rcu(client, &evdev->client_list, node)
+ evdev_pass_event(client, &event);
wake_up_interruptible(&evdev->wait);
}
@@ -88,38 +104,142 @@ static int evdev_flush(struct file *file
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
+ int retval;
+
+ retval = mutex_lock_interruptible(&evdev->mutex);
+ if (retval)
+ return retval;
if (!evdev->exist)
- return -ENODEV;
+ retval = -ENODEV;
+ else
+ retval = input_flush_device(&evdev->handle, file);
- return input_flush_device(&evdev->handle, file);
+ mutex_unlock(&evdev->mutex);
+ return retval;
}
static void evdev_free(struct device *dev)
{
struct evdev *evdev = container_of(dev, struct evdev, dev);
- evdev_table[evdev->minor] = NULL;
kfree(evdev);
}
+/*
+ * Grabs an event device (along with underlying input device).
+ * This function is called with evdev->mutex taken.
+ */
+static int evdev_grab(struct evdev *evdev, struct evdev_client *client)
+{
+ int error;
+
+ if (evdev->grab)
+ return -EBUSY;
+
+ error = input_grab_device(&evdev->handle);
+ if (error)
+ return error;
+
+ rcu_assign_pointer(evdev->grab, client);
+ /*
+ * We don't use synchronize_rcu() here because read-size
+ * critical section is protected by a spinlock instead
+ * of rcu_read_lock().
+ */
+ synchronize_sched();
+
+ return 0;
+}
+
+static int evdev_ungrab(struct evdev *evdev, struct evdev_client *client)
+{
+ if (evdev->grab != client)
+ return -EINVAL;
+
+ rcu_assign_pointer(evdev->grab, NULL);
+ synchronize_sched();
+ input_release_device(&evdev->handle);
+
+ return 0;
+}
+
+static void evdev_attach_client(struct evdev *evdev,
+ struct evdev_client *client)
+{
+ spin_lock(&evdev->client_lock);
+ list_add_tail_rcu(&client->node, &evdev->client_list);
+ spin_unlock(&evdev->client_lock);
+ synchronize_sched();
+}
+
+static void evdev_detach_client(struct evdev *evdev,
+ struct evdev_client *client)
+{
+ spin_lock(&evdev->client_lock);
+ list_del_rcu(&client->node);
+ spin_unlock(&evdev->client_lock);
+ synchronize_sched();
+}
+
+static int evdev_open_device(struct evdev *evdev)
+{
+ int retval;
+
+ retval = mutex_lock_interruptible(&evdev->mutex);
+ if (retval)
+ return retval;
+
+ if (!evdev->exist)
+ retval = -ENODEV;
+ else if (!evdev->open++)
+ retval = input_open_device(&evdev->handle);
+
+ mutex_unlock(&evdev->mutex);
+ return retval;
+}
+
+static void evdev_close_device(struct evdev *evdev)
+{
+ mutex_lock(&evdev->mutex);
+
+ if (evdev->exist && !--evdev->open)
+ input_close_device(&evdev->handle);
+
+ mutex_unlock(&evdev->mutex);
+}
+
+/*
+ * Wake up users waiting for IO so they can disconnect from
+ * dead device.
+ */
+static void evdev_hangup(struct evdev *evdev)
+{
+ struct evdev_client *client;
+
+ spin_lock(&evdev->client_lock);
+ list_for_each_entry(client, &evdev->client_list, node)
+ kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+ spin_unlock(&evdev->client_lock);
+
+ wake_up_interruptible(&evdev->wait);
+}
+
static int evdev_release(struct inode *inode, struct file *file)
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
- if (evdev->grab == client) {
- input_release_device(&evdev->handle);
- evdev->grab = NULL;
- }
+ mutex_lock(&evdev->mutex);
+ if (evdev->grab == client)
+ evdev_ungrab(evdev, client);
+ mutex_unlock(&evdev->mutex);
evdev_fasync(-1, file, 0);
- list_del(&client->node);
+ evdev_detach_client(evdev, client);
kfree(client);
- if (!--evdev->open && evdev->exist)
- input_close_device(&evdev->handle);
-
+ evdev_close_device(evdev);
put_device(&evdev->dev);
return 0;
@@ -127,41 +247,44 @@ static int evdev_release(struct inode *i
static int evdev_open(struct inode *inode, struct file *file)
{
- struct evdev_client *client;
struct evdev *evdev;
+ struct evdev_client *client;
int i = iminor(inode) - EVDEV_MINOR_BASE;
int error;
if (i >= EVDEV_MINORS)
return -ENODEV;
+ error = mutex_lock_interruptible(&evdev_table_mutex);
+ if (error)
+ return error;
evdev = evdev_table[i];
+ if (evdev)
+ get_device(&evdev->dev);
+ mutex_unlock(&evdev_table_mutex);
- if (!evdev || !evdev->exist)
+ if (!evdev)
return -ENODEV;
- get_device(&evdev->dev);
-
client = kzalloc(sizeof(struct evdev_client), GFP_KERNEL);
if (!client) {
error = -ENOMEM;
goto err_put_evdev;
}
+ spin_lock_init(&client->buffer_lock);
client->evdev = evdev;
- list_add_tail(&client->node, &evdev->client_list);
+ evdev_attach_client(evdev, client);
- if (!evdev->open++ && evdev->exist) {
- error = input_open_device(&evdev->handle);
- if (error)
- goto err_free_client;
- }
+ error = evdev_open_device(evdev);
+ if (error)
+ goto err_free_client;
file->private_data = client;
return 0;
err_free_client:
- list_del(&client->node);
+ evdev_detach_client(evdev, client);
kfree(client);
err_put_evdev:
put_device(&evdev->dev);
@@ -197,12 +320,14 @@ static inline size_t evdev_event_size(vo
sizeof(struct input_event_compat) : sizeof(struct input_event);
}
-static int evdev_event_from_user(const char __user *buffer, struct input_event *event)
+static int evdev_event_from_user(const char __user *buffer,
+ struct input_event *event)
{
if (COMPAT_TEST) {
struct input_event_compat compat_event;
- if (copy_from_user(&compat_event, buffer, sizeof(struct input_event_compat)))
+ if (copy_from_user(&compat_event, buffer,
+ sizeof(struct input_event_compat)))
return -EFAULT;
event->time.tv_sec = compat_event.time.tv_sec;
@@ -219,7 +344,8 @@ static int evdev_event_from_user(const c
return 0;
}
-static int evdev_event_to_user(char __user *buffer, const struct input_event *event)
+static int evdev_event_to_user(char __user *buffer,
+ const struct input_event *event)
{
if (COMPAT_TEST) {
struct input_event_compat compat_event;
@@ -230,7 +356,8 @@ static int evdev_event_to_user(char __us
compat_event.code = event->code;
compat_event.value = event->value;
- if (copy_to_user(buffer, &compat_event, sizeof(struct input_event_compat)))
+ if (copy_to_user(buffer, &compat_event,
+ sizeof(struct input_event_compat)))
return -EFAULT;
} else {
@@ -248,7 +375,8 @@ static inline size_t evdev_event_size(vo
return sizeof(struct input_event);
}
-static int evdev_event_from_user(const char __user *buffer, struct input_event *event)
+static int evdev_event_from_user(const char __user *buffer,
+ struct input_event *event)
{
if (copy_from_user(event, buffer, sizeof(struct input_event)))
return -EFAULT;
@@ -256,7 +384,8 @@ static int evdev_event_from_user(const c
return 0;
}
-static int evdev_event_to_user(char __user *buffer, const struct input_event *event)
+static int evdev_event_to_user(char __user *buffer,
+ const struct input_event *event)
{
if (copy_to_user(buffer, event, sizeof(struct input_event)))
return -EFAULT;
@@ -266,37 +395,71 @@ static int evdev_event_to_user(char __us
#endif /* CONFIG_COMPAT */
-static ssize_t evdev_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t evdev_write(struct file *file, const char __user *buffer,
+ size_t count, loff_t *ppos)
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
struct input_event event;
- int retval = 0;
+ int retval;
- if (!evdev->exist)
- return -ENODEV;
+ retval = mutex_lock_interruptible(&evdev->mutex);
+ if (retval)
+ return retval;
+
+ if (!evdev->exist) {
+ retval = -ENODEV;
+ goto out;
+ }
while (retval < count) {
- if (evdev_event_from_user(buffer + retval, &event))
- return -EFAULT;
- input_inject_event(&evdev->handle, event.type, event.code, event.value);
+ if (evdev_event_from_user(buffer + retval, &event)) {
+ retval = -EFAULT;
+ goto out;
+ }
+
+ input_inject_event(&evdev->handle,
+ event.type, event.code, event.value);
retval += evdev_event_size();
}
+ out:
+ mutex_unlock(&evdev->mutex);
return retval;
}
-static ssize_t evdev_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
+static int evdev_fetch_next_event(struct evdev_client *client,
+ struct input_event *event)
+{
+ int have_event;
+
+ spin_lock_irq(&client->buffer_lock);
+
+ have_event = client->head != client->tail;
+ if (have_event) {
+ *event = client->buffer[client->tail++];
+ client->tail &= EVDEV_BUFFER_SIZE - 1;
+ }
+
+ spin_unlock_irq(&client->buffer_lock);
+
+ return have_event;
+}
+
+static ssize_t evdev_read(struct file *file, char __user *buffer,
+ size_t count, loff_t *ppos)
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
+ struct input_event event;
int retval;
if (count < evdev_event_size())
return -EINVAL;
- if (client->head == client->tail && evdev->exist && (file->f_flags & O_NONBLOCK))
+ if (client->head == client->tail && evdev->exist &&
+ (file->f_flags & O_NONBLOCK))
return -EAGAIN;
retval = wait_event_interruptible(evdev->wait,
@@ -307,14 +470,12 @@ static ssize_t evdev_read(struct file *f
if (!evdev->exist)
return -ENODEV;
- while (client->head != client->tail && retval + evdev_event_size() <= count) {
-
- struct input_event *event = (struct input_event *) client->buffer + client->tail;
+ while (retval + evdev_event_size() <= count &&
+ evdev_fetch_next_event(client, &event)) {
- if (evdev_event_to_user(buffer + retval, event))
+ if (evdev_event_to_user(buffer + retval, &event))
return -EFAULT;
- client->tail = (client->tail + 1) & (EVDEV_BUFFER_SIZE - 1);
retval += evdev_event_size();
}
@@ -409,8 +570,8 @@ static int str_to_user(const char *str,
return copy_to_user(p, str, len) ? -EFAULT : len;
}
-static long evdev_ioctl_handler(struct file *file, unsigned int cmd,
- void __user *p, int compat_mode)
+static long evdev_do_ioctl(struct file *file, unsigned int cmd,
+ void __user *p, int compat_mode)
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
@@ -421,186 +582,208 @@ static long evdev_ioctl_handler(struct f
int i, t, u, v;
int error;
- if (!evdev->exist)
- return -ENODEV;
-
switch (cmd) {
- case EVIOCGVERSION:
- return put_user(EV_VERSION, ip);
+ case EVIOCGVERSION:
+ return put_user(EV_VERSION, ip);
- case EVIOCGID:
- if (copy_to_user(p, &dev->id, sizeof(struct input_id)))
- return -EFAULT;
- return 0;
+ case EVIOCGID:
+ if (copy_to_user(p, &dev->id, sizeof(struct input_id)))
+ return -EFAULT;
+ return 0;
- case EVIOCGREP:
- if (!test_bit(EV_REP, dev->evbit))
- return -ENOSYS;
- if (put_user(dev->rep[REP_DELAY], ip))
- return -EFAULT;
- if (put_user(dev->rep[REP_PERIOD], ip + 1))
- return -EFAULT;
- return 0;
+ case EVIOCGREP:
+ if (!test_bit(EV_REP, dev->evbit))
+ return -ENOSYS;
+ if (put_user(dev->rep[REP_DELAY], ip))
+ return -EFAULT;
+ if (put_user(dev->rep[REP_PERIOD], ip + 1))
+ return -EFAULT;
+ return 0;
- case EVIOCSREP:
- if (!test_bit(EV_REP, dev->evbit))
- return -ENOSYS;
- if (get_user(u, ip))
- return -EFAULT;
- if (get_user(v, ip + 1))
- return -EFAULT;
+ case EVIOCSREP:
+ if (!test_bit(EV_REP, dev->evbit))
+ return -ENOSYS;
+ if (get_user(u, ip))
+ return -EFAULT;
+ if (get_user(v, ip + 1))
+ return -EFAULT;
- input_inject_event(&evdev->handle, EV_REP, REP_DELAY, u);
- input_inject_event(&evdev->handle, EV_REP, REP_PERIOD, v);
+ input_inject_event(&evdev->handle, EV_REP, REP_DELAY, u);
+ input_inject_event(&evdev->handle, EV_REP, REP_PERIOD, v);
- return 0;
+ return 0;
- case EVIOCGKEYCODE:
- if (get_user(t, ip))
- return -EFAULT;
+ case EVIOCGKEYCODE:
+ if (get_user(t, ip))
+ return -EFAULT;
- error = dev->getkeycode(dev, t, &v);
- if (error)
- return error;
+ error = dev->getkeycode(dev, t, &v);
+ if (error)
+ return error;
- if (put_user(v, ip + 1))
- return -EFAULT;
+ if (put_user(v, ip + 1))
+ return -EFAULT;
- return 0;
+ return 0;
- case EVIOCSKEYCODE:
- if (get_user(t, ip) || get_user(v, ip + 1))
- return -EFAULT;
+ case EVIOCSKEYCODE:
+ if (get_user(t, ip) || get_user(v, ip + 1))
+ return -EFAULT;
- return dev->setkeycode(dev, t, v);
+ return dev->setkeycode(dev, t, v);
- case EVIOCSFF:
- if (copy_from_user(&effect, p, sizeof(effect)))
- return -EFAULT;
+ case EVIOCSFF:
+ if (copy_from_user(&effect, p, sizeof(effect)))
+ return -EFAULT;
- error = input_ff_upload(dev, &effect, file);
+ error = input_ff_upload(dev, &effect, file);
- if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
- return -EFAULT;
+ if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
+ return -EFAULT;
- return error;
+ return error;
- case EVIOCRMFF:
- return input_ff_erase(dev, (int)(unsigned long) p, file);
+ case EVIOCRMFF:
+ return input_ff_erase(dev, (int)(unsigned long) p, file);
- case EVIOCGEFFECTS:
- i = test_bit(EV_FF, dev->evbit) ? dev->ff->max_effects : 0;
- if (put_user(i, ip))
- return -EFAULT;
- return 0;
+ case EVIOCGEFFECTS:
+ i = test_bit(EV_FF, dev->evbit) ?
+ dev->ff->max_effects : 0;
+ if (put_user(i, ip))
+ return -EFAULT;
+ return 0;
- case EVIOCGRAB:
- if (p) {
- if (evdev->grab)
- return -EBUSY;
- if (input_grab_device(&evdev->handle))
- return -EBUSY;
- evdev->grab = client;
- return 0;
- } else {
- if (evdev->grab != client)
- return -EINVAL;
- input_release_device(&evdev->handle);
- evdev->grab = NULL;
- return 0;
+ case EVIOCGRAB:
+ if (p)
+ return evdev_grab(evdev, client);
+ else
+ return evdev_ungrab(evdev, client);
+
+ default:
+
+ if (_IOC_TYPE(cmd) != 'E')
+ return -EINVAL;
+
+ if (_IOC_DIR(cmd) == _IOC_READ) {
+
+ if ((_IOC_NR(cmd) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0, 0))) {
+
+ unsigned long *bits;
+ int len;
+
+ switch (_IOC_NR(cmd) & EV_MAX) {
+
+ case 0: bits = dev->evbit; len = EV_MAX; break;
+ case EV_KEY: bits = dev->keybit; len = KEY_MAX; break;
+ case EV_REL: bits = dev->relbit; len = REL_MAX; break;
+ case EV_ABS: bits = dev->absbit; len = ABS_MAX; break;
+ case EV_MSC: bits = dev->mscbit; len = MSC_MAX; break;
+ case EV_LED: bits = dev->ledbit; len = LED_MAX; break;
+ case EV_SND: bits = dev->sndbit; len = SND_MAX; break;
+ case EV_FF: bits = dev->ffbit; len = FF_MAX; break;
+ case EV_SW: bits = dev->swbit; len = SW_MAX; break;
+ default: return -EINVAL;
+ }
+ return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
}
- default:
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGKEY(0)))
+ return bits_to_user(dev->key, KEY_MAX, _IOC_SIZE(cmd),
+ p, compat_mode);
- if (_IOC_TYPE(cmd) != 'E')
- return -EINVAL;
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGLED(0)))
+ return bits_to_user(dev->led, LED_MAX, _IOC_SIZE(cmd),
+ p, compat_mode);
- if (_IOC_DIR(cmd) == _IOC_READ) {
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGSND(0)))
+ return bits_to_user(dev->snd, SND_MAX, _IOC_SIZE(cmd),
+ p, compat_mode);
- if ((_IOC_NR(cmd) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0,0))) {
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGSW(0)))
+ return bits_to_user(dev->sw, SW_MAX, _IOC_SIZE(cmd),
+ p, compat_mode);
- unsigned long *bits;
- int len;
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
+ return str_to_user(dev->name, _IOC_SIZE(cmd), p);
- switch (_IOC_NR(cmd) & EV_MAX) {
- case 0: bits = dev->evbit; len = EV_MAX; break;
- case EV_KEY: bits = dev->keybit; len = KEY_MAX; break;
- case EV_REL: bits = dev->relbit; len = REL_MAX; break;
- case EV_ABS: bits = dev->absbit; len = ABS_MAX; break;
- case EV_MSC: bits = dev->mscbit; len = MSC_MAX; break;
- case EV_LED: bits = dev->ledbit; len = LED_MAX; break;
- case EV_SND: bits = dev->sndbit; len = SND_MAX; break;
- case EV_FF: bits = dev->ffbit; len = FF_MAX; break;
- case EV_SW: bits = dev->swbit; len = SW_MAX; break;
- default: return -EINVAL;
- }
- return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
- }
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGPHYS(0)))
+ return str_to_user(dev->phys, _IOC_SIZE(cmd), p);
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGKEY(0)))
- return bits_to_user(dev->key, KEY_MAX, _IOC_SIZE(cmd),
- p, compat_mode);
+ if (_IOC_NR(cmd) == _IOC_NR(EVIOCGUNIQ(0)))
+ return str_to_user(dev->uniq, _IOC_SIZE(cmd), p);
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGLED(0)))
- return bits_to_user(dev->led, LED_MAX, _IOC_SIZE(cmd),
- p, compat_mode);
+ if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) {
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGSND(0)))
- return bits_to_user(dev->snd, SND_MAX, _IOC_SIZE(cmd),
- p, compat_mode);
+ t = _IOC_NR(cmd) & ABS_MAX;
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGSW(0)))
- return bits_to_user(dev->sw, SW_MAX, _IOC_SIZE(cmd),
- p, compat_mode);
+ abs.value = dev->abs[t];
+ abs.minimum = dev->absmin[t];
+ abs.maximum = dev->absmax[t];
+ abs.fuzz = dev->absfuzz[t];
+ abs.flat = dev->absflat[t];
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
- return str_to_user(dev->name, _IOC_SIZE(cmd), p);
+ if (copy_to_user(p, &abs, sizeof(struct input_absinfo)))
+ return -EFAULT;
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGPHYS(0)))
- return str_to_user(dev->phys, _IOC_SIZE(cmd), p);
+ return 0;
+ }
- if (_IOC_NR(cmd) == _IOC_NR(EVIOCGUNIQ(0)))
- return str_to_user(dev->uniq, _IOC_SIZE(cmd), p);
+ }
- if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) {
+ if (_IOC_DIR(cmd) == _IOC_WRITE) {
- t = _IOC_NR(cmd) & ABS_MAX;
+ if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
- abs.value = dev->abs[t];
- abs.minimum = dev->absmin[t];
- abs.maximum = dev->absmax[t];
- abs.fuzz = dev->absfuzz[t];
- abs.flat = dev->absflat[t];
+ t = _IOC_NR(cmd) & ABS_MAX;
- if (copy_to_user(p, &abs, sizeof(struct input_absinfo)))
- return -EFAULT;
+ if (copy_from_user(&abs, p,
+ sizeof(struct input_absinfo)))
+ return -EFAULT;
+
+ /*
+ * Take event lock to ensure that we are not
+ * changing device parameters in the middle
+ * of event.
+ */
+ spin_lock_irq(&dev->event_lock);
+
+ dev->abs[t] = abs.value;
+ dev->absmin[t] = abs.minimum;
+ dev->absmax[t] = abs.maximum;
+ dev->absfuzz[t] = abs.fuzz;
+ dev->absflat[t] = abs.flat;
- return 0;
- }
+ spin_unlock_irq(&dev->event_lock);
+ return 0;
}
+ }
+ }
+ return -EINVAL;
+}
- if (_IOC_DIR(cmd) == _IOC_WRITE) {
-
- if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
+static long evdev_ioctl_handler(struct file *file, unsigned int cmd,
+ void __user *p, int compat_mode)
+{
+ struct evdev_client *client = file->private_data;
+ struct evdev *evdev = client->evdev;
+ int retval;
- t = _IOC_NR(cmd) & ABS_MAX;
+ retval = mutex_lock_interruptible(&evdev->mutex);
+ if (retval)
+ return retval;
- if (copy_from_user(&abs, p, sizeof(struct input_absinfo)))
- return -EFAULT;
+ if (!evdev->exist) {
+ retval = -ENODEV;
+ goto out;
+ }
- dev->abs[t] = abs.value;
- dev->absmin[t] = abs.minimum;
- dev->absmax[t] = abs.maximum;
- dev->absfuzz[t] = abs.fuzz;
- dev->absflat[t] = abs.flat;
+ retval = evdev_do_ioctl(file, cmd, p, compat_mode);
- return 0;
- }
- }
- }
- return -EINVAL;
+ out:
+ mutex_unlock(&evdev->mutex);
+ return retval;
}
static long evdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
@@ -609,27 +792,79 @@ static long evdev_ioctl(struct file *fil
}
#ifdef CONFIG_COMPAT
-static long evdev_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
+static long evdev_ioctl_compat(struct file *file,
+ unsigned int cmd, unsigned long arg)
{
return evdev_ioctl_handler(file, cmd, compat_ptr(arg), 1);
}
#endif
static const struct file_operations evdev_fops = {
- .owner = THIS_MODULE,
- .read = evdev_read,
- .write = evdev_write,
- .poll = evdev_poll,
- .open = evdev_open,
- .release = evdev_release,
- .unlocked_ioctl = evdev_ioctl,
+ .owner = THIS_MODULE,
+ .read = evdev_read,
+ .write = evdev_write,
+ .poll = evdev_poll,
+ .open = evdev_open,
+ .release = evdev_release,
+ .unlocked_ioctl = evdev_ioctl,
#ifdef CONFIG_COMPAT
- .compat_ioctl = evdev_ioctl_compat,
+ .compat_ioctl = evdev_ioctl_compat,
#endif
- .fasync = evdev_fasync,
- .flush = evdev_flush
+ .fasync = evdev_fasync,
+ .flush = evdev_flush
};
+static int evdev_install_chrdev(struct evdev *evdev)
+{
+ /*
+ * No need to do any locking here as calls to connect and
+ * disconnect are serialized by the input core
+ */
+ evdev_table[evdev->minor] = evdev;
+ return 0;
+}
+
+static void evdev_remove_chrdev(struct evdev *evdev)
+{
+ /*
+ * Lock evdev table to prevent race with evdev_open()
+ */
+ mutex_lock(&evdev_table_mutex);
+ evdev_table[evdev->minor] = NULL;
+ mutex_unlock(&evdev_table_mutex);
+}
+
+/*
+ * Mark device non-existent. This disables writes, ioctls and
+ * prevents new users from opening the device. Already posted
+ * blocking reads will stay, however new ones will fail.
+ */
+static void evdev_mark_dead(struct evdev *evdev)
+{
+ mutex_lock(&evdev->mutex);
+ evdev->exist = 0;
+ mutex_unlock(&evdev->mutex);
+}
+
+static void evdev_cleanup(struct evdev *evdev)
+{
+ struct input_handle *handle = &evdev->handle;
+
+ evdev_mark_dead(evdev);
+ evdev_hangup(evdev);
+ evdev_remove_chrdev(evdev);
+
+ /* evdev is marked dead so no one else accesses evdev->open */
+ if (evdev->open) {
+ input_flush_device(handle, NULL);
+ input_close_device(handle);
+ }
+}
+
+/*
+ * Create new evdev device. Note that input core serializes calls
+ * to connect and disconnect so we don't need to lock evdev_table here.
+ */
static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
const struct input_device_id *id)
{
@@ -637,7 +872,10 @@ static int evdev_connect(struct input_ha
int minor;
int error;
- for (minor = 0; minor < EVDEV_MINORS && evdev_table[minor]; minor++);
+ for (minor = 0; minor < EVDEV_MINORS; minor++)
+ if (!evdev_table[minor])
+ break;
+
if (minor == EVDEV_MINORS) {
printk(KERN_ERR "evdev: no more free evdev devices\n");
return -ENFILE;
@@ -648,38 +886,44 @@ static int evdev_connect(struct input_ha
return -ENOMEM;
INIT_LIST_HEAD(&evdev->client_list);
+ spin_lock_init(&evdev->client_lock);
+ mutex_init(&evdev->mutex);
init_waitqueue_head(&evdev->wait);
+ snprintf(evdev->name, sizeof(evdev->name), "event%d", minor);
evdev->exist = 1;
evdev->minor = minor;
+
evdev->handle.dev = dev;
evdev->handle.name = evdev->name;
evdev->handle.handler = handler;
evdev->handle.private = evdev;
- snprintf(evdev->name, sizeof(evdev->name), "event%d", minor);
- snprintf(evdev->dev.bus_id, sizeof(evdev->dev.bus_id),
- "event%d", minor);
+ strlcpy(evdev->dev.bus_id, evdev->name, sizeof(evdev->dev.bus_id));
+ evdev->dev.devt = MKDEV(INPUT_MAJOR, EVDEV_MINOR_BASE + minor);
evdev->dev.class = &input_class;
evdev->dev.parent = &dev->dev;
- evdev->dev.devt = MKDEV(INPUT_MAJOR, EVDEV_MINOR_BASE + minor);
evdev->dev.release = evdev_free;
device_initialize(&evdev->dev);
- evdev_table[minor] = evdev;
-
- error = device_add(&evdev->dev);
+ error = input_register_handle(&evdev->handle);
if (error)
goto err_free_evdev;
- error = input_register_handle(&evdev->handle);
+ error = evdev_install_chrdev(evdev);
+ if (error)
+ goto err_unregister_handle;
+
+ error = device_add(&evdev->dev);
if (error)
- goto err_delete_evdev;
+ goto err_cleanup_evdev;
return 0;
- err_delete_evdev:
- device_del(&evdev->dev);
+ err_cleanup_evdev:
+ evdev_cleanup(evdev);
+ err_unregister_handle:
+ input_unregister_handle(&evdev->handle);
err_free_evdev:
put_device(&evdev->dev);
return error;
@@ -688,21 +932,10 @@ static int evdev_connect(struct input_ha
static void evdev_disconnect(struct input_handle *handle)
{
struct evdev *evdev = handle->private;
- struct evdev_client *client;
- input_unregister_handle(handle);
device_del(&evdev->dev);
-
- evdev->exist = 0;
-
- if (evdev->open) {
- input_flush_device(handle, NULL);
- input_close_device(handle);
- list_for_each_entry(client, &evdev->client_list, node)
- kill_fasync(&client->fasync, SIGIO, POLL_HUP);
- wake_up_interruptible(&evdev->wait);
- }
-
+ evdev_cleanup(evdev);
+ input_unregister_handle(handle);
put_device(&evdev->dev);
}
@@ -714,13 +947,13 @@ static const struct input_device_id evde
MODULE_DEVICE_TABLE(input, evdev_ids);
static struct input_handler evdev_handler = {
- .event = evdev_event,
- .connect = evdev_connect,
- .disconnect = evdev_disconnect,
- .fops = &evdev_fops,
- .minor = EVDEV_MINOR_BASE,
- .name = "evdev",
- .id_table = evdev_ids,
+ .event = evdev_event,
+ .connect = evdev_connect,
+ .disconnect = evdev_disconnect,
+ .fops = &evdev_fops,
+ .minor = EVDEV_MINOR_BASE,
+ .name = "evdev",
+ .id_table = evdev_ids,
};
static int __init evdev_init(void)
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC/RFT 3/5] Input: tsdev - implement proper locking
2007-07-24 4:45 [RFC/RFT 0/5] Input locking patches Dmitry Torokhov
2007-07-24 4:45 ` [RFC/RFT 1/5] Input: implement proper locking in input core Dmitry Torokhov
2007-07-24 4:45 ` [RFC/RFT 2/5] evdev - implement proper locking Dmitry Torokhov
@ 2007-07-24 4:45 ` Dmitry Torokhov
2007-07-24 4:45 ` [RFC/RFT 4/5] Input: mousedev " Dmitry Torokhov
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-24 4:45 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
[-- Attachment #1: tsdev-locking.patch --]
[-- Type: text/plain, Size: 16116 bytes --]
Input: tsdev - implement proper locking
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/tsdev.c | 392 +++++++++++++++++++++++++++++++++++---------------
1 files changed, 278 insertions(+), 114 deletions(-)
Index: work/drivers/input/tsdev.c
===================================================================
--- work.orig/drivers/input/tsdev.c
+++ work/drivers/input/tsdev.c
@@ -112,6 +112,8 @@ struct tsdev {
struct input_handle handle;
wait_queue_head_t wait;
struct list_head client_list;
+ spinlock_t client_lock; /* protects client_list */
+ struct mutex mutex;
struct device dev;
int x, y, pressure;
@@ -122,8 +124,9 @@ struct tsdev_client {
struct fasync_struct *fasync;
struct list_head node;
struct tsdev *tsdev;
+ struct ts_event buffer[TSDEV_BUFFER_SIZE];
int head, tail;
- struct ts_event event[TSDEV_BUFFER_SIZE];
+ spinlock_t buffer_lock; /* protects access to buffer, head and tail */
int raw;
};
@@ -137,6 +140,7 @@ struct tsdev_client {
#define TS_SET_CAL _IOW(IOC_H3600_TS_MAGIC, 11, struct ts_calibration)
static struct tsdev *tsdev_table[TSDEV_MINORS/2];
+static DEFINE_MUTEX(tsdev_table_mutex);
static int tsdev_fasync(int fd, struct file *file, int on)
{
@@ -144,9 +148,91 @@ static int tsdev_fasync(int fd, struct f
int retval;
retval = fasync_helper(fd, file, on, &client->fasync);
+
return retval < 0 ? retval : 0;
}
+static void tsdev_free(struct device *dev)
+{
+ struct tsdev *tsdev = container_of(dev, struct tsdev, dev);
+
+ kfree(tsdev);
+}
+
+static void tsdev_attach_client(struct tsdev *tsdev, struct tsdev_client *client)
+{
+ spin_lock(&tsdev->client_lock);
+ list_add_tail_rcu(&client->node, &tsdev->client_list);
+ spin_unlock(&tsdev->client_lock);
+ synchronize_sched();
+}
+
+static void tsdev_detach_client(struct tsdev *tsdev, struct tsdev_client *client)
+{
+ spin_lock(&tsdev->client_lock);
+ list_del_rcu(&client->node);
+ spin_unlock(&tsdev->client_lock);
+ synchronize_sched();
+}
+
+static int tsdev_open_device(struct tsdev *tsdev)
+{
+ int retval;
+
+ retval = mutex_lock_interruptible(&tsdev->mutex);
+ if (retval)
+ return retval;
+
+ if (!tsdev->exist)
+ retval = -ENODEV;
+ else if (!tsdev->open++)
+ retval = input_open_device(&tsdev->handle);
+
+ mutex_unlock(&tsdev->mutex);
+ return retval;
+}
+
+static void tsdev_close_device(struct tsdev *tsdev)
+{
+ mutex_lock(&tsdev->mutex);
+
+ if (tsdev->exist && !--tsdev->open)
+ input_close_device(&tsdev->handle);
+
+ mutex_unlock(&tsdev->mutex);
+}
+
+/*
+ * Wake up users waiting for IO so they can disconnect from
+ * dead device.
+ */
+static void tsdev_hangup(struct tsdev *tsdev)
+{
+ struct tsdev_client *client;
+
+ spin_lock(&tsdev->client_lock);
+ list_for_each_entry(client, &tsdev->client_list, node)
+ kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+ spin_unlock(&tsdev->client_lock);
+
+ wake_up_interruptible(&tsdev->wait);
+}
+
+static int tsdev_release(struct inode *inode, struct file *file)
+{
+ struct tsdev_client *client = file->private_data;
+ struct tsdev *tsdev = client->tsdev;
+
+ tsdev_fasync(-1, file, 0);
+ tsdev_detach_client(tsdev, client);
+ kfree(client);
+
+ tsdev_close_device(tsdev);
+ put_device(&tsdev->dev);
+
+ return 0;
+}
+
static int tsdev_open(struct inode *inode, struct file *file)
{
int i = iminor(inode) - TSDEV_MINOR_BASE;
@@ -161,11 +247,16 @@ static int tsdev_open(struct inode *inod
if (i >= TSDEV_MINORS)
return -ENODEV;
+ error = mutex_lock_interruptible(&tsdev_table_mutex);
+ if (error)
+ return error;
tsdev = tsdev_table[i & TSDEV_MINOR_MASK];
- if (!tsdev || !tsdev->exist)
- return -ENODEV;
+ if (tsdev)
+ get_device(&tsdev->dev);
+ mutex_unlock(&tsdev_table_mutex);
- get_device(&tsdev->dev);
+ if (!tsdev)
+ return -ENODEV;
client = kzalloc(sizeof(struct tsdev_client), GFP_KERNEL);
if (!client) {
@@ -173,51 +264,42 @@ static int tsdev_open(struct inode *inod
goto err_put_tsdev;
}
+ spin_lock_init(&client->buffer_lock);
client->tsdev = tsdev;
- client->raw = (i >= TSDEV_MINORS / 2) ? 1 : 0;
- list_add_tail(&client->node, &tsdev->client_list);
+ client->raw = i >= TSDEV_MINORS / 2;
+ tsdev_attach_client(tsdev, client);
- if (!tsdev->open++ && tsdev->exist) {
- error = input_open_device(&tsdev->handle);
- if (error)
- goto err_free_client;
- }
+ error = tsdev_open_device(tsdev);
+ if (error)
+ goto err_free_client;
file->private_data = client;
return 0;
err_free_client:
- list_del(&client->node);
+ tsdev_detach_client(tsdev, client);
kfree(client);
err_put_tsdev:
put_device(&tsdev->dev);
return error;
}
-static void tsdev_free(struct device *dev)
+static int tsdev_fetch_next_event(struct tsdev_client *client,
+ struct ts_event *event)
{
- struct tsdev *tsdev = container_of(dev, struct tsdev, dev);
+ int have_event;
- tsdev_table[tsdev->minor] = NULL;
- kfree(tsdev);
-}
-
-static int tsdev_release(struct inode *inode, struct file *file)
-{
- struct tsdev_client *client = file->private_data;
- struct tsdev *tsdev = client->tsdev;
-
- tsdev_fasync(-1, file, 0);
+ spin_lock_irq(&client->buffer_lock);
- list_del(&client->node);
- kfree(client);
-
- if (!--tsdev->open && tsdev->exist)
- input_close_device(&tsdev->handle);
+ have_event = client->head != client->tail;
+ if (have_event) {
+ *event = client->buffer[client->tail++];
+ client->tail &= TSDEV_BUFFER_SIZE - 1;
+ }
- put_device(&tsdev->dev);
+ spin_unlock_irq(&client->buffer_lock);
- return 0;
+ return have_event;
}
static ssize_t tsdev_read(struct file *file, char __user *buffer, size_t count,
@@ -225,9 +307,11 @@ static ssize_t tsdev_read(struct file *f
{
struct tsdev_client *client = file->private_data;
struct tsdev *tsdev = client->tsdev;
- int retval = 0;
+ struct ts_event event;
+ int retval;
- if (client->head == client->tail && tsdev->exist && (file->f_flags & O_NONBLOCK))
+ if (client->head == client->tail && tsdev->exist &&
+ (file->f_flags & O_NONBLOCK))
return -EAGAIN;
retval = wait_event_interruptible(tsdev->wait,
@@ -238,13 +322,14 @@ static ssize_t tsdev_read(struct file *f
if (!tsdev->exist)
return -ENODEV;
- while (client->head != client->tail &&
- retval + sizeof (struct ts_event) <= count) {
- if (copy_to_user (buffer + retval, client->event + client->tail,
- sizeof (struct ts_event)))
+ while (retval + sizeof(struct ts_event) <= count &&
+ tsdev_fetch_next_event(client, &event)) {
+
+ if (copy_to_user(buffer + retval, &event,
+ sizeof(struct ts_event)))
return -EFAULT;
- client->tail = (client->tail + 1) & (TSDEV_BUFFER_SIZE - 1);
- retval += sizeof (struct ts_event);
+
+ retval += sizeof(struct ts_event);
}
return retval;
@@ -261,14 +346,23 @@ static unsigned int tsdev_poll(struct fi
(tsdev->exist ? 0 : (POLLHUP | POLLERR));
}
-static int tsdev_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+static long tsdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
struct tsdev_client *client = file->private_data;
struct tsdev *tsdev = client->tsdev;
int retval = 0;
+ retval = mutex_lock_interruptible(&tsdev->mutex);
+ if (retval)
+ return retval;
+
+ if (!tsdev->exist) {
+ retval = -ENODEV;
+ goto out;
+ }
+
switch (cmd) {
+
case TS_GET_CAL:
if (copy_to_user((void __user *)arg, &tsdev->cal,
sizeof (struct ts_calibration)))
@@ -277,7 +371,7 @@ static int tsdev_ioctl(struct inode *ino
case TS_SET_CAL:
if (copy_from_user(&tsdev->cal, (void __user *)arg,
- sizeof (struct ts_calibration)))
+ sizeof(struct ts_calibration)))
retval = -EFAULT;
break;
@@ -286,29 +380,79 @@ static int tsdev_ioctl(struct inode *ino
break;
}
+ out:
+ mutex_unlock(&tsdev->mutex);
return retval;
}
static const struct file_operations tsdev_fops = {
- .owner = THIS_MODULE,
- .open = tsdev_open,
- .release = tsdev_release,
- .read = tsdev_read,
- .poll = tsdev_poll,
- .fasync = tsdev_fasync,
- .ioctl = tsdev_ioctl,
+ .owner = THIS_MODULE,
+ .open = tsdev_open,
+ .release = tsdev_release,
+ .read = tsdev_read,
+ .poll = tsdev_poll,
+ .fasync = tsdev_fasync,
+ .unlocked_ioctl = tsdev_ioctl,
};
+static void tsdev_pass_event(struct tsdev *tsdev, struct tsdev_client *client,
+ int x, int y, int pressure, int millisecs)
+{
+ struct ts_event *event;
+ int tmp;
+
+ /* Interrupts are already disabled, just acquire the lock */
+ spin_lock(&client->buffer_lock);
+
+ event = &client->buffer[client->head++];
+ client->head &= TSDEV_BUFFER_SIZE - 1;
+
+ /* Calibration */
+ if (!client->raw) {
+ x = ((x * tsdev->cal.xscale) >> 8) + tsdev->cal.xtrans;
+ y = ((y * tsdev->cal.yscale) >> 8) + tsdev->cal.ytrans;
+ if (tsdev->cal.xyswap) {
+ tmp = x; x = y; y = tmp;
+ }
+ }
+
+ event->millisecs = millisecs;
+ event->x = x;
+ event->y = y;
+ event->pressure = pressure;
+
+ spin_unlock(&client->buffer_lock);
+
+ kill_fasync(&client->fasync, SIGIO, POLL_IN);
+}
+
+static void tsdev_distribute_event(struct tsdev *tsdev)
+{
+ struct tsdev_client *client;
+ struct timeval time;
+ int millisecs;
+
+ do_gettimeofday(&time);
+ millisecs = time.tv_usec / 1000;
+
+ list_for_each_entry_rcu(client, &tsdev->client_list, node)
+ tsdev_pass_event(tsdev, client,
+ tsdev->x, tsdev->y,
+ tsdev->pressure, millisecs);
+}
+
static void tsdev_event(struct input_handle *handle, unsigned int type,
unsigned int code, int value)
{
struct tsdev *tsdev = handle->private;
- struct tsdev_client *client;
- struct timeval time;
+ struct input_dev *dev = handle->dev;
+ int wake_up_readers = 0;
switch (type) {
+
case EV_ABS:
switch (code) {
+
case ABS_X:
tsdev->x = value;
break;
@@ -318,9 +462,9 @@ static void tsdev_event(struct input_han
break;
case ABS_PRESSURE:
- if (value > handle->dev->absmax[ABS_PRESSURE])
- value = handle->dev->absmax[ABS_PRESSURE];
- value -= handle->dev->absmin[ABS_PRESSURE];
+ if (value > dev->absmax[ABS_PRESSURE])
+ value = dev->absmax[ABS_PRESSURE];
+ value -= dev->absmin[ABS_PRESSURE];
if (value < 0)
value = 0;
tsdev->pressure = value;
@@ -330,6 +474,7 @@ static void tsdev_event(struct input_han
case EV_REL:
switch (code) {
+
case REL_X:
tsdev->x += value;
if (tsdev->x < 0)
@@ -351,6 +496,7 @@ static void tsdev_event(struct input_han
case EV_KEY:
if (code == BTN_TOUCH || code == BTN_MOUSE) {
switch (value) {
+
case 0:
tsdev->pressure = 0;
break;
@@ -362,49 +508,71 @@ static void tsdev_event(struct input_han
}
}
break;
+
+ case EV_SYN:
+ if (code == SYN_REPORT) {
+ tsdev_distribute_event(tsdev);
+ wake_up_readers = 1;
+ }
+ break;
}
- if (type != EV_SYN || code != SYN_REPORT)
- return;
+ if (wake_up_readers)
+ wake_up_interruptible(&tsdev->wait);
+}
- list_for_each_entry(client, &tsdev->client_list, node) {
- int x, y, tmp;
+static int tsdev_install_chrdev(struct tsdev *tsdev)
+{
+ tsdev_table[tsdev->minor] = tsdev;
+ return 0;
+}
- do_gettimeofday(&time);
- client->event[client->head].millisecs = time.tv_usec / 1000;
- client->event[client->head].pressure = tsdev->pressure;
-
- x = tsdev->x;
- y = tsdev->y;
-
- /* Calibration */
- if (!client->raw) {
- x = ((x * tsdev->cal.xscale) >> 8) + tsdev->cal.xtrans;
- y = ((y * tsdev->cal.yscale) >> 8) + tsdev->cal.ytrans;
- if (tsdev->cal.xyswap) {
- tmp = x; x = y; y = tmp;
- }
- }
+static void tsdev_remove_chrdev(struct tsdev *tsdev)
+{
+ mutex_lock(&tsdev_table_mutex);
+ tsdev_table[tsdev->minor] = NULL;
+ mutex_unlock(&tsdev_table_mutex);
+}
- client->event[client->head].x = x;
- client->event[client->head].y = y;
- client->head = (client->head + 1) & (TSDEV_BUFFER_SIZE - 1);
- kill_fasync(&client->fasync, SIGIO, POLL_IN);
- }
- wake_up_interruptible(&tsdev->wait);
+/*
+ * Mark device non-existant. This disables writes, ioctls and
+ * prevents new users from opening the device. Already posted
+ * blocking reads will stay, however new ones will fail.
+ */
+static void tsdev_mark_dead(struct tsdev *tsdev)
+{
+ mutex_lock(&tsdev->mutex);
+ tsdev->exist = 0;
+ mutex_unlock(&tsdev->mutex);
+}
+
+static void tsdev_cleanup(struct tsdev *tsdev)
+{
+ struct input_handle *handle = &tsdev->handle;
+
+ tsdev_mark_dead(tsdev);
+ tsdev_hangup(tsdev);
+ tsdev_remove_chrdev(tsdev);
+
+ /* tsdev is marked dead so noone else accesses tsdev->open */
+ if (tsdev->open)
+ input_close_device(handle);
}
static int tsdev_connect(struct input_handler *handler, struct input_dev *dev,
const struct input_device_id *id)
{
struct tsdev *tsdev;
- int minor, delta;
+ int delta;
+ int minor;
int error;
- for (minor = 0; minor < TSDEV_MINORS / 2 && tsdev_table[minor]; minor++);
- if (minor >= TSDEV_MINORS / 2) {
- printk(KERN_ERR
- "tsdev: You have way too many touchscreens\n");
+ for (minor = 0; minor < TSDEV_MINORS / 2; minor++)
+ if (!tsdev_table[minor])
+ break;
+
+ if (minor == TSDEV_MINORS) {
+ printk(KERN_ERR "tsdev: no more free tsdev devices\n");
return -ENFILE;
}
@@ -413,15 +581,18 @@ static int tsdev_connect(struct input_ha
return -ENOMEM;
INIT_LIST_HEAD(&tsdev->client_list);
+ spin_lock_init(&tsdev->client_lock);
+ mutex_init(&tsdev->mutex);
init_waitqueue_head(&tsdev->wait);
+ snprintf(tsdev->name, sizeof(tsdev->name), "ts%d", minor);
tsdev->exist = 1;
tsdev->minor = minor;
+
tsdev->handle.dev = dev;
tsdev->handle.name = tsdev->name;
tsdev->handle.handler = handler;
tsdev->handle.private = tsdev;
- snprintf(tsdev->name, sizeof(tsdev->name), "ts%d", minor);
/* Precompute the rough calibration matrix */
delta = dev->absmax [ABS_X] - dev->absmin [ABS_X] + 1;
@@ -436,28 +607,31 @@ static int tsdev_connect(struct input_ha
tsdev->cal.yscale = (yres << 8) / delta;
tsdev->cal.ytrans = - ((dev->absmin [ABS_Y] * tsdev->cal.yscale) >> 8);
- snprintf(tsdev->dev.bus_id, sizeof(tsdev->dev.bus_id),
- "ts%d", minor);
+ strlcpy(tsdev->dev.bus_id, tsdev->name, sizeof(tsdev->dev.bus_id));
+ tsdev->dev.devt = MKDEV(INPUT_MAJOR, TSDEV_MINOR_BASE + minor);
tsdev->dev.class = &input_class;
tsdev->dev.parent = &dev->dev;
- tsdev->dev.devt = MKDEV(INPUT_MAJOR, TSDEV_MINOR_BASE + minor);
tsdev->dev.release = tsdev_free;
device_initialize(&tsdev->dev);
- tsdev_table[minor] = tsdev;
-
- error = device_add(&tsdev->dev);
+ error = input_register_handle(&tsdev->handle);
if (error)
goto err_free_tsdev;
- error = input_register_handle(&tsdev->handle);
+ error = tsdev_install_chrdev(tsdev);
if (error)
- goto err_delete_tsdev;
+ goto err_unregister_handle;
+
+ error = device_add(&tsdev->dev);
+ if (error)
+ goto err_cleanup_tsdev;
return 0;
- err_delete_tsdev:
- device_del(&tsdev->dev);
+ err_cleanup_tsdev:
+ tsdev_cleanup(tsdev);
+ err_unregister_handle:
+ input_unregister_handle(&tsdev->handle);
err_free_tsdev:
put_device(&tsdev->dev);
return error;
@@ -466,20 +640,10 @@ static int tsdev_connect(struct input_ha
static void tsdev_disconnect(struct input_handle *handle)
{
struct tsdev *tsdev = handle->private;
- struct tsdev_client *client;
- input_unregister_handle(handle);
device_del(&tsdev->dev);
-
- tsdev->exist = 0;
-
- if (tsdev->open) {
- input_close_device(handle);
- list_for_each_entry(client, &tsdev->client_list, node)
- kill_fasync(&client->fasync, SIGIO, POLL_HUP);
- wake_up_interruptible(&tsdev->wait);
- }
-
+ tsdev_cleanup(tsdev);
+ input_unregister_handle(handle);
put_device(&tsdev->dev);
}
@@ -510,13 +674,13 @@ static const struct input_device_id tsde
MODULE_DEVICE_TABLE(input, tsdev_ids);
static struct input_handler tsdev_handler = {
- .event = tsdev_event,
- .connect = tsdev_connect,
- .disconnect = tsdev_disconnect,
- .fops = &tsdev_fops,
- .minor = TSDEV_MINOR_BASE,
- .name = "tsdev",
- .id_table = tsdev_ids,
+ .event = tsdev_event,
+ .connect = tsdev_connect,
+ .disconnect = tsdev_disconnect,
+ .fops = &tsdev_fops,
+ .minor = TSDEV_MINOR_BASE,
+ .name = "tsdev",
+ .id_table = tsdev_ids,
};
static int __init tsdev_init(void)
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC/RFT 4/5] Input: mousedev - implement proper locking
2007-07-24 4:45 [RFC/RFT 0/5] Input locking patches Dmitry Torokhov
` (2 preceding siblings ...)
2007-07-24 4:45 ` [RFC/RFT 3/5] Input: tsdev " Dmitry Torokhov
@ 2007-07-24 4:45 ` Dmitry Torokhov
2007-07-24 4:45 ` [RFC/RFT 5/5] Input: joydev " Dmitry Torokhov
2007-07-27 22:25 ` [RFC/RFT 0/5] Input locking patches Indan Zupancic
5 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-24 4:45 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
[-- Attachment #1: mousedev-locking.patch --]
[-- Type: text/plain, Size: 31449 bytes --]
Input: mousedev - implement proper locking
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/mousedev.c | 736 +++++++++++++++++++++++++++++------------------
1 files changed, 464 insertions(+), 272 deletions(-)
Index: work/drivers/input/mousedev.c
===================================================================
--- work.orig/drivers/input/mousedev.c
+++ work/drivers/input/mousedev.c
@@ -61,9 +61,11 @@ struct mousedev {
int open;
int minor;
char name[16];
+ struct input_handle handle;
wait_queue_head_t wait;
struct list_head client_list;
- struct input_handle handle;
+ spinlock_t client_lock; /* protects client_list */
+ struct mutex mutex;
struct device dev;
struct list_head mixdev_node;
@@ -113,108 +115,137 @@ static unsigned char mousedev_imex_seq[]
static struct input_handler mousedev_handler;
static struct mousedev *mousedev_table[MOUSEDEV_MINORS];
+static DEFINE_MUTEX(mousedev_table_mutex);
static struct mousedev *mousedev_mix;
static LIST_HEAD(mousedev_mix_list);
+static void mixdev_open_devices(void);
+static void mixdev_close_devices(void);
+
#define fx(i) (mousedev->old_x[(mousedev->pkt_count - (i)) & 03])
#define fy(i) (mousedev->old_y[(mousedev->pkt_count - (i)) & 03])
-static void mousedev_touchpad_event(struct input_dev *dev, struct mousedev *mousedev, unsigned int code, int value)
+static void mousedev_touchpad_event(struct input_dev *dev,
+ struct mousedev *mousedev,
+ unsigned int code, int value)
{
int size, tmp;
enum { FRACTION_DENOM = 128 };
switch (code) {
- case ABS_X:
- fx(0) = value;
- if (mousedev->touch && mousedev->pkt_count >= 2) {
- size = dev->absmax[ABS_X] - dev->absmin[ABS_X];
- if (size == 0)
- size = 256 * 2;
- tmp = ((value - fx(2)) * (256 * FRACTION_DENOM)) / size;
- tmp += mousedev->frac_dx;
- mousedev->packet.dx = tmp / FRACTION_DENOM;
- mousedev->frac_dx = tmp - mousedev->packet.dx * FRACTION_DENOM;
- }
- break;
- case ABS_Y:
- fy(0) = value;
- if (mousedev->touch && mousedev->pkt_count >= 2) {
- /* use X size to keep the same scale */
- size = dev->absmax[ABS_X] - dev->absmin[ABS_X];
- if (size == 0)
- size = 256 * 2;
- tmp = -((value - fy(2)) * (256 * FRACTION_DENOM)) / size;
- tmp += mousedev->frac_dy;
- mousedev->packet.dy = tmp / FRACTION_DENOM;
- mousedev->frac_dy = tmp - mousedev->packet.dy * FRACTION_DENOM;
- }
- break;
+ case ABS_X:
+ fx(0) = value;
+ if (mousedev->touch && mousedev->pkt_count >= 2) {
+ size = dev->absmax[ABS_X] - dev->absmin[ABS_X];
+ if (size == 0)
+ size = 256 * 2;
+ tmp = ((value - fx(2)) * 256 * FRACTION_DENOM) / size;
+ tmp += mousedev->frac_dx;
+ mousedev->packet.dx = tmp / FRACTION_DENOM;
+ mousedev->frac_dx =
+ tmp - mousedev->packet.dx * FRACTION_DENOM;
+ }
+ break;
+
+ case ABS_Y:
+ fy(0) = value;
+ if (mousedev->touch && mousedev->pkt_count >= 2) {
+ /* use X size to keep the same scale */
+ size = dev->absmax[ABS_X] - dev->absmin[ABS_X];
+ if (size == 0)
+ size = 256 * 2;
+ tmp = -((value - fy(2)) * 256 * FRACTION_DENOM) / size;
+ tmp += mousedev->frac_dy;
+ mousedev->packet.dy = tmp / FRACTION_DENOM;
+ mousedev->frac_dy = tmp -
+ mousedev->packet.dy * FRACTION_DENOM;
+ }
+ break;
}
}
-static void mousedev_abs_event(struct input_dev *dev, struct mousedev *mousedev, unsigned int code, int value)
+static void mousedev_abs_event(struct input_dev *dev, struct mousedev *mousedev,
+ unsigned int code, int value)
{
int size;
switch (code) {
- case ABS_X:
- size = dev->absmax[ABS_X] - dev->absmin[ABS_X];
- if (size == 0)
- size = xres ? : 1;
- if (value > dev->absmax[ABS_X])
- value = dev->absmax[ABS_X];
- if (value < dev->absmin[ABS_X])
- value = dev->absmin[ABS_X];
- mousedev->packet.x = ((value - dev->absmin[ABS_X]) * xres) / size;
- mousedev->packet.abs_event = 1;
- break;
- case ABS_Y:
- size = dev->absmax[ABS_Y] - dev->absmin[ABS_Y];
- if (size == 0)
- size = yres ? : 1;
- if (value > dev->absmax[ABS_Y])
- value = dev->absmax[ABS_Y];
- if (value < dev->absmin[ABS_Y])
- value = dev->absmin[ABS_Y];
- mousedev->packet.y = yres - ((value - dev->absmin[ABS_Y]) * yres) / size;
- mousedev->packet.abs_event = 1;
- break;
+ case ABS_X:
+ size = dev->absmax[ABS_X] - dev->absmin[ABS_X];
+ if (size == 0)
+ size = xres ? : 1;
+ if (value > dev->absmax[ABS_X])
+ value = dev->absmax[ABS_X];
+ if (value < dev->absmin[ABS_X])
+ value = dev->absmin[ABS_X];
+ mousedev->packet.x =
+ ((value - dev->absmin[ABS_X]) * xres) / size;
+ mousedev->packet.abs_event = 1;
+ break;
+
+ case ABS_Y:
+ size = dev->absmax[ABS_Y] - dev->absmin[ABS_Y];
+ if (size == 0)
+ size = yres ? : 1;
+ if (value > dev->absmax[ABS_Y])
+ value = dev->absmax[ABS_Y];
+ if (value < dev->absmin[ABS_Y])
+ value = dev->absmin[ABS_Y];
+ mousedev->packet.y = yres -
+ ((value - dev->absmin[ABS_Y]) * yres) / size;
+ mousedev->packet.abs_event = 1;
+ break;
}
}
-static void mousedev_rel_event(struct mousedev *mousedev, unsigned int code, int value)
+static void mousedev_rel_event(struct mousedev *mousedev,
+ unsigned int code, int value)
{
switch (code) {
- case REL_X: mousedev->packet.dx += value; break;
- case REL_Y: mousedev->packet.dy -= value; break;
- case REL_WHEEL: mousedev->packet.dz -= value; break;
+ case REL_X:
+ mousedev->packet.dx += value;
+ break;
+
+ case REL_Y:
+ mousedev->packet.dy -= value;
+ break;
+
+ case REL_WHEEL:
+ mousedev->packet.dz -= value;
+ break;
}
}
-static void mousedev_key_event(struct mousedev *mousedev, unsigned int code, int value)
+static void mousedev_key_event(struct mousedev *mousedev,
+ unsigned int code, int value)
{
int index;
switch (code) {
- case BTN_TOUCH:
- case BTN_0:
- case BTN_LEFT: index = 0; break;
- case BTN_STYLUS:
- case BTN_1:
- case BTN_RIGHT: index = 1; break;
- case BTN_2:
- case BTN_FORWARD:
- case BTN_STYLUS2:
- case BTN_MIDDLE: index = 2; break;
- case BTN_3:
- case BTN_BACK:
- case BTN_SIDE: index = 3; break;
- case BTN_4:
- case BTN_EXTRA: index = 4; break;
- default: return;
+
+ case BTN_TOUCH:
+ case BTN_0:
+ case BTN_LEFT: index = 0; break;
+
+ case BTN_STYLUS:
+ case BTN_1:
+ case BTN_RIGHT: index = 1; break;
+
+ case BTN_2:
+ case BTN_FORWARD:
+ case BTN_STYLUS2:
+ case BTN_MIDDLE: index = 2; break;
+
+ case BTN_3:
+ case BTN_BACK:
+ case BTN_SIDE: index = 3; break;
+
+ case BTN_4:
+ case BTN_EXTRA: index = 4; break;
+
+ default: return;
}
if (value) {
@@ -226,19 +257,22 @@ static void mousedev_key_event(struct mo
}
}
-static void mousedev_notify_readers(struct mousedev *mousedev, struct mousedev_hw_data *packet)
+static void mousedev_notify_readers(struct mousedev *mousedev,
+ struct mousedev_hw_data *packet)
{
struct mousedev_client *client;
struct mousedev_motion *p;
- unsigned long flags;
+ unsigned int new_head;
int wake_readers = 0;
- list_for_each_entry(client, &mousedev->client_list, node) {
- spin_lock_irqsave(&client->packet_lock, flags);
+ list_for_each_entry_rcu(client, &mousedev->client_list, node) {
+
+ /* Just acquire the lock, interrupts already disabled */
+ spin_lock(&client->packet_lock);
p = &client->packets[client->head];
if (client->ready && p->buttons != mousedev->packet.buttons) {
- unsigned int new_head = (client->head + 1) % PACKET_QUEUE_LEN;
+ new_head = (client->head + 1) % PACKET_QUEUE_LEN;
if (new_head != client->tail) {
p = &client->packets[client->head = new_head];
memset(p, 0, sizeof(struct mousedev_motion));
@@ -253,19 +287,22 @@ static void mousedev_notify_readers(stru
}
client->pos_x += packet->dx;
- client->pos_x = client->pos_x < 0 ? 0 : (client->pos_x >= xres ? xres : client->pos_x);
+ client->pos_x = client->pos_x < 0 ?
+ 0 : (client->pos_x >= xres ? xres : client->pos_x);
client->pos_y += packet->dy;
- client->pos_y = client->pos_y < 0 ? 0 : (client->pos_y >= yres ? yres : client->pos_y);
+ client->pos_y = client->pos_y < 0 ?
+ 0 : (client->pos_y >= yres ? yres : client->pos_y);
p->dx += packet->dx;
p->dy += packet->dy;
p->dz += packet->dz;
p->buttons = mousedev->packet.buttons;
- if (p->dx || p->dy || p->dz || p->buttons != client->last_buttons)
+ if (p->dx || p->dy || p->dz ||
+ p->buttons != client->last_buttons)
client->ready = 1;
- spin_unlock_irqrestore(&client->packet_lock, flags);
+ spin_unlock(&client->packet_lock);
if (client->ready) {
kill_fasync(&client->fasync, SIGIO, POLL_IN);
@@ -281,7 +318,8 @@ static void mousedev_touchpad_touch(stru
{
if (!value) {
if (mousedev->touch &&
- time_before(jiffies, mousedev->touch + msecs_to_jiffies(tap_time))) {
+ time_before(jiffies,
+ mousedev->touch + msecs_to_jiffies(tap_time))) {
/*
* Toggle left button to emulate tap.
* We rely on the fact that mousedev_mix always has 0
@@ -290,7 +328,8 @@ static void mousedev_touchpad_touch(stru
set_bit(0, &mousedev->packet.buttons);
set_bit(0, &mousedev_mix->packet.buttons);
mousedev_notify_readers(mousedev, &mousedev_mix->packet);
- mousedev_notify_readers(mousedev_mix, &mousedev_mix->packet);
+ mousedev_notify_readers(mousedev_mix,
+ &mousedev_mix->packet);
clear_bit(0, &mousedev->packet.buttons);
clear_bit(0, &mousedev_mix->packet.buttons);
}
@@ -302,54 +341,61 @@ static void mousedev_touchpad_touch(stru
mousedev->touch = jiffies;
}
-static void mousedev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
+static void mousedev_event(struct input_handle *handle,
+ unsigned int type, unsigned int code, int value)
{
struct mousedev *mousedev = handle->private;
switch (type) {
- case EV_ABS:
- /* Ignore joysticks */
- if (test_bit(BTN_TRIGGER, handle->dev->keybit))
- return;
- if (test_bit(BTN_TOOL_FINGER, handle->dev->keybit))
- mousedev_touchpad_event(handle->dev, mousedev, code, value);
+ case EV_ABS:
+ /* Ignore joysticks */
+ if (test_bit(BTN_TRIGGER, handle->dev->keybit))
+ return;
+
+ if (test_bit(BTN_TOOL_FINGER, handle->dev->keybit))
+ mousedev_touchpad_event(handle->dev,
+ mousedev, code, value);
+ else
+ mousedev_abs_event(handle->dev, mousedev, code, value);
+
+ break;
+
+ case EV_REL:
+ mousedev_rel_event(mousedev, code, value);
+ break;
+
+ case EV_KEY:
+ if (value != 2) {
+ if (code == BTN_TOUCH &&
+ test_bit(BTN_TOOL_FINGER, handle->dev->keybit))
+ mousedev_touchpad_touch(mousedev, value);
else
- mousedev_abs_event(handle->dev, mousedev, code, value);
-
- break;
-
- case EV_REL:
- mousedev_rel_event(mousedev, code, value);
- break;
+ mousedev_key_event(mousedev, code, value);
+ }
+ break;
- case EV_KEY:
- if (value != 2) {
- if (code == BTN_TOUCH && test_bit(BTN_TOOL_FINGER, handle->dev->keybit))
- mousedev_touchpad_touch(mousedev, value);
- else
- mousedev_key_event(mousedev, code, value);
+ case EV_SYN:
+ if (code == SYN_REPORT) {
+ if (mousedev->touch) {
+ mousedev->pkt_count++;
+ /*
+ * Input system eats duplicate events,
+ * but we need all of them to do correct
+ * averaging so apply present one forward
+ */
+ fx(0) = fx(1);
+ fy(0) = fy(1);
}
- break;
-
- case EV_SYN:
- if (code == SYN_REPORT) {
- if (mousedev->touch) {
- mousedev->pkt_count++;
- /* Input system eats duplicate events, but we need all of them
- * to do correct averaging so apply present one forward
- */
- fx(0) = fx(1);
- fy(0) = fy(1);
- }
- mousedev_notify_readers(mousedev, &mousedev->packet);
- mousedev_notify_readers(mousedev_mix, &mousedev->packet);
+ mousedev_notify_readers(mousedev, &mousedev->packet);
+ mousedev_notify_readers(mousedev_mix, &mousedev->packet);
- mousedev->packet.dx = mousedev->packet.dy = mousedev->packet.dz = 0;
- mousedev->packet.abs_event = 0;
- }
- break;
+ mousedev->packet.dx = mousedev->packet.dy =
+ mousedev->packet.dz = 0;
+ mousedev->packet.abs_event = 0;
+ }
+ break;
}
}
@@ -367,41 +413,45 @@ static void mousedev_free(struct device
{
struct mousedev *mousedev = container_of(dev, struct mousedev, dev);
- mousedev_table[mousedev->minor] = NULL;
kfree(mousedev);
}
-static int mixdev_add_device(struct mousedev *mousedev)
+static int mousedev_open_device(struct mousedev *mousedev)
{
- int error;
-
- if (mousedev_mix->open) {
- error = input_open_device(&mousedev->handle);
- if (error)
- return error;
+ int retval;
- mousedev->open++;
- mousedev->mixdev_open = 1;
- }
+ retval = mutex_lock_interruptible(&mousedev->mutex);
+ if (retval)
+ return retval;
- get_device(&mousedev->dev);
- list_add_tail(&mousedev->mixdev_node, &mousedev_mix_list);
+ if (mousedev->minor == MOUSEDEV_MIX)
+ mixdev_open_devices();
+ else if (!mousedev->exist)
+ retval = -ENODEV;
+ else if (!mousedev->open++)
+ retval = input_open_device(&mousedev->handle);
- return 0;
+ mutex_unlock(&mousedev->mutex);
+ return retval;
}
-static void mixdev_remove_device(struct mousedev *mousedev)
+static void mousedev_close_device(struct mousedev *mousedev)
{
- if (mousedev->mixdev_open) {
- mousedev->mixdev_open = 0;
- if (!--mousedev->open && mousedev->exist)
- input_close_device(&mousedev->handle);
- }
+ mutex_lock(&mousedev->mutex);
- list_del_init(&mousedev->mixdev_node);
- put_device(&mousedev->dev);
+ if (mousedev->minor == MOUSEDEV_MIX)
+ mixdev_close_devices();
+ else if (mousedev->exist && !--mousedev->open)
+ input_close_device(&mousedev->handle);
+
+ mutex_unlock(&mousedev->mutex);
}
+/*
+ * Open all available devices so they can all be multiplexed in one.
+ * stream. Note that this function is called with mousedev_mix->mutex
+ * held.
+ */
static void mixdev_open_devices(void)
{
struct mousedev *mousedev;
@@ -411,16 +461,19 @@ static void mixdev_open_devices(void)
list_for_each_entry(mousedev, &mousedev_mix_list, mixdev_node) {
if (!mousedev->mixdev_open) {
- if (!mousedev->open && mousedev->exist)
- if (input_open_device(&mousedev->handle))
- continue;
+ if (mousedev_open_device(mousedev));
+ continue;
- mousedev->open++;
mousedev->mixdev_open = 1;
}
}
}
+/*
+ * Close all devices that were opened as part of multiplexed
+ * device. Note that this function is called with mousedev_mix->mutex
+ * held.
+ */
static void mixdev_close_devices(void)
{
struct mousedev *mousedev;
@@ -431,33 +484,50 @@ static void mixdev_close_devices(void)
list_for_each_entry(mousedev, &mousedev_mix_list, mixdev_node) {
if (mousedev->mixdev_open) {
mousedev->mixdev_open = 0;
- if (!--mousedev->open && mousedev->exist)
- input_close_device(&mousedev->handle);
+ mousedev_close_device(mousedev);
}
}
}
+
+static void mousedev_attach_client(struct mousedev *mousedev,
+ struct mousedev_client *client)
+{
+ spin_lock(&mousedev->client_lock);
+ list_add_tail_rcu(&client->node, &mousedev->client_list);
+ spin_unlock(&mousedev->client_lock);
+ /*
+ * We don't use synchronize_rcu() here because read-size
+ * critical section is protected by a spinlock (dev->event_lock)
+ * instead of rcu_read_lock().
+ */
+ synchronize_sched();
+}
+
+static void mousedev_detach_client(struct mousedev *mousedev,
+ struct mousedev_client *client)
+{
+ spin_lock(&mousedev->client_lock);
+ list_del_rcu(&client->node);
+ spin_unlock(&mousedev->client_lock);
+ synchronize_sched();
+}
+
static int mousedev_release(struct inode *inode, struct file *file)
{
struct mousedev_client *client = file->private_data;
struct mousedev *mousedev = client->mousedev;
mousedev_fasync(-1, file, 0);
-
- list_del(&client->node);
+ mousedev_detach_client(mousedev, client);
kfree(client);
- if (mousedev->minor == MOUSEDEV_MIX)
- mixdev_close_devices();
- else if (!--mousedev->open && mousedev->exist)
- input_close_device(&mousedev->handle);
-
+ mousedev_close_device(mousedev);
put_device(&mousedev->dev);
return 0;
}
-
static int mousedev_open(struct inode *inode, struct file *file)
{
struct mousedev_client *client;
@@ -475,12 +545,17 @@ static int mousedev_open(struct inode *i
if (i >= MOUSEDEV_MINORS)
return -ENODEV;
+ error = mutex_lock_interruptible(&mousedev_table_mutex);
+ if (error)
+ return error;
mousedev = mousedev_table[i];
+ if (mousedev)
+ get_device(&mousedev->dev);
+ mutex_unlock(&mousedev_table_mutex);
+
if (!mousedev)
return -ENODEV;
- get_device(&mousedev->dev);
-
client = kzalloc(sizeof(struct mousedev_client), GFP_KERNEL);
if (!client) {
error = -ENOMEM;
@@ -491,21 +566,17 @@ static int mousedev_open(struct inode *i
client->pos_x = xres / 2;
client->pos_y = yres / 2;
client->mousedev = mousedev;
- list_add_tail(&client->node, &mousedev->client_list);
+ mousedev_attach_client(mousedev, client);
- if (mousedev->minor == MOUSEDEV_MIX)
- mixdev_open_devices();
- else if (!mousedev->open++ && mousedev->exist) {
- error = input_open_device(&mousedev->handle);
- if (error)
- goto err_free_client;
- }
+ error = mousedev_open_device(mousedev);
+ if (error)
+ goto err_free_client;
file->private_data = client;
return 0;
err_free_client:
- list_del(&client->node);
+ mousedev_detach_client(mousedev, client);
kfree(client);
err_put_mousedev:
put_device(&mousedev->dev);
@@ -517,41 +588,41 @@ static inline int mousedev_limit_delta(i
return delta > limit ? limit : (delta < -limit ? -limit : delta);
}
-static void mousedev_packet(struct mousedev_client *client, signed char *ps2_data)
+static void mousedev_packet(struct mousedev_client *client,
+ signed char *ps2_data)
{
- struct mousedev_motion *p;
- unsigned long flags;
-
- spin_lock_irqsave(&client->packet_lock, flags);
- p = &client->packets[client->tail];
+ struct mousedev_motion *p = &client->packets[client->tail];
- ps2_data[0] = 0x08 | ((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07);
+ ps2_data[0] = 0x08 |
+ ((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07);
ps2_data[1] = mousedev_limit_delta(p->dx, 127);
ps2_data[2] = mousedev_limit_delta(p->dy, 127);
p->dx -= ps2_data[1];
p->dy -= ps2_data[2];
switch (client->mode) {
- case MOUSEDEV_EMUL_EXPS:
- ps2_data[3] = mousedev_limit_delta(p->dz, 7);
- p->dz -= ps2_data[3];
- ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1);
- client->bufsiz = 4;
- break;
-
- case MOUSEDEV_EMUL_IMPS:
- ps2_data[0] |= ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1);
- ps2_data[3] = mousedev_limit_delta(p->dz, 127);
- p->dz -= ps2_data[3];
- client->bufsiz = 4;
- break;
-
- case MOUSEDEV_EMUL_PS2:
- default:
- ps2_data[0] |= ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1);
- p->dz = 0;
- client->bufsiz = 3;
- break;
+ case MOUSEDEV_EMUL_EXPS:
+ ps2_data[3] = mousedev_limit_delta(p->dz, 7);
+ p->dz -= ps2_data[3];
+ ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1);
+ client->bufsiz = 4;
+ break;
+
+ case MOUSEDEV_EMUL_IMPS:
+ ps2_data[0] |=
+ ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1);
+ ps2_data[3] = mousedev_limit_delta(p->dz, 127);
+ p->dz -= ps2_data[3];
+ client->bufsiz = 4;
+ break;
+
+ case MOUSEDEV_EMUL_PS2:
+ default:
+ ps2_data[0] |=
+ ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1);
+ p->dz = 0;
+ client->bufsiz = 3;
+ break;
}
if (!p->dx && !p->dy && !p->dz) {
@@ -561,12 +632,50 @@ static void mousedev_packet(struct mouse
} else
client->tail = (client->tail + 1) % PACKET_QUEUE_LEN;
}
-
- spin_unlock_irqrestore(&client->packet_lock, flags);
}
+static void mousedev_generate_response(struct mousedev_client *client,
+ int command)
+{
+ client->ps2[0] = 0xfa; /* ACK */
+
+ switch (command) {
+
+ case 0xeb: /* Poll */
+ mousedev_packet(client, &client->ps2[1]);
+ client->bufsiz++; /* account for leading ACK */
+ break;
+
+ case 0xf2: /* Get ID */
+ switch (client->mode) {
+ case MOUSEDEV_EMUL_PS2: client->ps2[1] = 0; break;
+ case MOUSEDEV_EMUL_IMPS: client->ps2[1] = 3; break;
+ case MOUSEDEV_EMUL_EXPS: client->ps2[1] = 4; break;
+ }
+ client->bufsiz = 2;
+ break;
+
+ case 0xe9: /* Get info */
+ client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200;
+ client->bufsiz = 4;
+ break;
+
+ case 0xff: /* Reset */
+ client->impsseq = client->imexseq = 0;
+ client->mode = MOUSEDEV_EMUL_PS2;
+ client->ps2[1] = 0xaa; client->ps2[2] = 0x00;
+ client->bufsiz = 3;
+ break;
+
+ default:
+ client->bufsiz = 1;
+ break;
+ }
+ client->buffer = client->bufsiz;
+}
-static ssize_t mousedev_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t mousedev_write(struct file *file, const char __user *buffer,
+ size_t count, loff_t *ppos)
{
struct mousedev_client *client = file->private_data;
unsigned char c;
@@ -577,6 +686,8 @@ static ssize_t mousedev_write(struct fil
if (get_user(c, buffer + i))
return -EFAULT;
+ spin_lock_irq(&client->packet_lock);
+
if (c == mousedev_imex_seq[client->imexseq]) {
if (++client->imexseq == MOUSEDEV_SEQ_LEN) {
client->imexseq = 0;
@@ -593,68 +704,39 @@ static ssize_t mousedev_write(struct fil
} else
client->impsseq = 0;
- client->ps2[0] = 0xfa;
-
- switch (c) {
+ mousedev_generate_response(client, c);
- case 0xeb: /* Poll */
- mousedev_packet(client, &client->ps2[1]);
- client->bufsiz++; /* account for leading ACK */
- break;
-
- case 0xf2: /* Get ID */
- switch (client->mode) {
- case MOUSEDEV_EMUL_PS2: client->ps2[1] = 0; break;
- case MOUSEDEV_EMUL_IMPS: client->ps2[1] = 3; break;
- case MOUSEDEV_EMUL_EXPS: client->ps2[1] = 4; break;
- }
- client->bufsiz = 2;
- break;
-
- case 0xe9: /* Get info */
- client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200;
- client->bufsiz = 4;
- break;
-
- case 0xff: /* Reset */
- client->impsseq = client->imexseq = 0;
- client->mode = MOUSEDEV_EMUL_PS2;
- client->ps2[1] = 0xaa; client->ps2[2] = 0x00;
- client->bufsiz = 3;
- break;
-
- default:
- client->bufsiz = 1;
- break;
- }
-
- client->buffer = client->bufsiz;
+ spin_unlock_irq(&client->packet_lock);
}
kill_fasync(&client->fasync, SIGIO, POLL_IN);
-
wake_up_interruptible(&client->mousedev->wait);
return count;
}
-static ssize_t mousedev_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
+static ssize_t mousedev_read(struct file *file, char __user *buffer,
+ size_t count, loff_t *ppos)
{
struct mousedev_client *client = file->private_data;
+ struct mousedev *mousedev = client->mousedev;
+ signed char data[sizeof(client->ps2)];
int retval = 0;
- if (!client->ready && !client->buffer && (file->f_flags & O_NONBLOCK))
+ if (!client->ready && !client->buffer && mousedev->exist &&
+ (file->f_flags & O_NONBLOCK))
return -EAGAIN;
- retval = wait_event_interruptible(client->mousedev->wait,
- !client->mousedev->exist || client->ready || client->buffer);
-
+ retval = wait_event_interruptible(mousedev->wait,
+ !mousedev->exist || client->ready || client->buffer);
if (retval)
return retval;
- if (!client->mousedev->exist)
+ if (!mousedev->exist)
return -ENODEV;
+ spin_lock_irq(&client->packet_lock);
+
if (!client->buffer && client->ready) {
mousedev_packet(client, client->ps2);
client->buffer = client->bufsiz;
@@ -663,9 +745,12 @@ static ssize_t mousedev_read(struct file
if (count > client->buffer)
count = client->buffer;
+ memcpy(data, client->ps2 + client->bufsiz - client->buffer, count);
client->buffer -= count;
- if (copy_to_user(buffer, client->ps2 + client->bufsiz - client->buffer - count, count))
+ spin_unlock_irq(&client->packet_lock);
+
+ if (copy_to_user(buffer, data, count))
return -EFAULT;
return count;
@@ -692,6 +777,60 @@ static const struct file_operations mous
.fasync = mousedev_fasync,
};
+static int mousedev_install_chrdev(struct mousedev *mousedev)
+{
+ mousedev_table[mousedev->minor] = mousedev;
+ return 0;
+}
+
+static void mousedev_remove_chrdev(struct mousedev *mousedev)
+{
+ mutex_lock(&mousedev_table_mutex);
+ mousedev_table[mousedev->minor] = NULL;
+ mutex_unlock(&mousedev_table_mutex);
+}
+
+/*
+ * Mark device non-existent. This disables writes, ioctls and
+ * prevents new users from opening the device. Already posted
+ * blocking reads will stay, however new ones will fail.
+ */
+static void mousedev_mark_dead(struct mousedev *mousedev)
+{
+ mutex_lock(&mousedev->mutex);
+ mousedev->exist = 0;
+ mutex_unlock(&mousedev->mutex);
+}
+
+/*
+ * Wake up users waiting for IO so they can disconnect from
+ * dead device.
+ */
+static void mousedev_hangup(struct mousedev *mousedev)
+{
+ struct mousedev_client *client;
+
+ spin_lock(&mousedev->client_lock);
+ list_for_each_entry(client, &mousedev->client_list, node)
+ kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+ spin_unlock(&mousedev->client_lock);
+
+ wake_up_interruptible(&mousedev->wait);
+}
+
+static void mousedev_cleanup(struct mousedev *mousedev)
+{
+ struct input_handle *handle = &mousedev->handle;
+
+ mousedev_mark_dead(mousedev);
+ mousedev_hangup(mousedev);
+ mousedev_remove_chrdev(mousedev);
+
+ /* mousedev is marked dead so no one else accesses mousedev->open */
+ if (mousedev->open)
+ input_close_device(handle);
+}
+
static struct mousedev *mousedev_create(struct input_dev *dev,
struct input_handler *handler,
int minor)
@@ -707,6 +846,10 @@ static struct mousedev *mousedev_create(
INIT_LIST_HEAD(&mousedev->client_list);
INIT_LIST_HEAD(&mousedev->mixdev_node);
+ spin_lock_init(&mousedev->client_lock);
+ mutex_init(&mousedev->mutex);
+ lockdep_set_subclass(&mousedev->mutex,
+ minor == MOUSEDEV_MIX ? MOUSEDEV_MIX : 0);
init_waitqueue_head(&mousedev->wait);
if (minor == MOUSEDEV_MIX)
@@ -731,14 +874,27 @@ static struct mousedev *mousedev_create(
mousedev->dev.release = mousedev_free;
device_initialize(&mousedev->dev);
- mousedev_table[minor] = mousedev;
+ if (minor != MOUSEDEV_MIX) {
+ error = input_register_handle(&mousedev->handle);
+ if (error)
+ goto err_free_mousedev;
+ }
+
+ error = mousedev_install_chrdev(mousedev);
+ if (error)
+ goto err_unregister_handle;
error = device_add(&mousedev->dev);
if (error)
- goto err_free_mousedev;
+ goto err_cleanup_mousedev;
return mousedev;
+ err_cleanup_mousedev:
+ mousedev_cleanup(mousedev);
+ err_unregister_handle:
+ if (minor != MOUSEDEV_MIX)
+ input_unregister_handle(&mousedev->handle);
err_free_mousedev:
put_device(&mousedev->dev);
err_out:
@@ -747,29 +903,64 @@ static struct mousedev *mousedev_create(
static void mousedev_destroy(struct mousedev *mousedev)
{
- struct mousedev_client *client;
-
device_del(&mousedev->dev);
- mousedev->exist = 0;
+ mousedev_cleanup(mousedev);
+ if (mousedev->minor != MOUSEDEV_MIX)
+ input_unregister_handle(&mousedev->handle);
+ put_device(&mousedev->dev);
+}
- if (mousedev->open) {
- input_close_device(&mousedev->handle);
- list_for_each_entry(client, &mousedev->client_list, node)
- kill_fasync(&client->fasync, SIGIO, POLL_HUP);
- wake_up_interruptible(&mousedev->wait);
+static int mixdev_add_device(struct mousedev *mousedev)
+{
+ int retval;
+
+ retval = mutex_lock_interruptible(&mousedev_mix->mutex);
+ if (retval)
+ return retval;
+
+ if (mousedev_mix->open) {
+ retval = mousedev_open_device(mousedev);
+ if (retval)
+ goto out;
+
+ mousedev->mixdev_open = 1;
+ }
+
+ get_device(&mousedev->dev);
+ list_add_tail(&mousedev->mixdev_node, &mousedev_mix_list);
+
+ out:
+ mutex_unlock(&mousedev_mix->mutex);
+ return retval;
+}
+
+static void mixdev_remove_device(struct mousedev *mousedev)
+{
+ mutex_lock(&mousedev_mix->mutex);
+
+ if (mousedev->mixdev_open) {
+ mousedev->mixdev_open = 0;
+ mousedev_close_device(mousedev);
}
+ list_del_init(&mousedev->mixdev_node);
+ mutex_unlock(&mousedev_mix->mutex);
+
put_device(&mousedev->dev);
}
-static int mousedev_connect(struct input_handler *handler, struct input_dev *dev,
+static int mousedev_connect(struct input_handler *handler,
+ struct input_dev *dev,
const struct input_device_id *id)
{
struct mousedev *mousedev;
int minor;
int error;
- for (minor = 0; minor < MOUSEDEV_MINORS && mousedev_table[minor]; minor++);
+ for (minor = 0; minor < MOUSEDEV_MINORS; minor++)
+ if (!mousedev_table[minor])
+ break;
+
if (minor == MOUSEDEV_MINORS) {
printk(KERN_ERR "mousedev: no more free mousedev devices\n");
return -ENFILE;
@@ -779,21 +970,13 @@ static int mousedev_connect(struct input
if (IS_ERR(mousedev))
return PTR_ERR(mousedev);
- error = input_register_handle(&mousedev->handle);
- if (error)
- goto err_delete_mousedev;
-
error = mixdev_add_device(mousedev);
- if (error)
- goto err_unregister_handle;
+ if (error) {
+ mousedev_destroy(mousedev);
+ return error;
+ }
return 0;
-
- err_unregister_handle:
- input_unregister_handle(&mousedev->handle);
- err_delete_mousedev:
- device_unregister(&mousedev->dev);
- return error;
}
static void mousedev_disconnect(struct input_handle *handle)
@@ -801,33 +984,42 @@ static void mousedev_disconnect(struct i
struct mousedev *mousedev = handle->private;
mixdev_remove_device(mousedev);
- input_unregister_handle(handle);
mousedev_destroy(mousedev);
}
static const struct input_device_id mousedev_ids[] = {
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT | INPUT_DEVICE_ID_MATCH_RELBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_KEYBIT |
+ INPUT_DEVICE_ID_MATCH_RELBIT,
.evbit = { BIT(EV_KEY) | BIT(EV_REL) },
.keybit = { [LONG(BTN_LEFT)] = BIT(BTN_LEFT) },
.relbit = { BIT(REL_X) | BIT(REL_Y) },
- }, /* A mouse like device, at least one button, two relative axes */
+ }, /* A mouse like device, at least one button,
+ two relative axes */
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_RELBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_RELBIT,
.evbit = { BIT(EV_KEY) | BIT(EV_REL) },
.relbit = { BIT(REL_WHEEL) },
}, /* A separate scrollwheel */
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_KEYBIT |
+ INPUT_DEVICE_ID_MATCH_ABSBIT,
.evbit = { BIT(EV_KEY) | BIT(EV_ABS) },
.keybit = { [LONG(BTN_TOUCH)] = BIT(BTN_TOUCH) },
.absbit = { BIT(ABS_X) | BIT(ABS_Y) },
- }, /* A tablet like device, at least touch detection, two absolute axes */
+ }, /* A tablet like device, at least touch detection,
+ two absolute axes */
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_KEYBIT |
+ INPUT_DEVICE_ID_MATCH_ABSBIT,
.evbit = { BIT(EV_KEY) | BIT(EV_ABS) },
.keybit = { [LONG(BTN_TOOL_FINGER)] = BIT(BTN_TOOL_FINGER) },
- .absbit = { BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE) | BIT(ABS_TOOL_WIDTH) },
+ .absbit = { BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE) |
+ BIT(ABS_TOOL_WIDTH) },
}, /* A touchpad */
{ }, /* Terminating entry */
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC/RFT 5/5] Input: joydev - implement proper locking
2007-07-24 4:45 [RFC/RFT 0/5] Input locking patches Dmitry Torokhov
` (3 preceding siblings ...)
2007-07-24 4:45 ` [RFC/RFT 4/5] Input: mousedev " Dmitry Torokhov
@ 2007-07-24 4:45 ` Dmitry Torokhov
2007-07-27 22:25 ` [RFC/RFT 0/5] Input locking patches Indan Zupancic
5 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-24 4:45 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
[-- Attachment #1: joydev-locking.patch --]
[-- Type: text/plain, Size: 28880 bytes --]
Input: joydev - implement proper locking
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/joydev.c | 745 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 493 insertions(+), 252 deletions(-)
Index: work/drivers/input/joydev.c
===================================================================
--- work.orig/drivers/input/joydev.c
+++ work/drivers/input/joydev.c
@@ -43,6 +43,8 @@ struct joydev {
struct input_handle handle;
wait_queue_head_t wait;
struct list_head client_list;
+ spinlock_t client_lock; /* protects client_list */
+ struct mutex mutex;
struct device dev;
struct js_corr corr[ABS_MAX + 1];
@@ -61,31 +63,61 @@ struct joydev_client {
int head;
int tail;
int startup;
+ spinlock_t buffer_lock; /* protects access to buffer, head and tail */
struct fasync_struct *fasync;
struct joydev *joydev;
struct list_head node;
};
static struct joydev *joydev_table[JOYDEV_MINORS];
+static DEFINE_MUTEX(joydev_table_mutex);
static int joydev_correct(int value, struct js_corr *corr)
{
switch (corr->type) {
- case JS_CORR_NONE:
- break;
- case JS_CORR_BROKEN:
- value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
- ((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
- ((corr->coef[2] * (value - corr->coef[0])) >> 14);
- break;
- default:
- return 0;
+
+ case JS_CORR_NONE:
+ break;
+
+ case JS_CORR_BROKEN:
+ value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
+ ((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
+ ((corr->coef[2] * (value - corr->coef[0])) >> 14);
+ break;
+
+ default:
+ return 0;
}
return value < -32767 ? -32767 : (value > 32767 ? 32767 : value);
}
-static void joydev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
+static void joydev_pass_event(struct joydev_client *client,
+ struct js_event *event)
+{
+ struct joydev *joydev = client->joydev;
+
+ /*
+ * IRQs already disabled, just acquire the lock
+ */
+ spin_lock(&client->buffer_lock);
+
+ client->buffer[client->head] = *event;
+
+ if (client->startup == joydev->nabs + joydev->nkey) {
+ client->head++;
+ client->head &= JOYDEV_BUFFER_SIZE - 1;
+ if (client->tail == client->head)
+ client->startup = 0;
+ }
+
+ spin_unlock(&client->buffer_lock);
+
+ kill_fasync(&client->fasync, SIGIO, POLL_IN);
+}
+
+static void joydev_event(struct input_handle *handle,
+ unsigned int type, unsigned int code, int value)
{
struct joydev *joydev = handle->private;
struct joydev_client *client;
@@ -93,39 +125,32 @@ static void joydev_event(struct input_ha
switch (type) {
- case EV_KEY:
- if (code < BTN_MISC || value == 2)
- return;
- event.type = JS_EVENT_BUTTON;
- event.number = joydev->keymap[code - BTN_MISC];
- event.value = value;
- break;
-
- case EV_ABS:
- event.type = JS_EVENT_AXIS;
- event.number = joydev->absmap[code];
- event.value = joydev_correct(value, joydev->corr + event.number);
- if (event.value == joydev->abs[event.number])
- return;
- joydev->abs[event.number] = event.value;
- break;
+ case EV_KEY:
+ if (code < BTN_MISC || value == 2)
+ return;
+ event.type = JS_EVENT_BUTTON;
+ event.number = joydev->keymap[code - BTN_MISC];
+ event.value = value;
+ break;
- default:
+ case EV_ABS:
+ event.type = JS_EVENT_AXIS;
+ event.number = joydev->absmap[code];
+ event.value = joydev_correct(value,
+ &joydev->corr[event.number]);
+ if (event.value == joydev->abs[event.number])
return;
+ joydev->abs[event.number] = event.value;
+ break;
+
+ default:
+ return;
}
event.time = jiffies_to_msecs(jiffies);
- list_for_each_entry(client, &joydev->client_list, node) {
-
- memcpy(client->buffer + client->head, &event, sizeof(struct js_event));
-
- if (client->startup == joydev->nabs + joydev->nkey)
- if (client->tail == (client->head = (client->head + 1) & (JOYDEV_BUFFER_SIZE - 1)))
- client->startup = 0;
-
- kill_fasync(&client->fasync, SIGIO, POLL_IN);
- }
+ list_for_each_entry_rcu(client, &joydev->client_list, node)
+ joydev_pass_event(client, &event);
wake_up_interruptible(&joydev->wait);
}
@@ -144,23 +169,85 @@ static void joydev_free(struct device *d
{
struct joydev *joydev = container_of(dev, struct joydev, dev);
- joydev_table[joydev->minor] = NULL;
kfree(joydev);
}
+static void joydev_attach_client(struct joydev *joydev,
+ struct joydev_client *client)
+{
+ spin_lock(&joydev->client_lock);
+ list_add_tail_rcu(&client->node, &joydev->client_list);
+ spin_unlock(&joydev->client_lock);
+ /*
+ * We don't use synchronize_rcu() here because read-size
+ * critical section is protected by a spinlock (dev->event_lock)
+ * instead of rcu_read_lock().
+ */
+ synchronize_sched();
+}
+
+static void joydev_detach_client(struct joydev *joydev,
+ struct joydev_client *client)
+{
+ spin_lock(&joydev->client_lock);
+ list_del_rcu(&client->node);
+ spin_unlock(&joydev->client_lock);
+ synchronize_sched();
+}
+
+static int joydev_open_device(struct joydev *joydev)
+{
+ int retval;
+
+ retval = mutex_lock_interruptible(&joydev->mutex);
+ if (retval)
+ return retval;
+
+ if (!joydev->exist)
+ retval = -ENODEV;
+ else if (!joydev->open++)
+ retval = input_open_device(&joydev->handle);
+
+ mutex_unlock(&joydev->mutex);
+ return retval;
+}
+
+static void joydev_close_device(struct joydev *joydev)
+{
+ mutex_lock(&joydev->mutex);
+
+ if (joydev->exist && !--joydev->open)
+ input_close_device(&joydev->handle);
+
+ mutex_unlock(&joydev->mutex);
+}
+
+/*
+ * Wake up users waiting for IO so they can disconnect from
+ * dead device.
+ */
+static void joydev_hangup(struct joydev *joydev)
+{
+ struct joydev_client *client;
+
+ spin_lock(&joydev->client_lock);
+ list_for_each_entry(client, &joydev->client_list, node)
+ kill_fasync(&client->fasync, SIGIO, POLL_HUP);
+ spin_unlock(&joydev->client_lock);
+
+ wake_up_interruptible(&joydev->wait);
+}
+
static int joydev_release(struct inode *inode, struct file *file)
{
struct joydev_client *client = file->private_data;
struct joydev *joydev = client->joydev;
joydev_fasync(-1, file, 0);
-
- list_del(&client->node);
+ joydev_detach_client(joydev, client);
kfree(client);
- if (!--joydev->open && joydev->exist)
- input_close_device(&joydev->handle);
-
+ joydev_close_device(joydev);
put_device(&joydev->dev);
return 0;
@@ -176,11 +263,16 @@ static int joydev_open(struct inode *ino
if (i >= JOYDEV_MINORS)
return -ENODEV;
+ error = mutex_lock_interruptible(&joydev_table_mutex);
+ if (error)
+ return error;
joydev = joydev_table[i];
- if (!joydev || !joydev->exist)
- return -ENODEV;
+ if (joydev)
+ get_device(&joydev->dev);
+ mutex_unlock(&joydev_table_mutex);
- get_device(&joydev->dev);
+ if (!joydev)
+ return -ENODEV;
client = kzalloc(sizeof(struct joydev_client), GFP_KERNEL);
if (!client) {
@@ -188,37 +280,129 @@ static int joydev_open(struct inode *ino
goto err_put_joydev;
}
+ spin_lock_init(&client->buffer_lock);
client->joydev = joydev;
- list_add_tail(&client->node, &joydev->client_list);
+ joydev_attach_client(joydev, client);
- if (!joydev->open++ && joydev->exist) {
- error = input_open_device(&joydev->handle);
- if (error)
- goto err_free_client;
- }
+ error = joydev_open_device(joydev);
+ if (error)
+ goto err_free_client;
file->private_data = client;
return 0;
err_free_client:
- list_del(&client->node);
+ joydev_detach_client(joydev, client);
kfree(client);
err_put_joydev:
put_device(&joydev->dev);
return error;
}
-static ssize_t joydev_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+static int joydev_generate_startup_event(struct joydev_client *client,
+ struct input_dev *input,
+ struct js_event *event)
{
- return -EINVAL;
+ struct joydev *joydev = client->joydev;
+ int have_event;
+
+ spin_lock_irq(&client->buffer_lock);
+
+ have_event = client->startup < joydev->nabs + joydev->nkey;
+
+ if (have_event) {
+
+ event->time = jiffies_to_msecs(jiffies);
+ if (client->startup < joydev->nkey) {
+ event->type = JS_EVENT_BUTTON | JS_EVENT_INIT;
+ event->number = client->startup;
+ event->value = !!test_bit(joydev->keypam[event->number],
+ input->key);
+ } else {
+ event->type = JS_EVENT_AXIS | JS_EVENT_INIT;
+ event->number = client->startup - joydev->nkey;
+ event->value = joydev->abs[event->number];
+ }
+ client->startup++;
+ }
+
+ spin_unlock_irq(&client->buffer_lock);
+
+ return have_event;
+}
+
+static int joydev_fetch_next_event(struct joydev_client *client,
+ struct js_event *event)
+{
+ int have_event;
+
+ spin_lock_irq(&client->buffer_lock);
+
+ have_event = client->head != client->tail;
+ if (have_event) {
+ *event = client->buffer[client->tail++];
+ client->tail &= JOYDEV_BUFFER_SIZE - 1;
+ }
+
+ spin_unlock_irq(&client->buffer_lock);
+
+ return have_event;
}
-static ssize_t joydev_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+/*
+ * Old joystick interface
+ */
+static ssize_t joydev_0x_read(struct joydev_client *client,
+ struct input_dev *input,
+ char __user *buf)
+{
+ struct joydev *joydev = client->joydev;
+ struct JS_DATA_TYPE data;
+ int i;
+
+ spin_lock_irq(&input->event_lock);
+
+ /*
+ * Get device state
+ */
+ for (data.buttons = i = 0; i < 32 && i < joydev->nkey; i++)
+ data.buttons |=
+ test_bit(joydev->keypam[i], input->key) ? (1 << i) : 0;
+ data.x = (joydev->abs[0] / 256 + 128) >> joydev->glue.JS_CORR.x;
+ data.y = (joydev->abs[1] / 256 + 128) >> joydev->glue.JS_CORR.y;
+
+ /*
+ * Reset reader's event queue
+ */
+ spin_lock(&client->buffer_lock);
+ client->startup = 0;
+ client->tail = client->head;
+ spin_unlock(&client->buffer_lock);
+
+ spin_unlock_irq(&input->event_lock);
+
+ if (copy_to_user(buf, &data, sizeof(struct JS_DATA_TYPE)))
+ return -EFAULT;
+
+ return sizeof(struct JS_DATA_TYPE);
+}
+
+static inline int joydev_data_pending(struct joydev_client *client)
+{
+ struct joydev *joydev = client->joydev;
+
+ return client->startup < joydev->nabs + joydev->nkey ||
+ client->head != client->tail;
+}
+
+static ssize_t joydev_read(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
{
struct joydev_client *client = file->private_data;
struct joydev *joydev = client->joydev;
struct input_dev *input = joydev->handle.dev;
- int retval = 0;
+ struct js_event event;
+ int retval;
if (!joydev->exist)
return -ENODEV;
@@ -226,68 +410,35 @@ static ssize_t joydev_read(struct file *
if (count < sizeof(struct js_event))
return -EINVAL;
- if (count == sizeof(struct JS_DATA_TYPE)) {
-
- struct JS_DATA_TYPE data;
- int i;
-
- for (data.buttons = i = 0; i < 32 && i < joydev->nkey; i++)
- data.buttons |= test_bit(joydev->keypam[i], input->key) ? (1 << i) : 0;
- data.x = (joydev->abs[0] / 256 + 128) >> joydev->glue.JS_CORR.x;
- data.y = (joydev->abs[1] / 256 + 128) >> joydev->glue.JS_CORR.y;
-
- if (copy_to_user(buf, &data, sizeof(struct JS_DATA_TYPE)))
- return -EFAULT;
-
- client->startup = 0;
- client->tail = client->head;
-
- return sizeof(struct JS_DATA_TYPE);
- }
+ if (count == sizeof(struct JS_DATA_TYPE))
+ return joydev_0x_read(client, input, buf);
- if (client->startup == joydev->nabs + joydev->nkey &&
- client->head == client->tail && (file->f_flags & O_NONBLOCK))
+ if (!joydev_data_pending(client) && (file->f_flags & O_NONBLOCK))
return -EAGAIN;
retval = wait_event_interruptible(joydev->wait,
- !joydev->exist ||
- client->startup < joydev->nabs + joydev->nkey ||
- client->head != client->tail);
+ !joydev->exist || joydev_data_pending(client));
if (retval)
return retval;
if (!joydev->exist)
return -ENODEV;
- while (client->startup < joydev->nabs + joydev->nkey && retval + sizeof(struct js_event) <= count) {
-
- struct js_event event;
-
- event.time = jiffies_to_msecs(jiffies);
-
- if (client->startup < joydev->nkey) {
- event.type = JS_EVENT_BUTTON | JS_EVENT_INIT;
- event.number = client->startup;
- event.value = !!test_bit(joydev->keypam[event.number], input->key);
- } else {
- event.type = JS_EVENT_AXIS | JS_EVENT_INIT;
- event.number = client->startup - joydev->nkey;
- event.value = joydev->abs[event.number];
- }
+ while (retval + sizeof(struct js_event) <= count &&
+ joydev_generate_startup_event(client, input, &event)) {
if (copy_to_user(buf + retval, &event, sizeof(struct js_event)))
return -EFAULT;
- client->startup++;
retval += sizeof(struct js_event);
}
- while (client->head != client->tail && retval + sizeof(struct js_event) <= count) {
+ while (retval + sizeof(struct js_event) <= count &&
+ joydev_fetch_next_event(client, &event)) {
- if (copy_to_user(buf + retval, client->buffer + client->tail, sizeof(struct js_event)))
+ if (copy_to_user(buf + retval, &event, sizeof(struct js_event)))
return -EFAULT;
- client->tail = (client->tail + 1) & (JOYDEV_BUFFER_SIZE - 1);
retval += sizeof(struct js_event);
}
@@ -301,126 +452,144 @@ static unsigned int joydev_poll(struct f
struct joydev *joydev = client->joydev;
poll_wait(file, &joydev->wait, wait);
- return ((client->head != client->tail || client->startup < joydev->nabs + joydev->nkey) ?
- (POLLIN | POLLRDNORM) : 0) | (joydev->exist ? 0 : (POLLHUP | POLLERR));
+ return (joydev_data_pending(client) ? (POLLIN | POLLRDNORM) : 0) |
+ (joydev->exist ? 0 : (POLLHUP | POLLERR));
}
-static int joydev_ioctl_common(struct joydev *joydev, unsigned int cmd, void __user *argp)
+static int joydev_ioctl_common(struct joydev *joydev,
+ unsigned int cmd, void __user *argp)
{
struct input_dev *dev = joydev->handle.dev;
int i, j;
switch (cmd) {
- case JS_SET_CAL:
- return copy_from_user(&joydev->glue.JS_CORR, argp,
+ case JS_SET_CAL:
+ return copy_from_user(&joydev->glue.JS_CORR, argp,
sizeof(joydev->glue.JS_CORR)) ? -EFAULT : 0;
- case JS_GET_CAL:
- return copy_to_user(argp, &joydev->glue.JS_CORR,
+ case JS_GET_CAL:
+ return copy_to_user(argp, &joydev->glue.JS_CORR,
sizeof(joydev->glue.JS_CORR)) ? -EFAULT : 0;
- case JS_SET_TIMEOUT:
- return get_user(joydev->glue.JS_TIMEOUT, (s32 __user *) argp);
+ case JS_SET_TIMEOUT:
+ return get_user(joydev->glue.JS_TIMEOUT, (s32 __user *) argp);
- case JS_GET_TIMEOUT:
- return put_user(joydev->glue.JS_TIMEOUT, (s32 __user *) argp);
+ case JS_GET_TIMEOUT:
+ return put_user(joydev->glue.JS_TIMEOUT, (s32 __user *) argp);
- case JSIOCGVERSION:
- return put_user(JS_VERSION, (__u32 __user *) argp);
+ case JSIOCGVERSION:
+ return put_user(JS_VERSION, (__u32 __user *) argp);
- case JSIOCGAXES:
- return put_user(joydev->nabs, (__u8 __user *) argp);
-
- case JSIOCGBUTTONS:
- return put_user(joydev->nkey, (__u8 __user *) argp);
-
- case JSIOCSCORR:
- if (copy_from_user(joydev->corr, argp,
- sizeof(joydev->corr[0]) * joydev->nabs))
- return -EFAULT;
- for (i = 0; i < joydev->nabs; i++) {
- j = joydev->abspam[i];
- joydev->abs[i] = joydev_correct(dev->abs[j], joydev->corr + i);
- }
- return 0;
-
- case JSIOCGCORR:
- return copy_to_user(argp, joydev->corr,
- sizeof(joydev->corr[0]) * joydev->nabs) ? -EFAULT : 0;
+ case JSIOCGAXES:
+ return put_user(joydev->nabs, (__u8 __user *) argp);
+
+ case JSIOCGBUTTONS:
+ return put_user(joydev->nkey, (__u8 __user *) argp);
+
+ case JSIOCSCORR:
+ if (copy_from_user(joydev->corr, argp,
+ sizeof(joydev->corr[0]) * joydev->nabs))
+ return -EFAULT;
+
+ for (i = 0; i < joydev->nabs; i++) {
+ j = joydev->abspam[i];
+ joydev->abs[i] = joydev_correct(dev->abs[j],
+ &joydev->corr[i]);
+ }
+ return 0;
- case JSIOCSAXMAP:
- if (copy_from_user(joydev->abspam, argp, sizeof(__u8) * (ABS_MAX + 1)))
- return -EFAULT;
- for (i = 0; i < joydev->nabs; i++) {
- if (joydev->abspam[i] > ABS_MAX)
- return -EINVAL;
- joydev->absmap[joydev->abspam[i]] = i;
- }
- return 0;
-
- case JSIOCGAXMAP:
- return copy_to_user(argp, joydev->abspam,
- sizeof(__u8) * (ABS_MAX + 1)) ? -EFAULT : 0;
+ case JSIOCGCORR:
+ return copy_to_user(argp, joydev->corr,
+ sizeof(joydev->corr[0]) * joydev->nabs) ? -EFAULT : 0;
+
+ case JSIOCSAXMAP:
+ if (copy_from_user(joydev->abspam, argp,
+ sizeof(__u8) * (ABS_MAX + 1)))
+ return -EFAULT;
+
+ for (i = 0; i < joydev->nabs; i++) {
+ if (joydev->abspam[i] > ABS_MAX)
+ return -EINVAL;
+ joydev->absmap[joydev->abspam[i]] = i;
+ }
+ return 0;
+
+ case JSIOCGAXMAP:
+ return copy_to_user(argp, joydev->abspam,
+ sizeof(__u8) * (ABS_MAX + 1)) ? -EFAULT : 0;
+
+ case JSIOCSBTNMAP:
+ if (copy_from_user(joydev->keypam, argp,
+ sizeof(__u16) * (KEY_MAX - BTN_MISC + 1)))
+ return -EFAULT;
+
+ for (i = 0; i < joydev->nkey; i++) {
+ if (joydev->keypam[i] > KEY_MAX ||
+ joydev->keypam[i] < BTN_MISC)
+ return -EINVAL;
+ joydev->keymap[joydev->keypam[i] - BTN_MISC] = i;
+ }
- case JSIOCSBTNMAP:
- if (copy_from_user(joydev->keypam, argp, sizeof(__u16) * (KEY_MAX - BTN_MISC + 1)))
+ return 0;
+
+ case JSIOCGBTNMAP:
+ return copy_to_user(argp, joydev->keypam,
+ sizeof(__u16) * (KEY_MAX - BTN_MISC + 1)) ? -EFAULT : 0;
+
+ default:
+ if ((cmd & ~IOCSIZE_MASK) == JSIOCGNAME(0)) {
+ int len;
+ if (!dev->name)
+ return 0;
+ len = strlen(dev->name) + 1;
+ if (len > _IOC_SIZE(cmd))
+ len = _IOC_SIZE(cmd);
+ if (copy_to_user(argp, dev->name, len))
return -EFAULT;
- for (i = 0; i < joydev->nkey; i++) {
- if (joydev->keypam[i] > KEY_MAX || joydev->keypam[i] < BTN_MISC)
- return -EINVAL;
- joydev->keymap[joydev->keypam[i] - BTN_MISC] = i;
- }
- return 0;
-
- case JSIOCGBTNMAP:
- return copy_to_user(argp, joydev->keypam,
- sizeof(__u16) * (KEY_MAX - BTN_MISC + 1)) ? -EFAULT : 0;
-
- default:
- if ((cmd & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT)) == JSIOCGNAME(0)) {
- int len;
- if (!dev->name)
- return 0;
- len = strlen(dev->name) + 1;
- if (len > _IOC_SIZE(cmd))
- len = _IOC_SIZE(cmd);
- if (copy_to_user(argp, dev->name, len))
- return -EFAULT;
- return len;
- }
+ return len;
+ }
}
return -EINVAL;
}
#ifdef CONFIG_COMPAT
-static long joydev_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long joydev_compat_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
{
struct joydev_client *client = file->private_data;
struct joydev *joydev = client->joydev;
void __user *argp = (void __user *)arg;
s32 tmp32;
struct JS_DATA_SAVE_TYPE_32 ds32;
- int err;
+ int retval;
- if (!joydev->exist)
- return -ENODEV;
+ retval = mutex_lock_interruptible(&joydev->mutex);
+ if (retval)
+ return retval;
+
+ if (!joydev->exist) {
+ retval = -ENODEV;
+ goto out;
+ }
+
+ switch (cmd) {
- switch(cmd) {
case JS_SET_TIMELIMIT:
- err = get_user(tmp32, (s32 __user *) arg);
- if (err == 0)
+ retval = get_user(tmp32, (s32 __user *) arg);
+ if (retval == 0)
joydev->glue.JS_TIMELIMIT = tmp32;
break;
+
case JS_GET_TIMELIMIT:
tmp32 = joydev->glue.JS_TIMELIMIT;
- err = put_user(tmp32, (s32 __user *) arg);
+ retval = put_user(tmp32, (s32 __user *) arg);
break;
case JS_SET_ALL:
- err = copy_from_user(&ds32, argp,
- sizeof(ds32)) ? -EFAULT : 0;
- if (err == 0) {
+ retval = copy_from_user(&ds32, argp,
+ sizeof(ds32)) ? -EFAULT : 0;
+ if (retval == 0) {
joydev->glue.JS_TIMEOUT = ds32.JS_TIMEOUT;
joydev->glue.BUSY = ds32.BUSY;
joydev->glue.JS_EXPIRETIME = ds32.JS_EXPIRETIME;
@@ -438,55 +607,119 @@ static long joydev_compat_ioctl(struct f
ds32.JS_SAVE = joydev->glue.JS_SAVE;
ds32.JS_CORR = joydev->glue.JS_CORR;
- err = copy_to_user(argp, &ds32, sizeof(ds32)) ? -EFAULT : 0;
+ retval = copy_to_user(argp, &ds32, sizeof(ds32)) ? -EFAULT : 0;
break;
default:
- err = joydev_ioctl_common(joydev, cmd, argp);
+ retval = joydev_ioctl_common(joydev, cmd, argp);
+ break;
}
- return err;
+
+ out:
+ mutex_unlock(&joydev->mutex);
+ return retval;
}
#endif /* CONFIG_COMPAT */
-static int joydev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
+static long joydev_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
{
struct joydev_client *client = file->private_data;
struct joydev *joydev = client->joydev;
void __user *argp = (void __user *)arg;
+ int retval;
- if (!joydev->exist)
- return -ENODEV;
+ retval = mutex_lock_interruptible(&joydev->mutex);
+ if (retval)
+ return retval;
+
+ if (!joydev->exist) {
+ retval = -ENODEV;
+ goto out;
+ }
+
+ switch (cmd) {
+
+ case JS_SET_TIMELIMIT:
+ retval = get_user(joydev->glue.JS_TIMELIMIT,
+ (long __user *) arg);
+ break;
+
+ case JS_GET_TIMELIMIT:
+ retval = put_user(joydev->glue.JS_TIMELIMIT,
+ (long __user *) arg);
+ break;
+
+ case JS_SET_ALL:
+ retval = copy_from_user(&joydev->glue, argp,
+ sizeof(joydev->glue)) ? -EFAULT: 0;
+ break;
- switch(cmd) {
- case JS_SET_TIMELIMIT:
- return get_user(joydev->glue.JS_TIMELIMIT, (long __user *) arg);
- case JS_GET_TIMELIMIT:
- return put_user(joydev->glue.JS_TIMELIMIT, (long __user *) arg);
- case JS_SET_ALL:
- return copy_from_user(&joydev->glue, argp,
- sizeof(joydev->glue)) ? -EFAULT : 0;
- case JS_GET_ALL:
- return copy_to_user(argp, &joydev->glue,
- sizeof(joydev->glue)) ? -EFAULT : 0;
- default:
- return joydev_ioctl_common(joydev, cmd, argp);
+ case JS_GET_ALL:
+ retval = copy_to_user(argp, &joydev->glue,
+ sizeof(joydev->glue)) ? -EFAULT : 0;
+ break;
+
+ default:
+ retval = joydev_ioctl_common(joydev, cmd, argp);
+ break;
}
+ out:
+ mutex_unlock(&joydev->mutex);
+ return retval;
}
static const struct file_operations joydev_fops = {
- .owner = THIS_MODULE,
- .read = joydev_read,
- .write = joydev_write,
- .poll = joydev_poll,
- .open = joydev_open,
- .release = joydev_release,
- .ioctl = joydev_ioctl,
+ .owner = THIS_MODULE,
+ .read = joydev_read,
+ .poll = joydev_poll,
+ .open = joydev_open,
+ .release = joydev_release,
+ .unlocked_ioctl = joydev_ioctl,
#ifdef CONFIG_COMPAT
- .compat_ioctl = joydev_compat_ioctl,
+ .compat_ioctl = joydev_compat_ioctl,
#endif
- .fasync = joydev_fasync,
+ .fasync = joydev_fasync,
};
+static int joydev_install_chrdev(struct joydev *joydev)
+{
+ joydev_table[joydev->minor] = joydev;
+ return 0;
+}
+
+static void joydev_remove_chrdev(struct joydev *joydev)
+{
+ mutex_lock(&joydev_table_mutex);
+ joydev_table[joydev->minor] = NULL;
+ mutex_unlock(&joydev_table_mutex);
+}
+
+/*
+ * Mark device non-existant. This disables writes, ioctls and
+ * prevents new users from opening the device. Already posted
+ * blocking reads will stay, however new ones will fail.
+ */
+static void joydev_mark_dead(struct joydev *joydev)
+{
+ mutex_lock(&joydev->mutex);
+ joydev->exist = 0;
+ mutex_unlock(&joydev->mutex);
+}
+
+static void joydev_cleanup(struct joydev *joydev)
+{
+ struct input_handle *handle = &joydev->handle;
+
+ joydev_mark_dead(joydev);
+ joydev_hangup(joydev);
+ joydev_remove_chrdev(joydev);
+
+ /* joydev is marked dead so noone else accesses joydev->open */
+ if (joydev->open)
+ input_close_device(handle);
+}
+
static int joydev_connect(struct input_handler *handler, struct input_dev *dev,
const struct input_device_id *id)
{
@@ -494,7 +727,10 @@ static int joydev_connect(struct input_h
int i, j, t, minor;
int error;
- for (minor = 0; minor < JOYDEV_MINORS && joydev_table[minor]; minor++);
+ for (minor = 0; minor < JOYDEV_MINORS; minor++)
+ if (!joydev_table[minor])
+ break;
+
if (minor == JOYDEV_MINORS) {
printk(KERN_ERR "joydev: no more free joydev devices\n");
return -ENFILE;
@@ -505,15 +741,19 @@ static int joydev_connect(struct input_h
return -ENOMEM;
INIT_LIST_HEAD(&joydev->client_list);
+ spin_lock_init(&joydev->client_lock);
+ mutex_init(&joydev->mutex);
init_waitqueue_head(&joydev->wait);
+ snprintf(joydev->name, sizeof(joydev->name), "js%d", minor);
+ joydev->exist = 1;
joydev->minor = minor;
+
joydev->exist = 1;
joydev->handle.dev = dev;
joydev->handle.name = joydev->name;
joydev->handle.handler = handler;
joydev->handle.private = joydev;
- snprintf(joydev->name, sizeof(joydev->name), "js%d", minor);
for (i = 0; i < ABS_MAX + 1; i++)
if (test_bit(i, dev->absbit)) {
@@ -545,67 +785,65 @@ static int joydev_connect(struct input_h
}
joydev->corr[i].type = JS_CORR_BROKEN;
joydev->corr[i].prec = dev->absfuzz[j];
- joydev->corr[i].coef[0] = (dev->absmax[j] + dev->absmin[j]) / 2 - dev->absflat[j];
- joydev->corr[i].coef[1] = (dev->absmax[j] + dev->absmin[j]) / 2 + dev->absflat[j];
- if (!(t = ((dev->absmax[j] - dev->absmin[j]) / 2 - 2 * dev->absflat[j])))
- continue;
- joydev->corr[i].coef[2] = (1 << 29) / t;
- joydev->corr[i].coef[3] = (1 << 29) / t;
+ joydev->corr[i].coef[0] =
+ (dev->absmax[j] + dev->absmin[j]) / 2 - dev->absflat[j];
+ joydev->corr[i].coef[1] =
+ (dev->absmax[j] + dev->absmin[j]) / 2 + dev->absflat[j];
+
+ t = (dev->absmax[j] - dev->absmin[j]) / 2 - 2 * dev->absflat[j];
+ if (t) {
+ joydev->corr[i].coef[2] = (1 << 29) / t;
+ joydev->corr[i].coef[3] = (1 << 29) / t;
- joydev->abs[i] = joydev_correct(dev->abs[j], joydev->corr + i);
+ joydev->abs[i] = joydev_correct(dev->abs[j],
+ joydev->corr + i);
+ }
}
- snprintf(joydev->dev.bus_id, sizeof(joydev->dev.bus_id),
- "js%d", minor);
+ strlcpy(joydev->dev.bus_id, joydev->name, sizeof(joydev->dev.bus_id));
+ joydev->dev.devt = MKDEV(INPUT_MAJOR, JOYDEV_MINOR_BASE + minor);
joydev->dev.class = &input_class;
joydev->dev.parent = &dev->dev;
- joydev->dev.devt = MKDEV(INPUT_MAJOR, JOYDEV_MINOR_BASE + minor);
joydev->dev.release = joydev_free;
device_initialize(&joydev->dev);
- joydev_table[minor] = joydev;
-
- error = device_add(&joydev->dev);
+ error = input_register_handle(&joydev->handle);
if (error)
goto err_free_joydev;
- error = input_register_handle(&joydev->handle);
+ error = joydev_install_chrdev(joydev);
if (error)
- goto err_delete_joydev;
+ goto err_unregister_handle;
+
+ error = device_add(&joydev->dev);
+ if (error)
+ goto err_cleanup_joydev;
return 0;
- err_delete_joydev:
- device_del(&joydev->dev);
+ err_cleanup_joydev:
+ joydev_cleanup(joydev);
+ err_unregister_handle:
+ input_unregister_handle(&joydev->handle);
err_free_joydev:
put_device(&joydev->dev);
return error;
}
-
static void joydev_disconnect(struct input_handle *handle)
{
struct joydev *joydev = handle->private;
- struct joydev_client *client;
- input_unregister_handle(handle);
device_del(&joydev->dev);
-
- joydev->exist = 0;
-
- if (joydev->open) {
- input_close_device(handle);
- list_for_each_entry(client, &joydev->client_list, node)
- kill_fasync(&client->fasync, SIGIO, POLL_HUP);
- wake_up_interruptible(&joydev->wait);
- }
-
+ joydev_cleanup(joydev);
+ input_unregister_handle(handle);
put_device(&joydev->dev);
}
static const struct input_device_id joydev_blacklist[] = {
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_KEYBIT,
.evbit = { BIT(EV_KEY) },
.keybit = { [LONG(BTN_TOUCH)] = BIT(BTN_TOUCH) },
}, /* Avoid itouchpads, touchscreens and tablets */
@@ -614,17 +852,20 @@ static const struct input_device_id joyd
static const struct input_device_id joydev_ids[] = {
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_ABSBIT,
.evbit = { BIT(EV_ABS) },
.absbit = { BIT(ABS_X) },
},
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_ABSBIT,
.evbit = { BIT(EV_ABS) },
.absbit = { BIT(ABS_WHEEL) },
},
{
- .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+ INPUT_DEVICE_ID_MATCH_ABSBIT,
.evbit = { BIT(EV_ABS) },
.absbit = { BIT(ABS_THROTTLE) },
},
@@ -634,14 +875,14 @@ static const struct input_device_id joyd
MODULE_DEVICE_TABLE(input, joydev_ids);
static struct input_handler joydev_handler = {
- .event = joydev_event,
- .connect = joydev_connect,
- .disconnect = joydev_disconnect,
- .fops = &joydev_fops,
- .minor = JOYDEV_MINOR_BASE,
- .name = "joydev",
- .id_table = joydev_ids,
- .blacklist = joydev_blacklist,
+ .event = joydev_event,
+ .connect = joydev_connect,
+ .disconnect = joydev_disconnect,
+ .fops = &joydev_fops,
+ .minor = JOYDEV_MINOR_BASE,
+ .name = "joydev",
+ .id_table = joydev_ids,
+ .blacklist = joydev_blacklist,
};
static int __init joydev_init(void)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC/RFT 0/5] Input locking patches
2007-07-24 4:45 [RFC/RFT 0/5] Input locking patches Dmitry Torokhov
` (4 preceding siblings ...)
2007-07-24 4:45 ` [RFC/RFT 5/5] Input: joydev " Dmitry Torokhov
@ 2007-07-27 22:25 ` Indan Zupancic
2007-07-29 3:38 ` Dmitry Torokhov
5 siblings, 1 reply; 15+ messages in thread
From: Indan Zupancic @ 2007-07-27 22:25 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
On Tue, July 24, 2007 06:45, Dmitry Torokhov wrote:
> Hi everyone,
>
> I finally managed to put together some patches implementing
> locking in input core and main input handles. Please look
> over them and give them a spin.
Since kernel 2.6.21 or so I was annoyed by a warping mouse, and
one kernel version later also by "stuck" keys, causing repeated input
at the most inconvenient moments (e.g. when opening a program by
pressing F1).
As it happened irregularly and unpredictable it was hard to debug,
and I suspected faulty hardware. My cpu was quite hot, but after
removing all the dust it seems all right again. Unfortunately that
was about the same time I upgraded to 2.6.23-rc1, so all I can say
is that the stuck key problem seems to be gone, though not sure
thanks to what, but that neither the cleaning nor the upgrade fixed
the warping mouse problem.
I'm running with these locking patches for two days now and the
mouse doesn't warp any more (it can also have fixed the stuck key
problem, not sure). Normally it would warp several times a day,
and that didn't happen yet, so I'm tempted to praise your patches.
Sorry for the babbling, just wanted to say that I've tested these
patches and that they seem to fix real problems.
Thanks,
Indan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC/RFT 0/5] Input locking patches
2007-07-27 22:25 ` [RFC/RFT 0/5] Input locking patches Indan Zupancic
@ 2007-07-29 3:38 ` Dmitry Torokhov
2007-07-29 12:15 ` Indan Zupancic
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2007-07-29 3:38 UTC (permalink / raw)
To: Indan Zupancic; +Cc: linux-input, linux-kernel
Hi Indan,
On Friday 27 July 2007 18:25, Indan Zupancic wrote:
> Sorry for the babbling, just wanted to say that I've tested these
> patches and that they seem to fix real problems.
>
Thank you for testing the patches.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC/RFT 0/5] Input locking patches
2007-07-29 3:38 ` Dmitry Torokhov
@ 2007-07-29 12:15 ` Indan Zupancic
0 siblings, 0 replies; 15+ messages in thread
From: Indan Zupancic @ 2007-07-29 12:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
On Sun, July 29, 2007 05:38, Dmitry Torokhov wrote:
> Hi Indan,
>
> On Friday 27 July 2007 18:25, Indan Zupancic wrote:
>> Sorry for the babbling, just wanted to say that I've tested these
>> patches and that they seem to fix real problems.
>>
>
> Thank you for testing the patches.
Unfortunately I spoke too soon and some mouse warps occurred
yesterday. I suspect faulty hardware, so I'll try with my old
non-optical mouse. If it still happens then it's either the mobo
or the kernel. In the meantime I'll plug this one in another pc
and see if the warps happen there. After that it's running old
kernels to see if the warps happen there too. Any other ideas
to check whether it's a software or hardware problem?
The more annoying sticking key problem is gone for good though.
Greetings,
Indan
^ permalink raw reply [flat|nested] 15+ messages in thread