On 8/2/22 14:49, Andy Shevchenko wrote: > On Tue, Aug 2, 2022 at 2:45 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: >> On 8/2/22 14:26, Andy Shevchenko wrote: >>> On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko >>> <andy.shevchenko@xxxxxxxxx> wrote: >>>> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko >>>> <andy.shevchenko@xxxxxxxxx> wrote: > > ... > >>>>> You are blaming the wrong man here, i.e. devm. The problem as I stated >>>>> above is developers who do not understand (pay attention to) the >>>>> lifetime of the objects. >>>> >>>> That said, the devm has nothing to do with the driver still being >>>> problematic for the scenario you described, no? >>> >>> And the cleanest (at the first glance) solution is to make v4l2 to fix >>> this bug by suppressing unbind attributes when the device is opened >>> for all v4l2 subdev drivers, and restore it back when it's closed. >> >> Why would we do that? The patch works in the scenario that I described: >> the memory is freed in the struct video_device release() callback, which >> is called when the last reference to the video node goes away. This is >> standard V4L2 framework behavior that works great in the case of a unbind. >> >> Without devm_kzalloc it works fine, even when unbind is called. With >> devm_kzalloc the unbind attributes would have to be suppressed. I see no >> reason for that as media maintainer. > > I'm not sure anymore that we are talking about the same thing. > > Your driver allocates memory with kzalloc() in ->probe() and frees it > in ->remove(). How is this different from the lifetime of a devm:ed > object? If what you said is true, than driver is still problematic, > since ->remove() frees this memory the very same way at unbind call. No, it is not freed in remove(). remove() calls video_unregister_device(), and that calls cat24c208_release() when the last user of /dev/videoX closes its filehandle. And it is cat24c208_release() that finally frees the memory. Regards, Hans
