[review,1/6] radio-mr800: remove redundant lock/unlock_kernel
Commit Message
Remove redundant lock/unlock_kernel() calls from usb_amradio_open/close
functions.
Signed-off-by: Alexey Klimov <klimov.linux@gmail.com>
--
Comments
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
@@ -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;
}