Re: [PATCH v3] drm/drv: Convert wedged event string building to seq_buf

On 28-07-2026 11:26 pm, Jani Nikula wrote:
> On Tue, 28 Jul 2026, Mallesh Koujalagi<[email protected]> wrote:
>> event_string[] has a fixed size of WEDGE_STR_LEN (32) bytes.
>> The original scnprintf()-based loop required a manual pre-flight
>> bounds check.
>>
>> Replace the manual bookkeeping with seq_buf, which tracks overflow
>> internally. seq_buf_printf() writes each "method," token into the
>> buffer, and seq_buf_has_overflowed() detects truncation immediately
>> after the write that caused it — using drm_WARN_ONCE() to report it.
>>
>> On overflow, len retains the position of the last
>> successful write, so the trailing comma is stripped cleanly without
>> including any partial method name in the uevent payload.
>>
>> Fixes: b7cf9f4ac1b8 ("drm: Introduce device wedged event")
>> Signed-off-by: Mallesh Koujalagi<[email protected]>
>> ---
>> v2:
>> - Add proper logic to handle recovery string. (Raag)
>>
>> v3:
>> - Convert manual bounds check to seq_buf. (Jani Nikula)
>> - Use drm_WARN_ONCE() instead of drm_WARN_ON() for overflow. (Raag)
>> ---
>>   drivers/gpu/drm/drm_drv.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index e51ed959da89..6a5e3fa1350c 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -36,6 +36,7 @@
>>   #include <linux/mount.h>
>>   #include <linux/pseudo_fs.h>
>>   #include <linux/sched.h>
>> +#include <linux/seq_buf.h>
>>   #include <linux/slab.h>
>>   #include <linux/sprintf.h>
>>   #include <linux/srcu.h>
>> @@ -578,15 +579,22 @@ int drm_dev_wedged_event(struct drm_device *dev, unsigned long method,
>>   	char *envp[] = { event_string, NULL, NULL, NULL };
>>   	const char *recovery = NULL;
>>   	unsigned int len, opt;
>> +	struct seq_buf buf;
>>   
>> -	len = scnprintf(event_string, sizeof(event_string), "%s", "WEDGED=");
>> +	seq_buf_init(&buf, event_string, sizeof(event_string));
> See DECLARE_SEQ_BUF(). Remove event_string altogether (or make that the
> seq_buf name).
Sure!
>> +	seq_buf_puts(&buf, "WEDGED=");
>> +	len = seq_buf_used(&buf);
> Why do you need to keep len around?

|len is kept as the end of the last complete entry, not as an append 
cursor.
On overflow I use it to roll back to the last known-good comma and 
NUL-terminate there,
avoiding a partially written recovery method in the uevent string.|

>
>>   
>>   	for_each_set_bit(opt, &method, BITS_PER_TYPE(method)) {
>>   		recovery = drm_get_wedge_recovery(opt);
>>   		if (drm_WARN_ONCE(dev, !recovery, "invalid recovery method %u\n", opt))
>>   			break;
>>   
>> -		len += scnprintf(event_string + len, sizeof(event_string) - len, "%s,", recovery);
>> +		seq_buf_printf(&buf, "%s,", recovery);
>> +		if (drm_WARN_ONCE(dev, seq_buf_has_overflowed(&buf),
>> +				  "WEDGED event string truncated\n"))
>> +			break;
> Nah, you can keep printing to seq_buf, and only check at the end once
> instead of looping.

Agreed!, will update next revision.

Thanks,

-/Mallesh

>> +		len = seq_buf_used(&buf);
>>   	}
>>   
>>   	if (recovery)