2007-05-12 03:18:45

by Fengwei Yin

[permalink] [raw]
Subject: Is this a preempt issue in drivers/input/evdev.c

Hi,
When open/close evdev, the code is as following to handle multi-client
operation:
static int evdev_release(...)
{
...
if (!--evdev->open) {
<------------preempted
if (evdev->exist)
input_close_device(...);
else
evdev_free(evdev);
}
return 0;
}

static int evdev_open(...)
{
...
if (!evdev->open++ && evdev->exist){
errror = input_open_device(...);
if (error) {
...
}
}
...
return 0;
}

So if the evdev_release() is preempted at the point marked by another
process which will open the evdev, which will make operation sequence
like:

--evdev->open in evdev_release()
-----> preempted
evdev->open++ and input_open_devie()
<----- reschedule
input_close_device()

Should we introduce a mutex here? Or do I miss something? Thanks.

Regards
Yin, Fengwei







2007-05-12 05:05:46

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: Is this a preempt issue in drivers/input/evdev.c

Hi,

On Friday 11 May 2007 23:18, Yin,Fengwei wrote:
>
> So if the evdev_release() is preempted at the point marked by another
> process which will open the evdev, which will make operation sequence
> like:
>
> --evdev->open in evdev_release()
> -----> preempted
> evdev->open++ and input_open_devie()
> <----- reschedule
> input_close_device()
>
> Should we introduce a mutex here? Or do I miss something? Thanks.
>

Locking is completely absent in evdev. There was a patch introducing
locking in recent -mm's but it got dropped. I need to refresh it.

--
Dmitry

2007-05-13 18:13:36

by Jens Axboe

[permalink] [raw]
Subject: Re: Is this a preempt issue in drivers/input/evdev.c

On Sat, May 12 2007, Dmitry Torokhov wrote:
> Hi,
>
> On Friday 11 May 2007 23:18, Yin,Fengwei wrote:
> >
> > So if the evdev_release() is preempted at the point marked by another
> > process which will open the evdev, which will make operation sequence
> > like:
> >
> > --evdev->open in evdev_release()
> > -----> preempted
> > evdev->open++ and input_open_devie()
> > <----- reschedule
> > input_close_device()
> >
> > Should we introduce a mutex here? Or do I miss something? Thanks.
> >
>
> Locking is completely absent in evdev. There was a patch introducing
> locking in recent -mm's but it got dropped. I need to refresh it.

How did stuff like that ever get merged, btw? I still have a completely
trivially oopsable rmmod psmouse problem, I hope it'll get fixed for
2.6.22.

--
Jens Axboe