Re: [PATCH 0/5] Pushdown bkl from v4l ioctls

From: Hans Verkuil
Date: Fri Apr 30 2010 - 21:03:17 EST

Next message: Michael Cree: "[PATCH 1/3] alpha: Add performance monitor interrupt counter."
Previous message: H Hartley Sweeten: "[PATCH] sst25l.c: fix multi-part messages with broken spi masters"
In reply to: Frederic Weisbecker: "[PATCH 0/5] Pushdown bkl from v4l ioctls"
Next in thread: Laurent Pinchart: "Re: [PATCH 0/5] Pushdown bkl from v4l ioctls"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thursday 29 April 2010 05:42:39 Frederic Weisbecker wrote:
> Hi,
>
> Linus suggested to rename struct v4l2_file_operations::ioctl
> into bkl_ioctl to eventually get something greppable and make
> its background explicit.
>
> While at it I thought it could be a good idea to just pushdown
> the bkl to every v4l drivers that have an .ioctl, so that we
> actually remove struct v4l2_file_operations::ioctl for good.
>
> It passed make allyesconfig on sparc.
> Please tell me what you think.

I much prefer to keep the bkl inside the v4l2 core. One reason is that I
think that we can replace the bkl in the core with a mutex. Still not
ideal of course, so the next step will be to implement proper locking in
each driver. For this some additional v4l infrastructure work needs to be
done. I couldn't proceed with that until the v4l events API patches went in,
and that happened yesterday.

So from my point of view the timeline is this:

1) I do the infrastructure work this weekend. This will make it much easier to
convert drivers to do proper locking. And it will also simplify v4l2_priority
handling, so I'm killing two birds with one stone :-)

2) Wait until Arnd's patch gets merged that pushes the bkl down to v4l2-dev.c

3) Investigate what needs to be done to replace the bkl with a v4l2-dev.c
global mutex. Those drivers that call the bkl themselves should probably be
converted to do proper locking, but there are only about 14 drivers that do
this. The other 60 or so drivers should work fine if a v4l2-dev global lock
is used. At this point the bkl is effectively removed from the v4l subsystem.

4) Work on the remaining 60 drivers to do proper locking and get rid of the
v4l2-dev global lock. This is probably less work than it sounds.

Since your patch moves everything down to the driver level it will actually
make this work harder rather than easier. And it touches almost all drivers
as well.

Regards,

Hans

>
> Thanks.
>
> Frederic Weisbecker (5):
> v4l: Pushdown bkl into video_ioctl2
> v4l: Use video_ioctl2_unlocked from drivers that don't want the bkl
> v4l: Change users of video_ioctl2 to use unlocked_ioctl
> v4l: Pushdown bkl to drivers that implement their own ioctl
> v4l: Remove struct v4l2_file_operations::ioctl
>
> drivers/media/common/saa7146_fops.c | 2 +-
> drivers/media/radio/dsbr100.c | 2 +-
> drivers/media/radio/radio-aimslab.c | 2 +-
> drivers/media/radio/radio-aztech.c | 2 +-
> drivers/media/radio/radio-cadet.c | 2 +-
> drivers/media/radio/radio-gemtek-pci.c | 2 +-
> drivers/media/radio/radio-gemtek.c | 2 +-
> drivers/media/radio/radio-maestro.c | 2 +-
> drivers/media/radio/radio-maxiradio.c | 2 +-
> drivers/media/radio/radio-miropcm20.c | 2 +-
> drivers/media/radio/radio-mr800.c | 2 +-
> drivers/media/radio/radio-rtrack2.c | 2 +-
> drivers/media/radio/radio-sf16fmi.c | 2 +-
> drivers/media/radio/radio-sf16fmr2.c | 2 +-
> drivers/media/radio/radio-si4713.c | 2 +-
> drivers/media/radio/radio-tea5764.c | 2 +-
> drivers/media/radio/radio-terratec.c | 2 +-
> drivers/media/radio/radio-timb.c | 2 +-
> drivers/media/radio/radio-trust.c | 2 +-
> drivers/media/radio/radio-typhoon.c | 2 +-
> drivers/media/radio/radio-zoltrix.c | 2 +-
> drivers/media/radio/si470x/radio-si470x-common.c | 2 +-
> drivers/media/video/arv.c | 2 +-
> drivers/media/video/au0828/au0828-video.c | 14 ++++----
> drivers/media/video/bt8xx/bttv-driver.c | 26 +++++++-------
> drivers/media/video/bw-qcam.c | 11 +++++-
> drivers/media/video/c-qcam.c | 11 +++++-
> drivers/media/video/cafe_ccic.c | 14 ++++----
> drivers/media/video/cpia.c | 11 +++++-
> drivers/media/video/cpia2/cpia2_v4l.c | 11 +++++-
> drivers/media/video/cx18/cx18-streams.c | 12 +++---
> drivers/media/video/cx231xx/cx231xx-video.c | 4 +-
> drivers/media/video/cx23885/cx23885-417.c | 2 +-
> drivers/media/video/cx23885/cx23885-video.c | 4 +-
> drivers/media/video/cx88/cx88-blackbird.c | 2 +-
> drivers/media/video/cx88/cx88-video.c | 4 +-
> drivers/media/video/davinci/vpfe_capture.c | 2 +-
> drivers/media/video/davinci/vpif_capture.c | 2 +-
> drivers/media/video/davinci/vpif_display.c | 2 +-
> drivers/media/video/em28xx/em28xx-video.c | 4 +-
> drivers/media/video/et61x251/et61x251_core.c | 27 +++++++++++----
> drivers/media/video/gspca/gspca.c | 2 +-
> drivers/media/video/hdpvr/hdpvr-video.c | 2 +-
> drivers/media/video/meye.c | 2 +-
> drivers/media/video/omap24xxcam.c | 10 +++---
> drivers/media/video/ov511.c | 15 +++++---
> drivers/media/video/pms.c | 2 +-
> drivers/media/video/pvrusb2/pvrusb2-v4l2.c | 20 +++++++----
> drivers/media/video/pwc/pwc-if.c | 19 ++++++----
> drivers/media/video/s2255drv.c | 12 +++---
> drivers/media/video/saa5246a.c | 11 ++++--
> drivers/media/video/saa5249.c | 6 +++-
> drivers/media/video/saa7134/saa7134-empress.c | 14 ++++----
> drivers/media/video/saa7134/saa7134-video.c | 26 +++++++-------
> drivers/media/video/se401.c | 20 +++++++----
> drivers/media/video/sn9c102/sn9c102_core.c | 27 +++++++++++----
> drivers/media/video/soc_camera.c | 2 +-
> drivers/media/video/stk-webcam.c | 14 ++++----
> drivers/media/video/stradis.c | 26 +++++++++++----
> drivers/media/video/stv680.c | 20 +++++++----
> drivers/media/video/tlg2300/pd-radio.c | 8 ++--
> drivers/media/video/tlg2300/pd-video.c | 2 +-
> drivers/media/video/usbvideo/usbvideo.c | 21 ++++++++----
> drivers/media/video/usbvideo/vicam.c | 14 +++++++-
> drivers/media/video/usbvision/usbvision-video.c | 4 +-
> drivers/media/video/uvc/uvc_v4l2.c | 11 +++++-
> drivers/media/video/v4l2-dev.c | 38 ++-------------------
> drivers/media/video/v4l2-ioctl.c | 17 ++++++++-
> drivers/media/video/vivi.c | 2 +-
> drivers/media/video/w9966.c | 2 +-
> drivers/media/video/w9968cf.c | 25 +++++++++++---
> drivers/media/video/zc0301/zc0301_core.c | 27 +++++++++++----
> drivers/media/video/zoran/zoran_driver.c | 16 ++++----
> drivers/media/video/zr364xx.c | 14 ++++----
> drivers/staging/cx25821/cx25821-audups11.c | 18 ++++++----
> drivers/staging/cx25821/cx25821-video0.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video1.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video2.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video3.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video4.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video5.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video6.c | 14 ++++----
> drivers/staging/cx25821/cx25821-video7.c | 14 ++++----
> drivers/staging/cx25821/cx25821-videoioctl.c | 27 +++++++++++----
> drivers/staging/cx25821/cx25821-vidups10.c | 19 +++++++----
> drivers/staging/cx25821/cx25821-vidups9.c | 18 ++++++----
> drivers/staging/dream/camera/msm_v4l2.c | 27 +++++++++++----
> drivers/staging/go7007/go7007-v4l2.c | 2 +-
> drivers/staging/tm6000/tm6000-video.c | 2 +-
> include/media/v4l2-dev.h | 1 -
> include/media/v4l2-ioctl.h | 2 +
> sound/i2c/other/tea575x-tuner.c | 2 +-
> 92 files changed, 530 insertions(+), 360 deletions(-)
>

--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Michael Cree: "[PATCH 1/3] alpha: Add performance monitor interrupt counter."
Previous message: H Hartley Sweeten: "[PATCH] sst25l.c: fix multi-part messages with broken spi masters"
In reply to: Frederic Weisbecker: "[PATCH 0/5] Pushdown bkl from v4l ioctls"
Next in thread: Laurent Pinchart: "Re: [PATCH 0/5] Pushdown bkl from v4l ioctls"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]