Re: [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line

[Jocelyn Falempe]

On 10/16/26 07:27, Devin Wittmayer wrote:
> The kmsg panic screen reads the log back one chunk at a time. A record
> that does not fit in a chunk comes back empty without moving on, so the
> screen asks for it again forever and the panic never finishes. Every
> oops logs the list of loaded modules, and on my machine that list does
> not fit.
>
> Read into a buffer the size printk formats one record into, and stop if
> a read still comes back empty.

Thanks for investigating this issue.
That looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>

I'm sorry, I probably forgot to push [1], so I will push both patches 
next week if no other comments.

[1] https://lore.kernel.org/all/20260126100732.365-1-luyuantao01@xxxxxxx/

Best regards,

--

Jocelyn
> Fixes: 54034bebb22f ("drm/panic: Add a kmsg panic screen")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Devin Wittmayer <lucid_duck@xxxxxxxxxxxxx>
> ---
> I usually work on Wi-Fi drivers, so this is new ground for me. While I
> was testing a fix for a Wi-Fi driver crash, my test machine froze after
> the crash where it should have rebooted, and stayed that way until I
> power cycled it. Working out why led me here.
>
> To reproduce, with the kmsg screen and panic=10 on a display driver with
> panic support, as root:
>
>    printf "%600s\n" "" | tr ' ' x > /dev/kmsg
>    echo c > /proc/sysrq-trigger
>
> Before this patch the panic never gets to its reboot.
>
> QEMU, 10 runs each, on drm-misc-fixes and drm-misc-next (bochs) and
> 6.12.113 (simpledrm):
>
>                                       before    after
>    one 600 char line                  hangs     reboots
>    same, then 400 short lines         hangs     reboots
>    one record of 300 short lines      hangs     reboots
>    oops with 24 modules loaded        hangs     reboots
>    plain panic                        reboots   reboots
>
> amdgpu, 2 runs each, Arch's source and config, before and after:
>
>    6.18.52, the two commands above    hangs     reboots
>    6.18.52, one record of 300 lines   hangs     reboots
>    6.18.52, oops                      hangs     reboots
>    7.2.6, kmsg screen set, oops       hangs     reboots
>
> luyuantao's patch [1] stops reading once the screen is full, so it only
> helps the second row. With both on drm-misc-fixes, every row reboots.
>
> The buffer is shared, like the QR code buffers; a panic only draws from
> one CPU. In drm-misc-next this code is in drm_panic_helper.c, where the
> same change was tested.
>
> [1] https://lore.kernel.org/all/20260126100732.365-1-luyuantao01@xxxxxxx/
>
>   drivers/gpu/drm/drm_panic.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index e576c4791861d..29b1487de7ae0 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -556,7 +556,8 @@ static void draw_panic_screen_kmsg(struct drm_scanout_buffer *sb)
>   	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
>   	struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
>   	struct kmsg_dump_iter iter;
> -	char kmsg_buf[512];
> +	/* the size printk formats one record into, too big for the stack */
> +	static char kmsg_buf[2048];
>   	size_t kmsg_len;
>   	struct drm_panic_line line;
>   	int yoffset;
> @@ -574,6 +575,10 @@ static void draw_panic_screen_kmsg(struct drm_scanout_buffer *sb)
>   		char *start;
>   		char *end;
>   
> +		/* a record that does not fit comes back empty, every time */
> +		if (!kmsg_len)
> +			break;
> +
>   		/* ignore terminating NUL and newline */
>   		start = kmsg_buf + kmsg_len - 2;
>   		end = kmsg_buf + kmsg_len - 1;
>
> base-commit: 324fac81f365fc123247f9c8a52cb66ebd93b5e5