Re: [RFC v3 00/21] Make use of kref in media device, grab references as needed

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Fri, 16 Dec 2016 17:07:23 +0200
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Hans,

> > chrdev_open in fs/char_dev.c increases the refcount on open() and decreases it
> > on release(). Thus ensuring that the cdev can never be removed while in an
> > ioctl.  
> 
> It does, but it does not affect memory which is allocated separately of that.
> 
> See this:
> 
> <URL:https://www.mail-archive.com/linux-media@vger.kernel.org/msg106390.html>

That sounds promising. If this bug issues other drivers than OMAP3,
then indeed the core has a bug.

I'll see if I can reproduce it here with some USB drivers later this week.

> > If the omap3 is used as a testbed, then that's fine by me, but even then I
> > probably wouldn't want the omap3 code that makes this possible in the kernel.
> > It's just additional code for no purpose.  
> 
> The same problems exist on other devices, whether platform, pci or USB, as
> the problems are in the core frameworks rather than (only) in the drivers.
> 
> On platform devices, this happens also when removing the module.
> 
> I've used omap3isp as an example since it demonstrates well the problems and
> a lot of people have the hardware as well. Also, Mauro has requested all
> drivers to be converted to the new API. I'm fine doing that for the actually
> hot-pluggable hardware.

While IMHO it is overkill trying to support hot plug on omap3, I won't
mind if you do that, provided that your patch series can be applied in
a way that it won't cause regressions for real hot-pluggable hardware.

I still think that keeping cdev embedded in a structure that it is
created dynamically when registering the device node, instead of
embedding it at struct media_device. Yet, if you prove that this does
more harm than good, I'm ok on re-embeeding it. However, on such case,
you need to put the patches re-embeeding it at the end of the patch
series (and not at the beginning), as otherwise you'll be causing
regressions.

> One additional reason is that as the omap3isp driver has been used as an
> example to write a number of other drivers, people do see what's the right
> way to do these things, instead of copying code from a driver doing it
> wrong.

Interesting argument. Yet, IMHO, the best would be to do the proper
review on the first platform driver that would support hot-plug,
and use this as an example. It is a shame that project Aurora was
discontinued, as media drivers for such kind of hardware would be an
interesting example.

On that matter, just like we use vivid as a testbench and as an
example for other drivers, it would be great if we could merge
the vimc driver. What's the status of Helen's patchset?

Thanks,
Mauro