[PATCH] drm/atmel-hlcdc: Release CRTC commit when destroying plane state

From: Pierre-Louis Dourneau
Date: Wed Mar 06 2024 - 14:50:18 EST


From: Arnaud Lahache <a.lahache@xxxxxxxxxx>

Fixes a memory leak occurring on each modeset update.

Running a program such as libdrm's modetest[0] with this driver exhausts
all available memory after a few minutes. Enabling kmemleak yields a series
of such leak reports:

unreferenced object 0xc21acf40 (size 64):
comm "modetest", pid 210, jiffies 4294942460 (age 331.240s)
hex dump (first 32 bytes):
00 a0 a1 c1 01 00 00 00 ff ff ff ff 4c cf 1a c2 ............L...
4c cf 1a c2 ff ff ff ff 58 cf 1a c2 58 cf 1a c2 L.......X...X...
backtrace:
[<d68b3e09>] kmalloc_trace+0x18/0x24
[<f858a020>] drm_atomic_helper_setup_commit+0x1e0/0x7e0
[<26e8ab04>] drm_atomic_helper_commit+0x40/0x160
[<49708b0c>] drm_atomic_commit+0xa8/0xf0
[<e58c2942>] drm_mode_obj_set_property_ioctl+0x154/0x3d8
[<5e97e57d>] drm_ioctl+0x200/0x3c4
[<ed514ba1>] sys_ioctl+0x240/0xb48
[<26aab344>] ret_fast_syscall+0x0/0x44

drm_atomic_helper_setup_commit() acquires a reference to a drm_crtc_commit
for each CRTC and associated connectors and planes involved in a modeset.
64-byte leaks map well to the size of a drm_crtc_commit on 32-bit
architectures, and the second 4-byte chunk in the hex dump above awfully
looks like a reference count.

We tracked this missing reference decrement down to the driver's
atmel_hlcdc_plane_atomic_destroy_state() callback. Its CRTC counterpart,
atmel_hlcdc_crtc_destroy_state(), calls into the drm_atomic helpers and
properly releases its references to the commit. Planes didn't. Using the
default helper for that purpose, __drm_atomic_helper_plane_destroy_state(),
fixes the leak and avoids reimplementing the same logic in the driver.

[0]: https://gitlab.freedesktop.org/mesa/drm/-/tree/main/tests/modetest
Invoke with `modetest -M atmel-hlcdc -s 32:#0 -v`, assuming 32 is the
ID of a connector.

Signed-off-by: Arnaud Lahache <a.lahache@xxxxxxxxxx>
Co-developed-by: Pierre-Louis Dourneau <pl.dourneau@xxxxxxxxxx>
Signed-off-by: Pierre-Louis Dourneau <pl.dourneau@xxxxxxxxxx>
Co-developed-by: Benoît Alcaina <b.alcaina@xxxxxxxxxx>
Signed-off-by: Benoît Alcaina <b.alcaina@xxxxxxxxxx>
---
As far as our testing goes, we've been running 6 of our production units
with this patch for more than 2 weeks as per the date this patch is sent
out. We can report stable memory usage.

Admittedly, our usage of the DRM uAPI is rather simple: create 2 dumb
buffers, do an initial MODE_SETCRTC, and then MODE_PAGE_FLIP between the
two dumb buffers at the rate of once per second on average. We haven't
evaluated more complex workloads.

drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index daa508504f47..390c4fc62af7 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -934,8 +934,7 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
state->dscrs[i]->self);
}

- if (s->fb)
- drm_framebuffer_put(s->fb);
+ __drm_atomic_helper_plane_destroy_state(s);

kfree(state);
}

base-commit: 9dfc46c87cdc8f5a42a71de247a744a6b8188980
--
2.34.1