Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hans Verkuil]

Hi all,

Now that the BKL patch series has been merged in 2.6.37 it is time to work
on replacing .ioctl by .unlocked_ioctl in all v4l drivers.

I've made an inventory of all drivers that still use .ioctl and I am looking
for volunteers to tackle one or more drivers.

I have CCed this email to the maintainers of the various drivers (if I know
who it is) in the hope that we can get this conversion done as quickly as
possible.

If I have added your name to a driver, then please confirm if you are able to
work on it or not. If you can't work on it, but you know someone else, then
let me know as well.

There is also a list of drivers where I do not know who can do the conversion.
If you can tackle one or more of those, please respond. Unfortunately, those
are among the hardest to convert :-(

It would be great if we can tackle most of these drivers for 2.6.38. I think
we should finish all drivers for 2.6.39 at the latest.

There are two ways of doing the conversion: one is to do all the locking within
the driver, the other is to use core-assisted locking. How to do the core-assisted
locking is described in Documentation/video4linux/v4l2-framework.txt, but I'll
repeat the relevant part here:

v4l2_file_operations and locking
--------------------------------

You can set a pointer to a mutex_lock in struct video_device. Usually this
will be either a top-level mutex or a mutex per device node. If you want
finer-grained locking then you have to set it to NULL and do you own locking.

If a lock is specified then all file operations will be serialized on that
lock. If you use videobuf then you must pass the same lock to the videobuf
queue initialize function: if videobuf has to wait for a frame to arrive, then
it will temporarily unlock the lock and relock it afterwards. If your driver
also waits in the code, then you should do the same to allow other processes
to access the device node while the first process is waiting for something.

The implementation of a hotplug disconnect should also take the lock before
calling v4l2_device_disconnect.


Driver list:

saa7146 (Hans Verkuil)
mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
cx23885 (Steve Toth)
cx18-alsa (Andy Walls)
omap24xxcam (Sakari Ailus or David Cohen)
au0828 (Janne Grunau)
cpia2 (Andy Walls or Hans Verkuil)
cx231xx (Mauro Carvalho Chehab)
davinci (Muralidharan Karicheri)
saa6588 (Hans Verkuil)
pvrusb2 (Mike Isely)
usbvision (Hans Verkuil)
s5p-fimc (Sylwester Nawrocki)
fsl-viu (Anatolij Gustschin)
tlg2300 (Mauro Carvalho Chehab)
zr364xx (Hans de Goede)
soc_camera (Guennadi Liakhovetski)
usbvideo/vicam (Hans de Goede)
s2255drv (Pete Eberlein)
bttv (Mauro Carvalho Chehab)
stk-webcam (Hans de Goede)
se401 (Hans de Goede)
si4713-i2c (Hans Verkuil)
dsbr100 (Hans Verkuil)

Staging driver list:

go7007 (Pete Eberlein)
tm6000 (Mauro Carvalho Chehab)
(stradis/cpia: will be removed in 2.6.38, so no need to do anything)

Unassigned drivers:

saa7134
em28xx
cx88
solo6x10 (staging driver)

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hans Verkuil]

On Saturday, December 18, 2010 12:31:26 Hans Verkuil wrote:
> Driver list:
> 
> saa7146 (Hans Verkuil)
> mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> cx23885 (Steve Toth)
> cx18-alsa (Andy Walls)
> omap24xxcam (Sakari Ailus or David Cohen)
> au0828 (Janne Grunau)
> cpia2 (Andy Walls or Hans Verkuil)
> cx231xx (Mauro Carvalho Chehab)
> davinci (Muralidharan Karicheri)
> saa6588 (Hans Verkuil)
> pvrusb2 (Mike Isely)
> usbvision (Hans Verkuil)
> s5p-fimc (Sylwester Nawrocki)
> fsl-viu (Anatolij Gustschin)
> tlg2300 (Mauro Carvalho Chehab)
> zr364xx (Hans de Goede)
> soc_camera (Guennadi Liakhovetski)
> usbvideo/vicam (Hans de Goede)
> s2255drv (Pete Eberlein)
> bttv (Mauro Carvalho Chehab)
> stk-webcam (Hans de Goede)
> se401 (Hans de Goede)
> si4713-i2c (Hans Verkuil)
> dsbr100 (Hans Verkuil)

Oops, si4713-i2c and saa6588 are subdevs, so those two can be removed from
this list.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Janne Grunau]

Hi,

On Sat, Dec 18, 2010 at 12:31:26PM +0100, Hans Verkuil wrote:
> 
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> 
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.
> 
> I have CCed this email to the maintainers of the various drivers (if I know
> who it is) in the hope that we can get this conversion done as quickly as
> possible.
> 
> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.

...

> Driver list:

...

> au0828 (Janne Grunau)

I did only two minor cleanups and don't have the hardware. Steven Toth,
Devin Heitmueller or Michael Krufky have done major changes to the
driver.

Added to CC and trimmed

Janne

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hans Verkuil]

On Saturday, December 18, 2010 13:20:40 Janne Grunau wrote:
> Hi,
> 
> On Sat, Dec 18, 2010 at 12:31:26PM +0100, Hans Verkuil wrote:
> > 
> > Now that the BKL patch series has been merged in 2.6.37 it is time to work
> > on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> > 
> > I've made an inventory of all drivers that still use .ioctl and I am looking
> > for volunteers to tackle one or more drivers.
> > 
> > I have CCed this email to the maintainers of the various drivers (if I know
> > who it is) in the hope that we can get this conversion done as quickly as
> > possible.
> > 
> > If I have added your name to a driver, then please confirm if you are able to
> > work on it or not. If you can't work on it, but you know someone else, then
> > let me know as well.
> 
> ...
> 
> > Driver list:
> 
> ...
> 
> > au0828 (Janne Grunau)
> 
> I did only two minor cleanups and don't have the hardware. Steven Toth,
> Devin Heitmueller or Michael Krufky have done major changes to the
> driver.
> 
> Added to CC and trimmed

Hi Janne,

My apologies, for some reason I had it in my head that you maintained that
driver. But I confused au0828 with hdpvr :-(

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hans Verkuil]

On Saturday, December 18, 2010 12:31:26 Hans Verkuil wrote:
> Unassigned drivers:
> 
> saa7134
> em28xx

em28xx was a trivial fix, so this can be removed from the list.

Regards,

	Hans

> cx88
> solo6x10 (staging driver)
> 
> Regards,
> 
> 	Hans
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Andy Walls]

On Sat, 2010-12-18 at 12:31 +0100, Hans Verkuil wrote:
> Hi all,
> 
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> 
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.

> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.

> There is also a list of drivers where I do not know who can do the conversion.
> If you can tackle one or more of those, please respond. Unfortunately, those
> are among the hardest to convert :-(


> Driver list:
> 
> saa7146 (Hans Verkuil)
> mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> cx23885 (Steve Toth)
> cx18-alsa (Andy Walls)

Ack.  This thing has locking problems between ALSA and /dev/video24
anyway.

> omap24xxcam (Sakari Ailus or David Cohen)
> au0828 (Janne Grunau)
> cpia2 (Andy Walls or Hans Verkuil)

Ack.  But -ENOHARWARE; I only have a cpia1 based device on hand.

The driver is small enough:

        $ wc -l *.[ch]
          2534 cpia2_core.c
            50 cpia2dev.h
           494 cpia2.h
           476 cpia2_registers.h
           914 cpia2_usb.c
          1776 cpia2_v4l.c
          6244 total
        
and has a pretty clean coding style, so conversion shouldn't be hard.
It would be nice to have a tester.

> cx231xx (Mauro Carvalho Chehab)
> davinci (Muralidharan Karicheri)
> saa6588 (Hans Verkuil)
> pvrusb2 (Mike Isely)
> usbvision (Hans Verkuil)
> s5p-fimc (Sylwester Nawrocki)
> fsl-viu (Anatolij Gustschin)
> tlg2300 (Mauro Carvalho Chehab)
> zr364xx (Hans de Goede)
> soc_camera (Guennadi Liakhovetski)
> usbvideo/vicam (Hans de Goede)
> s2255drv (Pete Eberlein)
> bttv (Mauro Carvalho Chehab)
> stk-webcam (Hans de Goede)
> se401 (Hans de Goede)
> si4713-i2c (Hans Verkuil)
> dsbr100 (Hans Verkuil)
> 
> Staging driver list:
> 
> go7007 (Pete Eberlein)
> tm6000 (Mauro Carvalho Chehab)
> (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
> 
> Unassigned drivers:
> 
> saa7134
> em28xx
> cx88
> solo6x10 (staging driver)
> 
> Regards,
> 
> 	Hans
> 

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Steven Toth]

Good work, thanks Hans.

> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.

> cx23885 (Steve Toth)
> cx88

I'll take care of these and also the saa7164 driver.

- Steve

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Muralidharan Karicheri]

Hans,

For davinci drivers, I will be able to do this for vpfe_capture driver
(DM355/6446/365). For DM6467, will someone from your company be able
to take care of this?

Murali

On Sat, Dec 18, 2010 at 6:31 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi all,
>
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
>
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.
>
> I have CCed this email to the maintainers of the various drivers (if I know
> who it is) in the hope that we can get this conversion done as quickly as
> possible.
>
> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.
>
> There is also a list of drivers where I do not know who can do the conversion.
> If you can tackle one or more of those, please respond. Unfortunately, those
> are among the hardest to convert :-(
>
> It would be great if we can tackle most of these drivers for 2.6.38. I think
> we should finish all drivers for 2.6.39 at the latest.
>
> There are two ways of doing the conversion: one is to do all the locking within
> the driver, the other is to use core-assisted locking. How to do the core-assisted
> locking is described in Documentation/video4linux/v4l2-framework.txt, but I'll
> repeat the relevant part here:
>
> v4l2_file_operations and locking
> --------------------------------
>
> You can set a pointer to a mutex_lock in struct video_device. Usually this
> will be either a top-level mutex or a mutex per device node. If you want
> finer-grained locking then you have to set it to NULL and do you own locking.
>
> If a lock is specified then all file operations will be serialized on that
> lock. If you use videobuf then you must pass the same lock to the videobuf
> queue initialize function: if videobuf has to wait for a frame to arrive, then
> it will temporarily unlock the lock and relock it afterwards. If your driver
> also waits in the code, then you should do the same to allow other processes
> to access the device node while the first process is waiting for something.
>
> The implementation of a hotplug disconnect should also take the lock before
> calling v4l2_device_disconnect.
>
>
> Driver list:
>
> saa7146 (Hans Verkuil)
> mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> cx23885 (Steve Toth)
> cx18-alsa (Andy Walls)
> omap24xxcam (Sakari Ailus or David Cohen)
> au0828 (Janne Grunau)
> cpia2 (Andy Walls or Hans Verkuil)
> cx231xx (Mauro Carvalho Chehab)
> davinci (Muralidharan Karicheri)
> saa6588 (Hans Verkuil)
> pvrusb2 (Mike Isely)
> usbvision (Hans Verkuil)
> s5p-fimc (Sylwester Nawrocki)
> fsl-viu (Anatolij Gustschin)
> tlg2300 (Mauro Carvalho Chehab)
> zr364xx (Hans de Goede)
> soc_camera (Guennadi Liakhovetski)
> usbvideo/vicam (Hans de Goede)
> s2255drv (Pete Eberlein)
> bttv (Mauro Carvalho Chehab)
> stk-webcam (Hans de Goede)
> se401 (Hans de Goede)
> si4713-i2c (Hans Verkuil)
> dsbr100 (Hans Verkuil)
>
> Staging driver list:
>
> go7007 (Pete Eberlein)
> tm6000 (Mauro Carvalho Chehab)
> (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
>
> Unassigned drivers:
>
> saa7134
> em28xx
> cx88
> solo6x10 (staging driver)
>
> Regards,
>
>        Hans
>
> --
> Hans Verkuil - video4linux developer - sponsored by Cisco
> --
> 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
>



-- 
Murali Karicheri
mkaricheri@gmail.com

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Sylwester Nawrocki]

Hi Hans,

On 12/18/2010 12:31 PM, Hans Verkuil wrote:
> Hi all,
>
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
>
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.
>
> I have CCed this email to the maintainers of the various drivers (if I know
> who it is) in the hope that we can get this conversion done as quickly as
> possible.
>
> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.
>
> There is also a list of drivers where I do not know who can do the conversion.
> If you can tackle one or more of those, please respond. Unfortunately, those
> are among the hardest to convert :-(
>
> It would be great if we can tackle most of these drivers for 2.6.38. I think
> we should finish all drivers for 2.6.39 at the latest.
>
> There are two ways of doing the conversion: one is to do all the locking within
> the driver, the other is to use core-assisted locking. How to do the core-assisted
> locking is described in Documentation/video4linux/v4l2-framework.txt, but I'll
> repeat the relevant part here:
>
> v4l2_file_operations and locking
> --------------------------------
>
> You can set a pointer to a mutex_lock in struct video_device. Usually this
> will be either a top-level mutex or a mutex per device node. If you want
> finer-grained locking then you have to set it to NULL and do you own locking.
>
> If a lock is specified then all file operations will be serialized on that
> lock. If you use videobuf then you must pass the same lock to the videobuf
> queue initialize function: if videobuf has to wait for a frame to arrive, then
> it will temporarily unlock the lock and relock it afterwards. If your driver
> also waits in the code, then you should do the same to allow other processes
> to access the device node while the first process is waiting for something.
>
> The implementation of a hotplug disconnect should also take the lock before
> calling v4l2_device_disconnect.
>
>
> Driver list:
>
> saa7146 (Hans Verkuil)
> mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> cx23885 (Steve Toth)
> cx18-alsa (Andy Walls)
> omap24xxcam (Sakari Ailus or David Cohen)
> au0828 (Janne Grunau)
> cpia2 (Andy Walls or Hans Verkuil)
> cx231xx (Mauro Carvalho Chehab)
> davinci (Muralidharan Karicheri)
> saa6588 (Hans Verkuil)
> pvrusb2 (Mike Isely)
> usbvision (Hans Verkuil)
> s5p-fimc (Sylwester Nawrocki)

I've done a conversion of s5p-fimc already. The relevant patch
is included in my last pull request of the driver bugfix changeset
for 2.6.37.

I am not sure what are Mauro's plans about this changeset, I hope it is 
not too late to have it in 2.6.37. If it is, then it would be good to 
pick up at least the commit
7db545a [media] s5p-fimc: BKL lock removal - compilation fix
because without it the driver is not even compiling. The above commit
fixes some integration problem, i.e. I submitted the camera capture
support patches and in parallel there were changes in the v4l core.

Then I have also prepared further patch converting from driver's own
to the core assisted locking, but it's on my queue for 2.6.38.
That was submitted together with the videobuf 2 patches

http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/vb2-mfc-fimc

> fsl-viu (Anatolij Gustschin)
> tlg2300 (Mauro Carvalho Chehab)
> zr364xx (Hans de Goede)
> soc_camera (Guennadi Liakhovetski)
> usbvideo/vicam (Hans de Goede)
> s2255drv (Pete Eberlein)
> bttv (Mauro Carvalho Chehab)
> stk-webcam (Hans de Goede)
> se401 (Hans de Goede)
> si4713-i2c (Hans Verkuil)
> dsbr100 (Hans Verkuil)
>
> Staging driver list:
>
> go7007 (Pete Eberlein)
> tm6000 (Mauro Carvalho Chehab)
> (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
>
> Unassigned drivers:
>
> saa7134

I could try converting saa7134 in my spare time, but it is rather
a huge driver and I have no experience with it.
If there is really nobody more familiar to take care of it I would
try it.

I have avermedia E506 TV card on hand, would it be enough for basic testing?

snawrocki@thinkpad:~/linux/v4l-dvb$ sudo lspci -vv
16:00.0 Multimedia controller: Philips Semiconductors 
SAA7131/SAA7133/SAA7135 Video Broadcast Decoder (rev d1)
	Subsystem: Avermedia Technologies Inc Device f436
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 64
	Interrupt: pin A routed to IRQ 16
	Region 0: Memory at 44000000 (32-bit, non-prefetchable) [size=2K]
	Capabilities: [40] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Kernel driver in use: saa7134
	Kernel modules: saa7134

> em28xx
> cx88
> solo6x10 (staging driver)
>
> Regards,
>
> 	Hans
>

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Mike Isely]

I'll take care of the pvrusb2 driver.  How soon does this need to be 
completed?

  -Mike


On Sat, 18 Dec 2010, Hans Verkuil wrote:

> On Saturday, December 18, 2010 12:31:26 Hans Verkuil wrote:
> > Driver list:
> > 
> > saa7146 (Hans Verkuil)
> > mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> > cx23885 (Steve Toth)
> > cx18-alsa (Andy Walls)
> > omap24xxcam (Sakari Ailus or David Cohen)
> > au0828 (Janne Grunau)
> > cpia2 (Andy Walls or Hans Verkuil)
> > cx231xx (Mauro Carvalho Chehab)
> > davinci (Muralidharan Karicheri)
> > saa6588 (Hans Verkuil)
> > pvrusb2 (Mike Isely)
> > usbvision (Hans Verkuil)
> > s5p-fimc (Sylwester Nawrocki)
> > fsl-viu (Anatolij Gustschin)
> > tlg2300 (Mauro Carvalho Chehab)
> > zr364xx (Hans de Goede)
> > soc_camera (Guennadi Liakhovetski)
> > usbvideo/vicam (Hans de Goede)
> > s2255drv (Pete Eberlein)
> > bttv (Mauro Carvalho Chehab)
> > stk-webcam (Hans de Goede)
> > se401 (Hans de Goede)
> > si4713-i2c (Hans Verkuil)
> > dsbr100 (Hans Verkuil)
> 
> Oops, si4713-i2c and saa6588 are subdevs, so those two can be removed from
> this list.
> 
> Regards,
> 
> 	Hans
> 
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hans Verkuil]

On Saturday, December 18, 2010 19:34:00 Mike Isely wrote:
> 
> I'll take care of the pvrusb2 driver.  How soon does this need to be 
> completed?

It would be great if we can finish this in time for 2.6.38. So that's in the
next three weeks.

I have to say that switching drivers to use the core-assisted lock tends to be
pretty easy. So I am hopeful that it is less work than it looks at first sight.

Regards,

	hans

> 
>   -Mike
> 
> 
> On Sat, 18 Dec 2010, Hans Verkuil wrote:
> 
> > On Saturday, December 18, 2010 12:31:26 Hans Verkuil wrote:
> > > Driver list:
> > > 
> > > saa7146 (Hans Verkuil)
> > > mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> > > cx23885 (Steve Toth)
> > > cx18-alsa (Andy Walls)
> > > omap24xxcam (Sakari Ailus or David Cohen)
> > > au0828 (Janne Grunau)
> > > cpia2 (Andy Walls or Hans Verkuil)
> > > cx231xx (Mauro Carvalho Chehab)
> > > davinci (Muralidharan Karicheri)
> > > saa6588 (Hans Verkuil)
> > > pvrusb2 (Mike Isely)
> > > usbvision (Hans Verkuil)
> > > s5p-fimc (Sylwester Nawrocki)
> > > fsl-viu (Anatolij Gustschin)
> > > tlg2300 (Mauro Carvalho Chehab)
> > > zr364xx (Hans de Goede)
> > > soc_camera (Guennadi Liakhovetski)
> > > usbvideo/vicam (Hans de Goede)
> > > s2255drv (Pete Eberlein)
> > > bttv (Mauro Carvalho Chehab)
> > > stk-webcam (Hans de Goede)
> > > se401 (Hans de Goede)
> > > si4713-i2c (Hans Verkuil)
> > > dsbr100 (Hans Verkuil)
> > 
> > Oops, si4713-i2c and saa6588 are subdevs, so those two can be removed from
> > this list.
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > 
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Guennadi Liakhovetski]

On Sat, 18 Dec 2010, Hans Verkuil wrote:

> Hi all,
> 
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> 
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.
> 
> I have CCed this email to the maintainers of the various drivers (if I know
> who it is) in the hope that we can get this conversion done as quickly as
> possible.
> 
> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.
> 
> There is also a list of drivers where I do not know who can do the conversion.
> If you can tackle one or more of those, please respond. Unfortunately, those
> are among the hardest to convert :-(
> 
> It would be great if we can tackle most of these drivers for 2.6.38. I think
> we should finish all drivers for 2.6.39 at the latest.
> 
> There are two ways of doing the conversion: one is to do all the locking within
> the driver, the other is to use core-assisted locking. How to do the core-assisted
> locking is described in Documentation/video4linux/v4l2-framework.txt, but I'll
> repeat the relevant part here:
> 
> v4l2_file_operations and locking
> --------------------------------
> 
> You can set a pointer to a mutex_lock in struct video_device. Usually this
> will be either a top-level mutex or a mutex per device node. If you want
> finer-grained locking then you have to set it to NULL and do you own locking.
> 
> If a lock is specified then all file operations will be serialized on that
> lock. If you use videobuf then you must pass the same lock to the videobuf
> queue initialize function: if videobuf has to wait for a frame to arrive, then
> it will temporarily unlock the lock and relock it afterwards. If your driver
> also waits in the code, then you should do the same to allow other processes
> to access the device node while the first process is waiting for something.
> 
> The implementation of a hotplug disconnect should also take the lock before
> calling v4l2_device_disconnect.
> 
> 
> Driver list:
> 
> saa7146 (Hans Verkuil)
> mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> cx23885 (Steve Toth)
> cx18-alsa (Andy Walls)
> omap24xxcam (Sakari Ailus or David Cohen)
> au0828 (Janne Grunau)
> cpia2 (Andy Walls or Hans Verkuil)
> cx231xx (Mauro Carvalho Chehab)
> davinci (Muralidharan Karicheri)
> saa6588 (Hans Verkuil)
> pvrusb2 (Mike Isely)
> usbvision (Hans Verkuil)
> s5p-fimc (Sylwester Nawrocki)
> fsl-viu (Anatolij Gustschin)
> tlg2300 (Mauro Carvalho Chehab)
> zr364xx (Hans de Goede)
> soc_camera (Guennadi Liakhovetski)

Will have a look.

Thanks
Guennadi

> usbvideo/vicam (Hans de Goede)
> s2255drv (Pete Eberlein)
> bttv (Mauro Carvalho Chehab)
> stk-webcam (Hans de Goede)
> se401 (Hans de Goede)
> si4713-i2c (Hans Verkuil)
> dsbr100 (Hans Verkuil)
> 
> Staging driver list:
> 
> go7007 (Pete Eberlein)
> tm6000 (Mauro Carvalho Chehab)
> (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
> 
> Unassigned drivers:
> 
> saa7134
> em28xx
> cx88
> solo6x10 (staging driver)
> 
> Regards,
> 
> 	Hans
> 
> -- 
> Hans Verkuil - video4linux developer - sponsored by Cisco
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hans Verkuil]

On Saturday, December 18, 2010 16:38:29 Muralidharan Karicheri wrote:
> Hans,
> 
> For davinci drivers, I will be able to do this for vpfe_capture driver
> (DM355/6446/365).

That one already uses unlocked_ioctl. You might want to consider auditing it
to see if converting it to core-assisted locking is better. It is much easier
to verify lock-correctness that way.

> For DM6467, will someone from your company be able
> to take care of this?

I'll see if I can do it and ask a colleague to test for me next week. Otherwise
I can do it in the first week of January.

Regards,

	Hans

> 
> Murali
> 
> On Sat, Dec 18, 2010 at 6:31 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > Hi all,
> >
> > Now that the BKL patch series has been merged in 2.6.37 it is time to work
> > on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> >
> > I've made an inventory of all drivers that still use .ioctl and I am looking
> > for volunteers to tackle one or more drivers.
> >
> > I have CCed this email to the maintainers of the various drivers (if I know
> > who it is) in the hope that we can get this conversion done as quickly as
> > possible.
> >
> > If I have added your name to a driver, then please confirm if you are able to
> > work on it or not. If you can't work on it, but you know someone else, then
> > let me know as well.
> >
> > There is also a list of drivers where I do not know who can do the conversion.
> > If you can tackle one or more of those, please respond. Unfortunately, those
> > are among the hardest to convert :-(
> >
> > It would be great if we can tackle most of these drivers for 2.6.38. I think
> > we should finish all drivers for 2.6.39 at the latest.
> >
> > There are two ways of doing the conversion: one is to do all the locking within
> > the driver, the other is to use core-assisted locking. How to do the core-assisted
> > locking is described in Documentation/video4linux/v4l2-framework.txt, but I'll
> > repeat the relevant part here:
> >
> > v4l2_file_operations and locking
> > --------------------------------
> >
> > You can set a pointer to a mutex_lock in struct video_device. Usually this
> > will be either a top-level mutex or a mutex per device node. If you want
> > finer-grained locking then you have to set it to NULL and do you own locking.
> >
> > If a lock is specified then all file operations will be serialized on that
> > lock. If you use videobuf then you must pass the same lock to the videobuf
> > queue initialize function: if videobuf has to wait for a frame to arrive, then
> > it will temporarily unlock the lock and relock it afterwards. If your driver
> > also waits in the code, then you should do the same to allow other processes
> > to access the device node while the first process is waiting for something.
> >
> > The implementation of a hotplug disconnect should also take the lock before
> > calling v4l2_device_disconnect.
> >
> >
> > Driver list:
> >
> > saa7146 (Hans Verkuil)
> > mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> > cx23885 (Steve Toth)
> > cx18-alsa (Andy Walls)
> > omap24xxcam (Sakari Ailus or David Cohen)
> > au0828 (Janne Grunau)
> > cpia2 (Andy Walls or Hans Verkuil)
> > cx231xx (Mauro Carvalho Chehab)
> > davinci (Muralidharan Karicheri)
> > saa6588 (Hans Verkuil)
> > pvrusb2 (Mike Isely)
> > usbvision (Hans Verkuil)
> > s5p-fimc (Sylwester Nawrocki)
> > fsl-viu (Anatolij Gustschin)
> > tlg2300 (Mauro Carvalho Chehab)
> > zr364xx (Hans de Goede)
> > soc_camera (Guennadi Liakhovetski)
> > usbvideo/vicam (Hans de Goede)
> > s2255drv (Pete Eberlein)
> > bttv (Mauro Carvalho Chehab)
> > stk-webcam (Hans de Goede)
> > se401 (Hans de Goede)
> > si4713-i2c (Hans Verkuil)
> > dsbr100 (Hans Verkuil)
> >
> > Staging driver list:
> >
> > go7007 (Pete Eberlein)
> > tm6000 (Mauro Carvalho Chehab)
> > (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
> >
> > Unassigned drivers:
> >
> > saa7134
> > em28xx
> > cx88
> > solo6x10 (staging driver)
> >
> > Regards,
> >
> >        Hans
> >
> > --
> > Hans Verkuil - video4linux developer - sponsored by Cisco
> > --
> > 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
> >
> 
> 
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

RE: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Hadli, Manjunath]

Hans,
  I can take a look at this (DM6467)conversion unless your colleague has already started working on t.

-Manju

On Sun, Dec 19, 2010 at 02:48:05, Hans Verkuil wrote:
> On Saturday, December 18, 2010 16:38:29 Muralidharan Karicheri wrote:
> > Hans,
> > 
> > For davinci drivers, I will be able to do this for vpfe_capture driver 
> > (DM355/6446/365).
> 
> That one already uses unlocked_ioctl. You might want to consider auditing it to see if converting it to core-assisted locking is better. It is much easier to verify lock-correctness that way.
> 
> > For DM6467, will someone from your company be able to take care of 
> > this?
> 
> I'll see if I can do it and ask a colleague to test for me next week. Otherwise I can do it in the first week of January.
> 
> Regards,
> 
> 	Hans
> 
> > 
> > Murali
> > 
> > On Sat, Dec 18, 2010 at 6:31 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > > Hi all,
> > >
> > > Now that the BKL patch series has been merged in 2.6.37 it is time 
> > > to work on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> > >
> > > I've made an inventory of all drivers that still use .ioctl and I am 
> > > looking for volunteers to tackle one or more drivers.
> > >
> > > I have CCed this email to the maintainers of the various drivers (if 
> > > I know who it is) in the hope that we can get this conversion done 
> > > as quickly as possible.
> > >
> > > If I have added your name to a driver, then please confirm if you 
> > > are able to work on it or not. If you can't work on it, but you know 
> > > someone else, then let me know as well.
> > >
> > > There is also a list of drivers where I do not know who can do the conversion.
> > > If you can tackle one or more of those, please respond. 
> > > Unfortunately, those are among the hardest to convert :-(
> > >
> > > It would be great if we can tackle most of these drivers for 2.6.38. 
> > > I think we should finish all drivers for 2.6.39 at the latest.
> > >
> > > There are two ways of doing the conversion: one is to do all the 
> > > locking within the driver, the other is to use core-assisted 
> > > locking. How to do the core-assisted locking is described in 
> > > Documentation/video4linux/v4l2-framework.txt, but I'll repeat the relevant part here:
> > >
> > > v4l2_file_operations and locking
> > > --------------------------------
> > >
> > > You can set a pointer to a mutex_lock in struct video_device. 
> > > Usually this will be either a top-level mutex or a mutex per device 
> > > node. If you want finer-grained locking then you have to set it to NULL and do you own locking.
> > >
> > > If a lock is specified then all file operations will be serialized 
> > > on that lock. If you use videobuf then you must pass the same lock 
> > > to the videobuf queue initialize function: if videobuf has to wait 
> > > for a frame to arrive, then it will temporarily unlock the lock and 
> > > relock it afterwards. If your driver also waits in the code, then 
> > > you should do the same to allow other processes to access the device node while the first process is waiting for something.
> > >
> > > The implementation of a hotplug disconnect should also take the lock 
> > > before calling v4l2_device_disconnect.
> > >
> > >
> > > Driver list:
> > >
> > > saa7146 (Hans Verkuil)
> > > mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> > > cx23885 (Steve Toth)
> > > cx18-alsa (Andy Walls)
> > > omap24xxcam (Sakari Ailus or David Cohen)
> > > au0828 (Janne Grunau)
> > > cpia2 (Andy Walls or Hans Verkuil)
> > > cx231xx (Mauro Carvalho Chehab)
> > > davinci (Muralidharan Karicheri)
> > > saa6588 (Hans Verkuil)
> > > pvrusb2 (Mike Isely)
> > > usbvision (Hans Verkuil)
> > > s5p-fimc (Sylwester Nawrocki)
> > > fsl-viu (Anatolij Gustschin)
> > > tlg2300 (Mauro Carvalho Chehab)
> > > zr364xx (Hans de Goede)
> > > soc_camera (Guennadi Liakhovetski)
> > > usbvideo/vicam (Hans de Goede)
> > > s2255drv (Pete Eberlein)
> > > bttv (Mauro Carvalho Chehab)
> > > stk-webcam (Hans de Goede)
> > > se401 (Hans de Goede)
> > > si4713-i2c (Hans Verkuil)
> > > dsbr100 (Hans Verkuil)
> > >
> > > Staging driver list:
> > >
> > > go7007 (Pete Eberlein)
> > > tm6000 (Mauro Carvalho Chehab)
> > > (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
> > >
> > > Unassigned drivers:
> > >
> > > saa7134
> > > em28xx
> > > cx88
> > > solo6x10 (staging driver)
> > >
> > > Regards,
> > >
> > >        Hans
> > >
> > > --
> > > Hans Verkuil - video4linux developer - sponsored by Cisco
> > > --
> > > 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
> > >
> > 
> > 
> > 
> > 
> 
> --
> Hans Verkuil - video4linux developer - sponsored by Cisco
> --
> 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
> 

[PATCH] v4l: mem2mem_testdev: remove BKL usage

[Marek Szyprowski]

Remove usage of BKL by usign per-device mutex.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
CC: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/video/mem2mem_testdev.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c
index 3b19f5b..c179041 100644
--- a/drivers/media/video/mem2mem_testdev.c
+++ b/drivers/media/video/mem2mem_testdev.c
@@ -524,7 +524,6 @@ static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
 {
 	struct m2mtest_q_data *q_data;
 	struct videobuf_queue *vq;
-	int ret = 0;
 
 	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type);
 	if (!vq)
@@ -534,12 +533,9 @@ static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
 	if (!q_data)
 		return -EINVAL;
 
-	mutex_lock(&vq->vb_lock);
-
 	if (videobuf_queue_is_busy(vq)) {
 		v4l2_err(&ctx->dev->v4l2_dev, "%s queue busy\n", __func__);
-		ret = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
 	q_data->fmt		= find_format(f);
@@ -553,9 +549,7 @@ static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
 		"Setting format for type %d, wxh: %dx%d, fmt: %d\n",
 		f->type, q_data->width, q_data->height, q_data->fmt->fourcc);
 
-out:
-	mutex_unlock(&vq->vb_lock);
-	return ret;
+	return 0;
 }
 
 static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
@@ -845,10 +839,12 @@ static void queue_init(void *priv, struct videobuf_queue *vq,
 		       enum v4l2_buf_type type)
 {
 	struct m2mtest_ctx *ctx = priv;
+	struct m2mtest_dev *dev = ctx->dev;
 
-	videobuf_queue_vmalloc_init(vq, &m2mtest_qops, ctx->dev->v4l2_dev.dev,
-				    &ctx->dev->irqlock, type, V4L2_FIELD_NONE,
-				    sizeof(struct m2mtest_buffer), priv, NULL);
+	videobuf_queue_vmalloc_init(vq, &m2mtest_qops, dev->v4l2_dev.dev,
+				    &dev->irqlock, type, V4L2_FIELD_NONE,
+				    sizeof(struct m2mtest_buffer), priv,
+				    &dev->dev_mutex);
 }
 
 
@@ -920,7 +916,7 @@ static const struct v4l2_file_operations m2mtest_fops = {
 	.open		= m2mtest_open,
 	.release	= m2mtest_release,
 	.poll		= m2mtest_poll,
-	.ioctl		= video_ioctl2,
+	.unlocked_ioctl	= video_ioctl2,
 	.mmap		= m2mtest_mmap,
 };
 
@@ -965,6 +961,7 @@ static int m2mtest_probe(struct platform_device *pdev)
 	}
 
 	*vfd = m2mtest_videodev;
+	vfd->lock = &dev->dev_mutex;
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
 	if (ret) {
-- 
1.7.1.569.g6f426

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[Pete Eberlein]

On Sat, 2010-12-18 at 12:31 +0100, Hans Verkuil wrote:
> Hi all,
> 
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
> 
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.
> 
> I have CCed this email to the maintainers of the various drivers (if I know
> who it is) in the hope that we can get this conversion done as quickly as
> possible.
> 
> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.

> s2255drv (Pete Eberlein)

I'll work on this one.

> Staging driver list:
> 
> go7007 (Pete Eberlein)

And this one.

Regards,
Pete Eberlein

Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

[David Ellingsworth]

On Sat, Dec 18, 2010 at 6:31 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi all,
>
> Now that the BKL patch series has been merged in 2.6.37 it is time to work
> on replacing .ioctl by .unlocked_ioctl in all v4l drivers.
>
> I've made an inventory of all drivers that still use .ioctl and I am looking
> for volunteers to tackle one or more drivers.
>
> I have CCed this email to the maintainers of the various drivers (if I know
> who it is) in the hope that we can get this conversion done as quickly as
> possible.
>
> If I have added your name to a driver, then please confirm if you are able to
> work on it or not. If you can't work on it, but you know someone else, then
> let me know as well.
>
> There is also a list of drivers where I do not know who can do the conversion.
> If you can tackle one or more of those, please respond. Unfortunately, those
> are among the hardest to convert :-(
>
> It would be great if we can tackle most of these drivers for 2.6.38. I think
> we should finish all drivers for 2.6.39 at the latest.
>
> There are two ways of doing the conversion: one is to do all the locking within
> the driver, the other is to use core-assisted locking. How to do the core-assisted
> locking is described in Documentation/video4linux/v4l2-framework.txt, but I'll
> repeat the relevant part here:
>
> v4l2_file_operations and locking
> --------------------------------
>
> You can set a pointer to a mutex_lock in struct video_device. Usually this
> will be either a top-level mutex or a mutex per device node. If you want
> finer-grained locking then you have to set it to NULL and do you own locking.
>
> If a lock is specified then all file operations will be serialized on that
> lock. If you use videobuf then you must pass the same lock to the videobuf
> queue initialize function: if videobuf has to wait for a frame to arrive, then
> it will temporarily unlock the lock and relock it afterwards. If your driver
> also waits in the code, then you should do the same to allow other processes
> to access the device node while the first process is waiting for something.
>
> The implementation of a hotplug disconnect should also take the lock before
> calling v4l2_device_disconnect.
>
>
> Driver list:
>
> saa7146 (Hans Verkuil)
> mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
> cx23885 (Steve Toth)
> cx18-alsa (Andy Walls)
> omap24xxcam (Sakari Ailus or David Cohen)
> au0828 (Janne Grunau)
> cpia2 (Andy Walls or Hans Verkuil)
> cx231xx (Mauro Carvalho Chehab)
> davinci (Muralidharan Karicheri)
> saa6588 (Hans Verkuil)
> pvrusb2 (Mike Isely)
> usbvision (Hans Verkuil)
> s5p-fimc (Sylwester Nawrocki)
> fsl-viu (Anatolij Gustschin)
> tlg2300 (Mauro Carvalho Chehab)
> zr364xx (Hans de Goede)
> soc_camera (Guennadi Liakhovetski)
> usbvideo/vicam (Hans de Goede)
> s2255drv (Pete Eberlein)
> bttv (Mauro Carvalho Chehab)
> stk-webcam (Hans de Goede)
> se401 (Hans de Goede)
> si4713-i2c (Hans Verkuil)
> dsbr100 (Hans Verkuil)

There are patches for dsbr100 here:
http://desource.dyndns.org/~atog/gitweb/?p=linux-media.git
These patches don't use the new core-assisted locking as they were
written back in MAY, but they DO remove the BKL from the driver.
The patches have been compile tested only, as I do not have access to
a device for this driver.

>
> Staging driver list:
>
> go7007 (Pete Eberlein)
> tm6000 (Mauro Carvalho Chehab)
> (stradis/cpia: will be removed in 2.6.38, so no need to do anything)
>
> Unassigned drivers:
>
> saa7134
> em28xx
> cx88
> solo6x10 (staging driver)
>
> Regards,
>
>        Hans
>

Regards,

David Ellingsworth