Re: [PATCH 1/4] media: nuvoton: Fix reference handling of ece_pdev

[Greg KH <gregkh@linuxfoundation.org>]

On Sun, Feb 23, 2025 at 07:34:30PM +0100, Ricardo Ribalda wrote:
> On Fri, 21 Feb 2025 at 10:18, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >
> > On 21/02/2025 10:04, Hans Verkuil wrote:
> > > Hi Ricardo,
> > >
> > > On 21/01/2025 22:14, Ricardo Ribalda wrote:
> > >> When we obtain a reference to of a platform_device, we need to release
> > >> it via put_device.
> > >>
> > >> Found by cocci:
> > >> ./platform/nuvoton/npcm-video.c:1677:3-9: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function.
> > >> ./platform/nuvoton/npcm-video.c:1684:3-9: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function.
> > >> ./platform/nuvoton/npcm-video.c:1690:3-9: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function.
> > >> ./platform/nuvoton/npcm-video.c:1694:1-7: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function.
> > >
> > > This driver uses this construct:
> > >
> > >                 struct device *ece_dev __free(put_device) = &ece_pdev->dev;
> > >
> > > to automatically call put_device. So this patch would 'put' the device twice.
> > >
> > > Does cocci understand constructs like this? If I hadn't looked closely at the
> > > code first, I would just have merged it.
> >
> > Oh wait, now that I am reading the following patches I see that it was those later
> > patches that add the __free code.
> >
> > This is far too confusing. Please post a v2 that just combines the 'fix references'
> > and 'use cleanup.h macros' in a single patch. It makes no sense to have this two-phase
> > approach.
> 
> I believe this is discouraged.
> 
> cleanup.h macros does not exist in old kernel versions, so makes it
> impossible to backport the fix to them.

That's not a problem, fix things properly in the main tree and let the
stable/lts kernels work it out on their own.

> This is an example of other series following this policy:
> https://lore.kernel.org/lkml/173608125422.1253657.3732758016133408588.stgit@devnote2/
> 
> They also mention the same here:
> https://hackerbikepacker.com/kernel-auto-cleanup-1 .... I am pretty
> sure that I read the policy in a more official location... but I
> cannot find it right now :)

No, it is NOT official policy at all.  Otherwise you would be saying
that no one could use these new functions for 6 years just because of
really old kernels still living around somewhere.  That's not how kernel
development works, thankfully.

thanks,

greg k-h