Re: [PATCH 01/12] drm/panic: Allocate QR-code buffers statically

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 19.08.26 um 11:38 schrieb Geert Uytterhoeven:
> Hi Thomas,
>
> On Tue, 18 Aug 2026 at 15:00, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Declare qrbuf1 and qrbuf2 as static arrays so that the module loader
>> allocates them for us. Avoids the kmalloc later on. Also allows for
>> using sizeof() to get the number of bytes in each array. Access the
>> arrays once with memset, so that the physical pages are available on
>> a panic.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Thanks for your patch!
>
>> --- a/drivers/gpu/drm/drm_panic.c
>> +++ b/drivers/gpu/drm/drm_panic.c
>> @@ -628,24 +628,23 @@ MODULE_PARM_DESC(panic_qr_version, "maximum version (size) of the QR code");
>>   #define WINDOW_BITS 12
>>   #define MEM_LEVEL 4
>>
>> -static char *qrbuf1;
>> -static char *qrbuf2;
>> +static u8 qrbuf1[QR_BUFFER1_SIZE];
>> +static u8 qrbuf2[QR_BUFFER2_SIZE];
> I am not a big fan of increasing kernel size like this.
> You may run (faster) into boot loader limitations.

Which limitation would this be?

>
>>   static struct z_stream_s stream;
>>
>>   static void __init drm_panic_qr_init(void)
>>   {
>> -       qrbuf1 = kmalloc(QR_BUFFER1_SIZE, GFP_KERNEL);
>> -       qrbuf2 = kmalloc(QR_BUFFER2_SIZE, GFP_KERNEL);
>> +       /* best-effort allocation; can be NULL */
>>          stream.workspace = kmalloc(zlib_deflate_workspacesize(WINDOW_BITS, MEM_LEVEL),
>>                                     GFP_KERNEL);
> <ironic>Why not use a static array for this, too?</ironic>

I tried, by the size is calculated at runtime.


>
>> +
>> +       /* touch memory so that pages are there in the case of a panic */
>> +       memset(qrbuf1, 0, sizeof(qrbuf1));
>> +       memset(qrbuf2, 0, sizeof(qrbuf2));
> Please clarify "there"?
> In cache? Not all systems have sufficient data cache, so it may be
> evicted at any time.
> In RAM? AFAIK kernel and module memory is not demand-paged.
> In TLB? Like cache, it may be evicted at any time.

I'd like to avoid looking for a page after a panic has already occurred. 
But I guess it might not make a difference, given all the DRM that is 
involved.

The motivation here was that I did not like that panic handling depends 
on a number of dynamic kmallocs, with which we don't even detect alloc 
failures until a panic has occurred.

>
> So IMHO this is futile.

Noted.

Best regards
Thomas

>
>>   }
> Gr{oetje,eeting}s,
>
>                          Geert
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)