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? > > 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 <mail@xxxxxxxxxxx> > > --- 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 -- 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
