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

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

Hi Sakari,

Just returned this week from vacations. I'm reading my long e-mail backlog,
starting from my main inbox...

Em Mon, 2 Jan 2017 09:53:49 +0200
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Mauro,
> 
> On Mon, Dec 19, 2016 at 07:46:55AM -0200, Mauro Carvalho Chehab wrote:
> > 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.  
> 
> It's not a driver problem so yes, it is reproducible on other hardware.

Didn't have time to test it before entering into vacations.

I guess I won't have any time this week to test those issues on
my hardware, as I suspect that my patch queue is full. Also, we're
approaching the next merge window. So, unfortunately, I won't have
much time those days to do much testing. 

Btw, Hans commented that you were planning to working on it this month.

Do you have some news with regards to the media controller bind/unbind
fixes?

> > 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.  
> 
> This is not really about the OMAP3 ISP driver hotplug support; it is indeed
> about the framework's ability to support hotpluggable hardware. The current
> painpoint is removing hardware; the current frameworks aren't quite up to
> that at the moment.

The point here is that, while it would be fun to allow unbinding OMAP3
V4L2 drivers, OMAP3 doesn't really require hotplug support. On the other
hand, on USB drivers, where unbind is a requirement, the current status
of the tree is that hotplug works. I did some massive parallel bind/unbind
loops here to double check, when we added such fixup patches. Granted, I
won't doubt that there are still some rare race conditions that I was
unable to reproduce on the time I tested. I also didn't try to hack the
Kernel to introduce extra delays to make those race conditions more
likely to happen.

Anyway, my main concern with this patch is that it breaks hotplug on devices
that really need it, while it fix support only for OMAP3 (with doesn't need).

Also, it starts with a series of patches that will cause regressions.

I won't matter changing the solution to some other approach that would
work, provided that the patches are added on an incremented way, and
won't introduce regressions to USB drivers.

> > 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?  
> 
> That's a good point. I wasn't reviewing that driver back then when the
> patches were posted, but should it go in, it should make a good example for
> writing other drivers as well.
> 

I saw Laurent's comments about Helen's last patch series. From his
comments:

	"I've reviewed the whole patch but haven't had time to test it. I've also 
	 skipped the items marked as TODO or FIXME as they're obviously not ready yet 
	 :-) Overall this looks good to me, all the issues are minor."

Helen promised a new version fixing those minor issues. Perhaps we should merge
her next series upstream with such issues addressed and see how it behaves.

Thanks,
Mauro