Re: [PATCH] Fix effect aborting in ff-memless
flat view
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-06-20 17:40:26
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?
quoted hunk ↗ jump to hunk
My patch fixes this by setting the trigger times to "now" after setting the aborting flag. This way the following code deals with aborting the effect immediately without setting a timer for it. There may be other ways to fix this, so I would be happy to get some feedback. Signed-off-by: Manuel Reimer <redacted>--- a/drivers/input/ff-memless.c 2016-05-13 16:06:29.722685021 +0200 +++ b/drivers/input/ff-memless.c 2016-06-19 14:25:39.790375270 +0200@@ -463,9 +463,11 @@ static int ml_ff_playback(struct input_d } else { pr_debug("initiated stop\n"); - if (test_bit(FF_EFFECT_PLAYING, &state->flags)) + if (test_bit(FF_EFFECT_PLAYING, &state->flags)) { __set_bit(FF_EFFECT_ABORTING, &state->flags); - else + state->play_at = jiffies; + state->adj_at = jiffies; + } else __clear_bit(FF_EFFECT_STARTED, &state->flags); }
Thanks. -- Dmitry