linux-kernel - Re: DRM GUD driver debug logging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ffbc01f2-5571-4fba-ae73-86f959922bcb@suse.de>
Date: Sat, 19 Jul 2025 18:44:17 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Ruben Wauters <rubenru09@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: DRM GUD driver debug logging

Hi

Am 17.07.25 um 17:02 schrieb Ruben Wauters:
> Hello
>
> I was taking a look at the code for the gud driver. I am aware this
> driver was recently orphaned and I wish to get up to speed on it, and
> maybe with enough learning and work I can take over maintainance of it
> in the future.

That's fantastic!

There's https://github.com/notro/gud?tab=readme-ov-file and 
https://github.com/notro/gud/wiki for more information about the gud 
protocol and driver.

>
> I noticed that in the function gud_disconnect in gud_driv.c on like 623
> there is the following code
>
> 	drm_dbg(drm, "%s:\n", __func__);
>
> checkpatch.pl marks this as unnecessary ftrace like logging, and
> suggests to use ftrace instead. Since (as far as I can tell) this
> effectively just prints the function name, would it not be better to
> just use ftrace for debugging and remove this call all together?

I'm not a great fan of these types of debugging code. We already have 
this in the DRM core/helpers. Whatever drivers print for debugging 
should be more helpful than just the function name. So IMHO this can be 
removed.

>
> While it isn't actively *harming* the code as such, it does seem a bit
> unnecessary.
>
> I'd like to know the DRM maintainers opinions. I know this particular
> driver does not have a maintainer dedicated to it, so I'd like to know
> the opinion of those that maintain the subsystem, and anyone else that
> has any opinion.

If you want to do meaningful work on the driver, you could replace 
struct drm_simple_display_pipe with the real DRM helpers.  The struct is 
an artifact from older driver designs, but is now obsolete. Drivers are 
supposed to be converted to DRM atomic helpers. For an experienced dev, 
it's a copy-past job. For a newcomer, it's a nice introduction to the 
DRM code. If you want to take a look, you can study commit 4963049ea1ae 
("drm/hyperv: Replace simple-KMS with regular atomic helpers"), which 
recently did this conversion for the hyperv driver.

Best regards
Thomas

>
> Thank you
>
> Ruben Wauters

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)