Re: [PATCH] drm/vblank: fix misuse of drm_WARN in drm_wait_one_vblank()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sat, 11 Jan 2025, Vitaliy Shevtsov <v.shevtsov@maxima.ru> wrote:
> On Fri, 10 Jan 2025 20:19:47 +0100
> Simona Vetter <simona.vetter@ffwll.ch> wrote:
>
>> Hm, unless a drivers vblank handling code is extremely fun, there should
>> be absolutely no memory allocations or user copies in there at all. Hence
>> I think you're papering over a real bug here. The vblank itself should be
>> purely a free-wheeling hrtimer, if those stop we have serious kernel bug
>> at our hands.
>> 
>> Which wouldn't be a big surprise, because we've fixed a _lot_ of bugs in
>> vkms' vblank and page flip code, it's surprisingly tricky.
>> 
>> Iow, what kind of memory allocation is holding up vkms vblanks?
>> 
>> Cheers, Sima
>> 
>
> I don't think this is because of memory allocation. As far as I can see
> there is no memory allocation in vblanks handling. Okay, there is a kzalloc()
> call in vkms_atomic_crtc_reset() without checking a pointer but this is
> not the root cause of this issue. My first thought was that somehow a
> vblank might not be successfully processed by drm_crtc_handle_vblank() in
> vkms_vblank_simulate() function which always returns HRTIMER_RESTART even
> if a vblank handling failed. But this hypothesis was not confirmed -
> all vblanks are fine. The hrtimers in vkms have a hardcoded framedur
> value of 16ms and what I can see is that the fault injection creates
> some delays by unwinding the call stack when it simulates an allocation
> failure and this caused the hrtimers to lag. This what I was able to
> investigate while I was debugging the kernel in the gdb.
>
> A similar issue was being discussed in
> https://lore.kernel.org/linux-kernel//0000000000009cd8d505bd545452@google.com/T/

Seems to me in most cases we do want the WARN, but there are corner
cases. Arguably those should be addressed instead to ensure we won't
ignore the real bugs. We want the warning, you want the panic.


BR,
Jani.


-- 
Jani Nikula, Intel