Linux-Next discussions
 help / color / mirror / Atom feed
From: "mikhail.v.gavrilov@gmail.com" <mikhail.v.gavrilov@gmail.com>
To: Bert Karwatzki <spasswolf@web.de>, linux-kernel@vger.kernel.org
Cc: linux-next@vger.kernel.org, linux-rt-devel@lists.linux.dev,
	 amd-gfx@lists.freedesktop.org,
	"# = v7 . 1" <stable@vger.kernel.org>,
	Alex Deucher <alexander.deucher@amd.com>,
	Rafal Ostrowski <rafal.ostrowski@amd.com>,
	Mario Limonciello	 <mario.limonciello@amd.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	 Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] drm/amd/display: fix usage of DC_FPU_{BEGIN,END} with PREEMPT_RT
Date: Tue, 28 Jul 2026 05:51:53 +0500	[thread overview]
Message-ID: <ef1bc3d070c4156f1811c0c4a567ec98ac8fadaf.camel@gmail.com> (raw)
In-Reply-To: <20260727105059.75716-1-spasswolf@web.de>

On Mon, 2026-07-27 at 12:50 +0200, Bert Karwatzki wrote:
> On PREEMPT_RT kernels kvzalloc_obj() can sleep because spin_lock is
> converted to rt_mutex. dc_create_plane_state() can be called while
> inside an FPU-guarded region, resuling in "scheduling while atomic"
> errors on PREEMPT_RT kernels.
>  Fix this by calling kvzalloc_obj() with
> DC_RUN_WITH_PREEMPTION_ENABLED().
> Also fix the error path in dc_create_stream_for_sink().
> 
> Fixes: 3539437f354b ("drm/amd/display: Move FPU Guards From DML To DC
> - Part 1")
> Link:
> https://lore.kernel.org/lkml/20260723123449.6494-1-spasswolf@web.de/
> 
> Signed-off-by: Bert Karwatzki <spasswolf@web.de>
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_stream.c  | 5 +++--
>  drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> index dbc12640b01c..4ac835777b58 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> @@ -233,8 +233,9 @@ struct dc_stream_state
> *dc_create_stream_for_sink(
>  
>  fail:
>  	if (stream) {
> -		kfree(stream->update_scratch);
> -		kfree(stream);
> +		if (stream->update_scratch)
> +			DC_RUN_WITH_PREEMPTION_ENABLED(kfree(stream-
> >update_scratch));
> +		DC_RUN_WITH_PREEMPTION_ENABLED(kfree(stream));
>  	}
>  
>  	return NULL;
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> index 88e825a6582c..d5c6427796b6 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> @@ -85,8 +85,8 @@ uint8_t  dc_plane_get_pipe_mask(struct dc_state
> *dc_state, const struct dc_plane
>  
> *********************************************************************
> *********/
>  struct dc_plane_state *dc_create_plane_state(const struct dc *dc)
>  {
> -	struct dc_plane_state *plane_state =
> kvzalloc_obj(*plane_state,
> -							 
> GFP_ATOMIC);
> +	struct dc_plane_state *plane_state;
> +	DC_RUN_WITH_PREEMPTION_ENABLED(plane_state =
> kvzalloc_obj(*plane_state, GFP_ATOMIC));
>  
>  	if (NULL == plane_state)
>  		return NULL;

Hi Bert,

You may not be aware that the same allocation is already wrapped once,
at the dcn32 call site:

  183182235f6d ("drm/amd/display: Wrap DCN32 phantom-plane allocation
in DC_RUN_WITH_PREEMPTION_ENABLED")

That one only covers the dcn32 DML1 path, while your trace goes through
dcn401_validate_bandwidth() and dml21 - so wrapping the call site could
never have caught your case.  Which is a good argument for guarding the
allocation in the callee, as you do: dc_create_plane_state() is reached
from every DCN and both DML generations.

On dcn32 the two wraps now nest.  That is harmless, because
DC_RUN_WITH_PREEMPTION_ENABLED() is conditional on dc_is_fp_enabled():
once the outer instance has left the FPU region, the inner one expands
to a plain call.  But it does make the dcn32 wrap redundant, and I
think it should be dropped in a follow-up now that the allocation is
guarded in the callee.

One thing that may be worth adjusting in the commit message: this is
not only a PREEMPT_RT problem.  183182235f6d was needed on a plain non-
RT x86 kernel. There DC_FP_START() takes fpregs_lock(), which disables
local softirqs, and dc_plane_state is around 335 KiB, so kvzalloc_obj()
falls through to the vmalloc path and hits BUG_ON(in_interrupt()).  So
on RT any allocation inside the FPU region is illegal, while on non-RT
it is specifically the large ones - two failure modes, one root cause.

About the FPU register question raised by the review bot: as far as I
can see it applies to every existing user of
DC_RUN_WITH_PREEMPTION_ENABLED(), 183182235f6d included, so it looks
like a property of the macro rather than something your patch
introduces.  An answer from AMD on that would be useful either way.

I have dcn32 hardware here (RX 7900 XTX) and can test the patch on
non-RT if that helps.

-- 
Thanks,
Mikhail

  reply	other threads:[~2026-07-28  0:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260723124353.13040-1-spasswolf@web.de>
2026-07-23 13:10 ` [Re] kernel panic during shutdown in v7.2-rc{3,4} and next-20260722 with PREEMPT_RT Bert Karwatzki
2026-07-23 13:23   ` Bert Karwatzki
2026-07-23 16:17     ` Bert Karwatzki
2026-07-23 22:51       ` [Re] kernel panic during shutdown in next-20260722 Bert Karwatzki
2026-07-24 15:08         ` Bert Karwatzki
2026-07-25 19:58           ` Bert Karwatzki
2026-07-25 23:16             ` Bert Karwatzki
2026-07-26 18:47               ` Bert Karwatzki
2026-07-26 22:52                 ` Bert Karwatzki
2026-07-27 10:06                   ` [Re] kernel panic during shutdown in v7.1+ with PREEMPT_RT Bert Karwatzki
2026-07-27 10:35                     ` Ostrowski, Rafal
2026-07-27 10:50                       ` [PATCH] drm/amd/display: fix usage of DC_FPU_{BEGIN,END} " Bert Karwatzki
2026-07-28  0:51                         ` mikhail.v.gavrilov [this message]
2026-07-29 12:35                           ` Bert Karwatzki
2026-07-29 14:39                             ` Mikhail Gavrilov
2026-07-29 17:46                               ` Bert Karwatzki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ef1bc3d070c4156f1811c0c4a567ec98ac8fadaf.camel@gmail.com \
    --to=mikhail.v.gavrilov@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=bigeasy@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=mario.limonciello@amd.com \
    --cc=rafal.ostrowski@amd.com \
    --cc=spasswolf@web.de \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox