Hi Laurent, Hans, On Tue, Mar 05, 2019 at 09:52:14PM +0200, Laurent Pinchart wrote: > Hi Hans, > > (CC'ing Sakari) > > Thank you for the patch. > > On Tue, Mar 05, 2019 at 10:58:45AM +0100, hverkuil-cisco@xxxxxxxxx wrote: > > From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > > > > The module ownership refcounting was done in media_entity_get/put, > > but that was very confusing and it did not work either in case an > > application had a v4l-subdevX device open and the module was > > unbound. When the v4l-subdevX device was closed the media_entity_put > > was never called and the module refcount was left one too high, making > > it impossible to unload it. Hmm. This is not really a proper fix for the problem although it lets you unload the module in the case things got horribly wrong. Instead the media device may not be allowed to disappear while it is being accessed elsewhere. I think that the merit of this patch is cleaning the code, not really fixing a bug. But keeping the owner you can't access later because you've released the memory isn't neat either. How about adding a comment next to the owner field, e.g. "TODO: address refcounting properly"? > > > > Since v4l2-subdev.c was the only place where media_entity_get/put was > > called, just move the functionality to v4l2-subdev.c and drop those > > confusing entity functions. > > I wonder if we will later need to refcount media entities, but we can > reintroduce a different version of those two functions then, it doesn't > prevent their removal now. I agree. > > Sakari, when working on lifetime management of objects in the media and > V4L2 core, did you come across a need to refcount entities ? We will need to do that to allow removing entities safely. The current patchset only deals with the media device and it's stille pending on DVB framework issues. -- Regards, Sakari Ailus
