Re: [PATCH] drm/radeon: free iio for atombios when driver shutdown

From: Daniel Vetter <daniel@ffwll.ch>
To: Alex Deucher <alexdeucher@gmail.com>
Cc: "Liwei Song" <liwei.song@windriver.com>,
	AlexDeucher <alexander.deucher@amd.com>,
	ChristianKönig <christian.koenig@amd.com>,
	PanXinhui <Xinhui.Pan@amd.com>, DavidAirlie <airlied@gmail.com>,
	DanielVetter <daniel@ffwll.ch>,
	amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/radeon: free iio for atombios when driver shutdown
Date: Fri, 6 Jan 2023 19:52:47 +0100	[thread overview]
Message-ID: <Y7ht/7gujgsrQt3Z@phenom.ffwll.local> (raw)
In-Reply-To: <CADnq5_Mu-9_HoH9PULANvWS39D0Sra+nxodwFspH-TXX_3GJKA@mail.gmail.com>

Just a quick drive-by. For these simple cases where we just need to make
sure that memory is freed using drmm_kmalloc and friends should help
simplify things. Probably not worth it for radeon, but figured I'll throw
it out there.

For more functional code switching to drmm is harder because you need the
right order. But for these all that matters is that stuff gets freed so
there's no leak, and drmm can take care of that without ordering
constraints.
-Daniel

On Fri, Jan 06, 2023 at 10:36:53AM -0500, Alex Deucher wrote:
> Applied.  Thanks!
> 
> Alex
> 
> On Fri, Jan 6, 2023 at 5:00 AM Liwei Song <liwei.song@windriver.com> wrote:
> >
> > Fix below kmemleak when unload radeon driver:
> >
> > unreferenced object 0xffff9f8608ede200 (size 512):
> >   comm "systemd-udevd", pid 326, jiffies 4294682822 (age 716.338s)
> >   hex dump (first 32 bytes):
> >     00 00 00 00 c4 aa ec aa 14 ab 00 00 00 00 00 00  ................
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<0000000062fadebe>] kmem_cache_alloc_trace+0x2f1/0x500
> >     [<00000000b6883cea>] atom_parse+0x117/0x230 [radeon]
> >     [<00000000158c23fd>] radeon_atombios_init+0xab/0x170 [radeon]
> >     [<00000000683f672e>] si_init+0x57/0x750 [radeon]
> >     [<00000000566cc31f>] radeon_device_init+0x559/0x9c0 [radeon]
> >     [<0000000046efabb3>] radeon_driver_load_kms+0xc1/0x1a0 [radeon]
> >     [<00000000b5155064>] drm_dev_register+0xdd/0x1d0
> >     [<0000000045fec835>] radeon_pci_probe+0xbd/0x100 [radeon]
> >     [<00000000e69ecca3>] pci_device_probe+0xe1/0x160
> >     [<0000000019484b76>] really_probe.part.0+0xc1/0x2c0
> >     [<000000003f2649da>] __driver_probe_device+0x96/0x130
> >     [<00000000231c5bb1>] driver_probe_device+0x24/0xf0
> >     [<0000000000a42377>] __driver_attach+0x77/0x190
> >     [<00000000d7574da6>] bus_for_each_dev+0x7f/0xd0
> >     [<00000000633166d2>] driver_attach+0x1e/0x30
> >     [<00000000313b05b8>] bus_add_driver+0x12c/0x1e0
> >
> > iio was allocated in atom_index_iio() called by atom_parse(),
> > but it doesn't got released when the dirver is shutdown.
> > Fix this kmemleak by free it in radeon_atombios_fini().
> >
> > Signed-off-by: Liwei Song <liwei.song@windriver.com>
> > ---
> >  drivers/gpu/drm/radeon/radeon_device.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> > index 92905ebb7b45..1c005e0ddd38 100644
> > --- a/drivers/gpu/drm/radeon/radeon_device.c
> > +++ b/drivers/gpu/drm/radeon/radeon_device.c
> > @@ -1022,6 +1022,7 @@ void radeon_atombios_fini(struct radeon_device *rdev)
> >  {
> >         if (rdev->mode_info.atom_context) {
> >                 kfree(rdev->mode_info.atom_context->scratch);
> > +               kfree(rdev->mode_info.atom_context->iio);
> >         }
> >         kfree(rdev->mode_info.atom_context);
> >         rdev->mode_info.atom_context = NULL;
> > --
> > 2.33.1
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch