Is this a preempt issue in drivers/input/evdev.c

Is this a preempt issue in drivers/input/evdev.c

[Yin,Fengwei]

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


	

Re: Is this a preempt issue in drivers/input/evdev.c

[Dmitry Torokhov]

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

Re: Is this a preempt issue in drivers/input/evdev.c

[Jens Axboe]

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