On Mon, Jun 20, 2016 at 10:33:03AM -0700, Dmitry Torokhov wrote: > On Sun, Jun 19, 2016 at 02:44:35PM +0200, Manuel Reimer wrote: > > Hello, > > > > while debugging a problem with hid-sony I got stuck with a problem > > actually caused by ff-memless. In some situations effect aborting is > > delayed, so it may be triggered seconds after all devices have been > > destroyed, which causes the kernel to panic. > > > > The aborting request actually gets received here: > > https://github.com/torvalds/linux/blob/master/drivers/input/ff-memless.c#L467 > > This "aborting" flag is then handled here: > > https://github.com/torvalds/linux/blob/master/drivers/input/ff-memless.c#L376 > > But before this line is reached, there is a time check to check if > > the effect actually is due to be started: > > https://github.com/torvalds/linux/blob/master/drivers/input/ff-memless.c#L359 > > > > This time check now causes a problem if the effect, which is meant > > to be *aborted* was scheduled to be *started* some time in future > > and the device is destroyed before this time is reached. > > I am not clear how this can happen. If effect hasn't actually started > playing (i.e. we have FF_EFFECT_STARTED bit set, but FF_EFFECT_PLAYING > is not yet set), then when stopping effect we do not need to do anything > except clear FF_EFFECT_STARTED (since we did not touch the hardware > yet). > > Now, if FF_EFFECT_PLAYING is set, that means that play_at time is in > the past and we won't be skipping this effect in ml_get_combo_effect(). > > Could you please post a stack trace of the crash you observed? Actually, I wonder if the patch below will fix the issue you are seeing. Thanks. -- Dmitry Input: fix flushing of FF effects when unregistering devices From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> --- drivers/input/input.c | 10 +++++++--- include/linux/input.h | 8 ++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/input/input.c b/drivers/input/input.c index d95c34e..220aa17 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -144,7 +144,7 @@ static void input_pass_values(struct input_dev *dev, count = input_to_handler(handle, vals, count); } else { list_for_each_entry_rcu(handle, &dev->h_list, d_node) - if (handle->open) { + if (handle->active) { count = input_to_handler(handle, vals, count); if (!count) break; @@ -552,7 +552,7 @@ static void __input_release_device(struct input_handle *handle) synchronize_rcu(); list_for_each_entry(handle, &dev->h_list, d_node) - if (handle->open && handle->handler->start) + if (handle->active && handle->handler->start) handle->handler->start(handle); } } @@ -613,6 +613,8 @@ int input_open_device(struct input_handle *handle) } } + handle->active = handle->open > 0; + out: mutex_unlock(&dev->mutex); return retval; @@ -663,6 +665,8 @@ void input_close_device(struct input_handle *handle) synchronize_rcu(); } + handle->active = handle->open > 0; + mutex_unlock(&dev->mutex); } EXPORT_SYMBOL(input_close_device); @@ -716,7 +720,7 @@ static void input_disconnect_device(struct input_dev *dev) input_dev_release_keys(dev); list_for_each_entry(handle, &dev->h_list, d_node) - handle->open = 0; + handle->active = false; spin_unlock_irq(&dev->event_lock); } diff --git a/include/linux/input.h b/include/linux/input.h index 1e96769..87ec2f7 100644 --- a/include/linux/input.h +++ b/include/linux/input.h @@ -307,8 +307,10 @@ struct input_handler { /** * 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 + * @open: counter showing whether the handle is 'open', i.e. there is + * a consumer for the events from input device + * @active: indicates that input events should be delivered from input + * device to input handler through this handle * @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 @@ -321,6 +323,8 @@ struct input_handle { void *private; int open; + bool active; + const char *name; struct input_dev *dev; -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html