On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Tue, Aug 2, 2022 at 11:06 AM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > > On 8/2/22 10:42, Andy Shevchenko wrote: > > > On Mon, Aug 1, 2022 at 8:35 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > > >> On 01/08/2022 16:57, Andy Shevchenko wrote: > > >>> On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr) > > >>> <hljunggr@xxxxxxxxx> wrote: > > >>>> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote: > > >>>>> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@xxxxxxxxx> > > >>>>> wrote: > > ... > > > >>>>>> + state = kzalloc(sizeof(*state), GFP_KERNEL); > > >>>>>> + if (!state) > > >>>>>> + return -ENOMEM; > > >>>>> > > >>>>> devm_kzalloc() ? > > >>>> > > >>>> This will fail if the device is forcibly unloaded while some > > >>>> application has the device node open. > > >>> > > >>> I'm not sure how it's related. Can you elaborate a bit, please? > > >>> > > >>> If you try to forcibly unload the device (driver) when it's open and > > >>> it somehow succeeds, that will be a sign of lifetime issues in the > > >>> code. > > >> > > >> Not with rmmod but using the unbind facility. > > > > > > And what is the difference? The device driver core calls the same, no? > > > > rmmod when the /dev/videoX is open won't work (device is busy), whereas > > unbind *will* work, and it is really the same as a USB unplug. > > Seems there are no guards against that. > > > >> For new media drivers we generally > > >> want to avoid using devm_*alloc, it causes more problems than it solves. > > > > > > I think it's because people don't think much about the lifetime of > > > objects. I don't think devm is an issue here. > > > > Yes, it is: unbind will call the driver remove() function, and after that > > function all memory allocated with devm_*alloc will be freed immediately. > > Yes. > > > But if an application still has a filehandle open and was possibly even in > > the middle of an ioctl call, then having the memory removed instantaneously > > is a really bad thing. > > True. > > > Hotpluggable devices in general definitely should not use it. I'm not a fan > > of devm_*alloc anymore. > > 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? -- With Best Regards, Andy Shevchenko
