Dmitry Torokhov wrote:
> On Monday 05 June 2006 17:11, Anssi Hannula wrote:
>
>>Dmitry Torokhov wrote:
>>
>>>On 5/30/06, Anssi Hannula <anssi.hannula@xxxxxxxxx> wrote:
>>>>--- linux-2.6.17-rc4-git12.orig/drivers/input/input.c 2006-05-27
>>>>02:28:57.000000000 +0300
>>>>+++ linux-2.6.17-rc4-git12/drivers/input/input.c 2006-05-27
>>>>02:38:35.000000000 +0300
>>>>@@ -733,6 +733,17 @@ static void input_dev_release(struct cla
>>>> {
>>>> struct input_dev *dev = to_input_dev(class_dev);
>>>>
>>>>+ if (dev->ff) {
>>>>+ struct ff_device *ff = dev->ff;
>>>>+ clear_bit(EV_FF, dev->evbit);
>>>>+ mutex_lock(&dev->ff_lock);
>>>>+ del_timer_sync(&ff->timer);
>>>
>>>
>>>This is too late. We need to stop timer when device gets unregistered.
>>
>>And what if driver has called input_allocate_device(),
>>input_ff_allocate(), input_ff_register(), but then decides to abort and
>>calls input_dev_release()? input_unregister_device() would not get
>>called at all.
>>
>
>
> Right, but if device was never registered there is no device node so noone
> could start the timer and deleting it is a noop. Hmm, I think even better
> place would be to stop ff timer when device is closed (i.e. when last user
> closes file handle).
>
Hmm... actually, they are stopped in flush(), and IIRC that is always
called before deleting input_dev.
>
>>>Clearing FF bits is pointless here as device is about to disappear;
>>>locking is also not needed because we are guaranteed to be the last
>>>user of the device structure.
>>
>>True, if that guarantee really exists.
>>
> Yes, this is guaranteed.
>
So, now you guarantee it, but it isn't really so? ;)
When we remove locking, timer_del, clear_bit, all that is left is
kfree() and I guess that has to still be run in the input_dev_release().