From a184cf98b1d4397fe7eca881da596059fea36a18 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 3 Jul 2024 14:37:49 -0700 Subject: Input: make sure input handlers define only one processing method Input core expects input handlers to be either filters, or regular handlers, but not both. Additionally, for regular handlers it does not make sense to define both single event method and batch method. Refuse registering handler if it defines more than one method. Reviewed-by: Jeff LaBundy Reviewed-by: Benjamin Tissoires Link: https://lore.kernel.org/r/20240703213756.3375978-3-dmitry.torokhov@gmail.com Signed-off-by: Dmitry Torokhov --- drivers/input/input.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'drivers/input/input.c') diff --git a/drivers/input/input.c b/drivers/input/input.c index fd4997ba263c..7e4f8824f4fd 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -2517,6 +2517,26 @@ void input_unregister_device(struct input_dev *dev) } EXPORT_SYMBOL(input_unregister_device); +static int input_handler_check_methods(const struct input_handler *handler) +{ + int count = 0; + + if (handler->filter) + count++; + if (handler->events) + count++; + if (handler->event) + count++; + + if (count > 1) { + pr_err("%s: only one event processing method can be defined (%s)\n", + __func__, handler->name); + return -EINVAL; + } + + return 0; +} + /** * input_register_handler - register a new input handler * @handler: handler to be registered @@ -2530,6 +2550,10 @@ int input_register_handler(struct input_handler *handler) struct input_dev *dev; int error; + error = input_handler_check_methods(handler); + if (error) + return error; + error = mutex_lock_interruptible(&input_mutex); if (error) return error; -- cgit v1.3 From d469647bafd9353730e0f74ec5fbefcd431c576b Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 3 Jul 2024 14:37:51 -0700 Subject: Input: simplify event handling logic Streamline event handling code by providing batch implementations for filtering and event processing and using them in place of the main event handler, as needed, instead of having complex branching logic in the middle of the event processing code. Reviewed-by: Jeff LaBundy Reviewed-by: Benjamin Tissoires Link: https://lore.kernel.org/r/20240703213756.3375978-5-dmitry.torokhov@gmail.com Signed-off-by: Dmitry Torokhov --- drivers/input/input.c | 109 +++++++++++++++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 41 deletions(-) (limited to 'drivers/input/input.c') diff --git a/drivers/input/input.c b/drivers/input/input.c index 7e4f8824f4fd..40a04154f99d 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -99,45 +99,13 @@ static void input_stop_autorepeat(struct input_dev *dev) del_timer(&dev->timer); } -/* - * Pass event first through all filters and then, if event has not been - * filtered out, through all open handles. This function is called with - * dev->event_lock held and interrupts disabled. - */ -static unsigned int input_to_handler(struct input_handle *handle, - struct input_value *vals, unsigned int count) -{ - struct input_handler *handler = handle->handler; - struct input_value *end = vals; - struct input_value *v; - - if (handler->filter) { - for (v = vals; v != vals + count; v++) { - if (handler->filter(handle, v->type, v->code, v->value)) - continue; - if (end != v) - *end = *v; - end++; - } - count = end - vals; - } - - if (!count) - return 0; - - if (handler->events) - handler->events(handle, vals, count); - else if (handler->event) - for (v = vals; v != vals + count; v++) - handler->event(handle, v->type, v->code, v->value); - - return count; -} - /* * Pass values first through all filters and then, if event has not been - * filtered out, through all open handles. This function is called with - * dev->event_lock held and interrupts disabled. + * filtered out, through all open handles. This order is achieved by placing + * filters at the head of the list of handles attached to the device, and + * placing regular handles at the tail of the list. + * + * This function is called with dev->event_lock held and interrupts disabled. */ static void input_pass_values(struct input_dev *dev, struct input_value *vals, unsigned int count) @@ -154,11 +122,12 @@ static void input_pass_values(struct input_dev *dev, handle = rcu_dereference(dev->grab); if (handle) { - count = input_to_handler(handle, vals, count); + count = handle->handler->events(handle, vals, count); } else { list_for_each_entry_rcu(handle, &dev->h_list, d_node) if (handle->open) { - count = input_to_handler(handle, vals, count); + count = handle->handler->events(handle, vals, + count); if (!count) break; } @@ -2537,6 +2506,57 @@ static int input_handler_check_methods(const struct input_handler *handler) return 0; } +/* + * An implementation of input_handler's events() method that simply + * invokes handler->event() method for each event one by one. + */ +static unsigned int input_handler_events_default(struct input_handle *handle, + struct input_value *vals, + unsigned int count) +{ + struct input_handler *handler = handle->handler; + struct input_value *v; + + for (v = vals; v != vals + count; v++) + handler->event(handle, v->type, v->code, v->value); + + return count; +} + +/* + * An implementation of input_handler's events() method that invokes + * handler->filter() method for each event one by one and removes events + * that were filtered out from the "vals" array. + */ +static unsigned int input_handler_events_filter(struct input_handle *handle, + struct input_value *vals, + unsigned int count) +{ + struct input_handler *handler = handle->handler; + struct input_value *end = vals; + struct input_value *v; + + for (v = vals; v != vals + count; v++) { + if (handler->filter(handle, v->type, v->code, v->value)) + continue; + if (end != v) + *end = *v; + end++; + } + + return end - vals; +} + +/* + * An implementation of input_handler's events() method that does nothing. + */ +static unsigned int input_handler_events_null(struct input_handle *handle, + struct input_value *vals, + unsigned int count) +{ + return count; +} + /** * input_register_handler - register a new input handler * @handler: handler to be registered @@ -2554,12 +2574,19 @@ int input_register_handler(struct input_handler *handler) if (error) return error; + INIT_LIST_HEAD(&handler->h_list); + + if (handler->filter) + handler->events = input_handler_events_filter; + else if (handler->event) + handler->events = input_handler_events_default; + else if (!handler->events) + handler->events = input_handler_events_null; + error = mutex_lock_interruptible(&input_mutex); if (error) return error; - INIT_LIST_HEAD(&handler->h_list); - list_add_tail(&handler->node, &input_handler_list); list_for_each_entry(dev, &input_dev_list, node) -- cgit v1.3 From 3544cf574a577d92111f0b29e6d649b7ea3210ed Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 3 Jul 2024 14:37:52 -0700 Subject: Input: rearrange input_alloc_device() to prepare for preallocating of vals In preparation to have dev->vals memory pre-allocated rearrange code in input_alloc_device() so that it allows handling multiple points of failure. Reviewed-by: Jeff LaBundy Reviewed-by: Benjamin Tissoires Link: https://lore.kernel.org/r/20240703213756.3375978-6-dmitry.torokhov@gmail.com Signed-off-by: Dmitry Torokhov --- drivers/input/input.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) (limited to 'drivers/input/input.c') diff --git a/drivers/input/input.c b/drivers/input/input.c index 40a04154f99d..9981fdfaee9f 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -1982,21 +1982,28 @@ struct input_dev *input_allocate_device(void) struct input_dev *dev; dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (dev) { - dev->dev.type = &input_dev_type; - dev->dev.class = &input_class; - device_initialize(&dev->dev); - mutex_init(&dev->mutex); - spin_lock_init(&dev->event_lock); - timer_setup(&dev->timer, NULL, 0); - INIT_LIST_HEAD(&dev->h_list); - INIT_LIST_HEAD(&dev->node); - - dev_set_name(&dev->dev, "input%lu", - (unsigned long)atomic_inc_return(&input_no)); - - __module_get(THIS_MODULE); - } + if (!dev) + return NULL; + + mutex_init(&dev->mutex); + spin_lock_init(&dev->event_lock); + timer_setup(&dev->timer, NULL, 0); + INIT_LIST_HEAD(&dev->h_list); + INIT_LIST_HEAD(&dev->node); + + dev->dev.type = &input_dev_type; + dev->dev.class = &input_class; + device_initialize(&dev->dev); + /* + * From this point on we can no longer simply "kfree(dev)", we need + * to use input_free_device() so that device core properly frees its + * resources associated with the input device. + */ + + dev_set_name(&dev->dev, "input%lu", + (unsigned long)atomic_inc_return(&input_no)); + + __module_get(THIS_MODULE); return dev; } -- cgit v1.3 From 0cd58773520584ccb4ce1eeebd8d43f1b27bb24a Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 3 Jul 2024 14:37:53 -0700 Subject: Input: preallocate memory to hold event values Preallocate memory for holding event values (input_dev->vals) so that there is no need to check if it was allocated or not in the event processing code. The amount of memory will be adjusted after input device has been fully set up upon registering device with the input core. Reviewed-by: Jeff LaBundy Reviewed-by: Benjamin Tissoires Link: https://lore.kernel.org/r/20240703213756.3375978-7-dmitry.torokhov@gmail.com Signed-off-by: Dmitry Torokhov --- drivers/input/input.c | 61 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 17 deletions(-) (limited to 'drivers/input/input.c') diff --git a/drivers/input/input.c b/drivers/input/input.c index 9981fdfaee9f..4e12fa79883e 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -323,9 +323,6 @@ static void input_event_dispose(struct input_dev *dev, int disposition, if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event) dev->event(dev, type, code, value); - if (!dev->vals) - return; - if (disposition & INPUT_PASS_TO_HANDLERS) { struct input_value *v; @@ -1985,6 +1982,18 @@ struct input_dev *input_allocate_device(void) if (!dev) return NULL; + /* + * Start with space for SYN_REPORT + 7 EV_KEY/EV_MSC events + 2 spare, + * see input_estimate_events_per_packet(). We will tune the number + * when we register the device. + */ + dev->max_vals = 10; + dev->vals = kcalloc(dev->max_vals, sizeof(*dev->vals), GFP_KERNEL); + if (!dev->vals) { + kfree(dev); + return NULL; + } + mutex_init(&dev->mutex); spin_lock_init(&dev->event_lock); timer_setup(&dev->timer, NULL, 0); @@ -2344,6 +2353,35 @@ bool input_device_enabled(struct input_dev *dev) } EXPORT_SYMBOL_GPL(input_device_enabled); +static int input_device_tune_vals(struct input_dev *dev) +{ + struct input_value *vals; + unsigned int packet_size; + unsigned int max_vals; + + packet_size = input_estimate_events_per_packet(dev); + if (dev->hint_events_per_packet < packet_size) + dev->hint_events_per_packet = packet_size; + + max_vals = dev->hint_events_per_packet + 2; + if (dev->max_vals >= max_vals) + return 0; + + vals = kcalloc(max_vals, sizeof(*vals), GFP_KERNEL); + if (!vals) + return -ENOMEM; + + spin_lock_irq(&dev->event_lock); + dev->max_vals = max_vals; + swap(dev->vals, vals); + spin_unlock_irq(&dev->event_lock); + + /* Because of swap() above, this frees the old vals memory */ + kfree(vals); + + return 0; +} + /** * input_register_device - register device with input core * @dev: device to be registered @@ -2371,7 +2409,6 @@ int input_register_device(struct input_dev *dev) { struct input_devres *devres = NULL; struct input_handler *handler; - unsigned int packet_size; const char *path; int error; @@ -2399,16 +2436,9 @@ int input_register_device(struct input_dev *dev) /* Make sure that bitmasks not mentioned in dev->evbit are clean. */ input_cleanse_bitmasks(dev); - packet_size = input_estimate_events_per_packet(dev); - if (dev->hint_events_per_packet < packet_size) - dev->hint_events_per_packet = packet_size; - - dev->max_vals = dev->hint_events_per_packet + 2; - dev->vals = kcalloc(dev->max_vals, sizeof(*dev->vals), GFP_KERNEL); - if (!dev->vals) { - error = -ENOMEM; + error = input_device_tune_vals(dev); + if (error) goto err_devres_free; - } /* * If delay and period are pre-set by the driver, then autorepeating @@ -2428,7 +2458,7 @@ int input_register_device(struct input_dev *dev) error = device_add(&dev->dev); if (error) - goto err_free_vals; + goto err_devres_free; path = kobject_get_path(&dev->dev.kobj, GFP_KERNEL); pr_info("%s as %s\n", @@ -2458,9 +2488,6 @@ int input_register_device(struct input_dev *dev) err_device_del: device_del(&dev->dev); -err_free_vals: - kfree(dev->vals); - dev->vals = NULL; err_devres_free: devres_free(devres); return error; -- cgit v1.3 From 735877fde06304ae9d90e17102dc2b139e8d802a Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 3 Jul 2024 14:37:54 -0700 Subject: Input: do not check number of events in input_pass_values() Now that the input_dev->vals array is always there we can be assured that input_pass_values() is always called with a non-0 number of events. Remove the check. Reviewed-by: Jeff LaBundy Reviewed-by: Benjamin Tissoires Link: https://lore.kernel.org/r/20240703213756.3375978-8-dmitry.torokhov@gmail.com Signed-off-by: Dmitry Torokhov --- drivers/input/input.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/input/input.c') diff --git a/drivers/input/input.c b/drivers/input/input.c index 4e12fa79883e..54c57b267b25 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -115,9 +115,6 @@ static void input_pass_values(struct input_dev *dev, lockdep_assert_held(&dev->event_lock); - if (!count) - return; - rcu_read_lock(); handle = rcu_dereference(dev->grab); -- cgit v1.3