Message ID | 1249753533.15160.241.camel@tux.localhost (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Douglas Landgraf |
Headers | show |
On Sat, Aug 8, 2009 at 1:45 PM, Alexey Klimov<klimov.linux@gmail.com> wrote: > Remove redundant lock/unlock_kernel() calls from usb_amradio_open/close > functions. > > Signed-off-by: Alexey Klimov <klimov.linux@gmail.com> > > -- > diff -r ee6cf88cb5d3 linux/drivers/media/radio/radio-mr800.c > --- a/linux/drivers/media/radio/radio-mr800.c  Wed Jul 29 01:42:02 2009 -0300 > +++ b/linux/drivers/media/radio/radio-mr800.c  Wed Jul 29 10:44:02 2009 +0400 > @@ -540,8 +540,6 @@ >     struct amradio_device *radio = video_get_drvdata(video_devdata(file)); >     int retval; > > -    lock_kernel(); > - Maybe I'm missing something here, but the lock_kernel() call seems very necessary here since you're modifying the state of the driver without taking the driver's lock. Last I checked the v4l2 subsystem doesn't call open with it's lock held. So by removing these locks you've introduced a race condition between open and close. >     radio->users = 1; >     radio->muted = 1; > > @@ -550,7 +548,6 @@ >         amradio_dev_warn(&radio->videodev->dev, >             "radio did not start up properly\n"); >         radio->users = 0; > -        unlock_kernel(); >         return -EIO; >     } > > @@ -564,7 +561,6 @@ >         amradio_dev_warn(&radio->videodev->dev, >             "set frequency failed\n"); > > -    unlock_kernel(); >     return 0; >  } > > > > -- > Best regards, Klimov Alexey > Regards, David Ellingsworth -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -r ee6cf88cb5d3 linux/drivers/media/radio/radio-mr800.c --- a/linux/drivers/media/radio/radio-mr800.c Wed Jul 29 01:42:02 2009 -0300 +++ b/linux/drivers/media/radio/radio-mr800.c Wed Jul 29 10:44:02 2009 +0400 @@ -540,8 +540,6 @@ struct amradio_device *radio = video_get_drvdata(video_devdata(file)); int retval; - lock_kernel(); - radio->users = 1; radio->muted = 1; @@ -550,7 +548,6 @@ amradio_dev_warn(&radio->videodev->dev, "radio did not start up properly\n"); radio->users = 0; - unlock_kernel(); return -EIO; } @@ -564,7 +561,6 @@ amradio_dev_warn(&radio->videodev->dev, "set frequency failed\n"); - unlock_kernel(); return 0; }
Remove redundant lock/unlock_kernel() calls from usb_amradio_open/close functions. Signed-off-by: Alexey Klimov <klimov.linux@gmail.com> --