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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mauro,

On Tue, Jan 24, 2017 at 08:49:02AM -0200, Mauro Carvalho Chehab wrote:
> 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@xxxxxx> 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@xxxxxx> 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@xxxxxxxxxxxxxxx/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?

I have a bunch of meeting notes to send from the Oslo meeting with Hans and
Laurent; I should have that ready by the end of the week. The RFC patchset
certainly needs changes based on that.

> 
> > > 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).

I don't disagree with you. Obviously the intent is not to break
hot-pluggable hardware, albeit the changes needed to avoid that haven't been
implemented yet. (One of the reasons it's been RFC all the time.)

> 
> 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.

It may be possible to avoid increasing the time window during which bad
things could happen before fully removing them. However the patchset is a
lot easier to work with without bundling the reverts into other (and likely
multiple) patches as the reverted patches took quite a different direction
than is followed in this patchset.

Let's discuss this later, at the time when we have a patchset that produces
a sound code base (on the top of that patchset) that is understood to be
free of object lifetime issues as long as hot-pluggable hardware goes.

> 
> > > 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.

I'll review Helen's set next.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx	XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux