[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251215181605.29f6afcc@pumpkin>
Date: Mon, 15 Dec 2025 18:16:05 +0000
From: David Laight <david.laight.linux@...il.com>
To: Daniel Stone <daniel@...ishbar.org>
Cc: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>, Sandy Huang
<hjc@...k-chips.com>, Heiko Stübner <heiko@...ech.de>,
Andy Yan <andy.yan@...k-chips.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Chaoyi Chen <chaoyi.chen@...k-chips.com>,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org, Daniel
Stone <daniels@...labora.com>, kernel@...labora.com
Subject: Re: [PATCH v4 1/8] drm/rockchip: vop2: Switch impossible format
conditional to WARN_ON
On Mon, 15 Dec 2025 17:55:13 +0000
Daniel Stone <daniel@...ishbar.org> wrote:
> Hi,
>
> On Fri, 12 Dec 2025 at 12:46, Nicolas Frattaroli
> <nicolas.frattaroli@...labora.com> wrote:
> > On Thursday, 11 December 2025 23:38:22 Central European Standard Time David Laight wrote:
> > > Except that all the systems with PANIC_ON_WARN set will panic.
> > > I believe that is somewhere over 90% of systems.
> >
> > I also like making up statistics. Warning here is the correct move
> > in my opinion because this warning being triggered indicates a bug
> > in the kernel code, and with PANIC_ON_WARN the user explicitly says
> > they would rather panic in such a case than treat it as an abnormal
> > condition that is recoverable.
> >
> > The reason why this condition ever occurring should be treated as an
> > abnormal condition is because the DRM subsystem should guarantee we
> > don't get a framebuffer of a format we didn't explicitly declare
> > support for in the first place. So this condition being hit either
> > means drm_universal_plane_init is broken, or the array of formats
> > that's passed to it is out of sync with the conversion code, which
> > is also a bug. Or someone managed to thoroughly hose DRM's internal
> > kernel-side data structures, which is precisely the kind of thing
> > PANIC_ON_WARN users want to abort for.
>
> Yes, that's exactly it. We make all kinds of load-bearing assumptions
> everywhere: that PM code won't pass in a NULL struct device pointer to
> the resume handler, that our driver callbacks won't get called whilst
> the device is runtime-suspended, etc. We could try to handle every
> single one of those with if (clk == NULL) return 0; /* ??? */, or we
> could not.
>
> If you'd like, we could just delete every one of these checks and
> replace them with comments, explaining what we assume the invariants
> to be, and wait for an OOPS due to dereferencing invalid pointers. But
> the MISRA style of 'handling' every possible impossible case is not
> tractable.
Especially since it is often easier to debug the NULL pointer reference
that the work out why a BUG_ON(!ptr) happened.
(In the former case you should have the contents of all the registers
making it easier to backtrack to where the NULL came from.)
David
>
> Cheers,
> Daniel
Powered by blists - more mailing lists