Re: [Re] kernel panic during shutdown in v7.1+ with PREEMPT_RT

[Ostrowski, Rafal]

> This can be fixed like this
> 
> struct dc_plane_state *dc_create_plane_state(const struct dc *dc)
> {
>         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;
> 
>         kref_init(&plane_state->refcount);
>         dc_plane_construct(dc->ctx, plane_state);
> 
>         return plane_state;
> }
> 
> I also found missing DC_RUN_WITH_PREEMPTION_ENABLED in an error path, I'm preparing
> a patch for next-20260721 (next-20260722+ have broken reboot due to the mt7925 issue
> mentioned above).
>  Also, is preempt_{disable,enable}() really necessary in DC_FPU_{BEGIN,END}?
> Could migrate_{disable,enable}() be enough instead? This way we wouldn't need
> DC_RUN_WITH_PREEMPTION_ENABLED at all(?).
> 
> Bert Karwatzki

Hi Bert,

Sorry for the trouble these changes caused on your setup. It reproduces deterministically on PREEMPT_RT — that's the key condition here. On RT the page allocator always takes a sleeping rt_spin_lock, so any allocation inside the FPU-protected region is illegal, whereas on non-RT kernels the same allocation usually stays on the fast path and doesn't sleep, which is why this hasn't been widely reported.

Some context on how the FPU handling is meant to work:

DC_FP_START/END (DC_FPU_BEGIN/END): these require a non-preemptible context. The kernel needs exclusive use of the FPU while we run FP code, so preemption must be disabled around it. Note that kernel_fpu_begin() itself does preempt_disable() (via fpregs_lock()) unconditionally — on PREEMPT_RT as well. That directly answers your migrate_disable() question: swapping the preempt_disable/enable in dc_fpu_begin/end for migrate_disable/enable would not make the region preemptible, because kernel_fpu_begin() disables preemption on its own regardless. migrate_disable() would only pin us to the CPU (enough to protect the per-CPU recursion counter), but the region would still be atomic, so it wouldn't remove the need for DC_RUN_WITH_PREEMPTION_ENABLED. The only thing that actually re-enables preemption is leaving the FPU (kernel_fpu_end()), which is exactly what that macro does.

DML layer: the whole DML layer is compiled as FPU code (see the FP compilation flags in the makefile), with only a few wrapper/exception units. As a result, any function that calls into DML must wrap the call in a DC_FP_START()/DC_FP_END() block.

DC_RUN_WITH_PREEMPTION_ENABLED: this is the workaround for the cases where we must allocate or sleep while inside an FPU region. The important rule is that this macro must always live in a non-FPU translation unit (a DC file or a DML wrapper/exception file) — never in pure FPU-compiled DML code, since it manipulates the FPU guard itself. With that in mind there are effectively two cases:

DC → DC_FP_START → call DML → no allocation/sleep in the DML path → back to DC → DC_FP_END. Clean case, no macro needed.
DC → DC_FP_START → call DML → DML calls back into a DC (or wrapper) function that allocates/sleeps → wrap the affected code (allocation, sleep) in DC_RUN_WITH_PREEMPTION_ENABLED in that DC function → back to DML → back to DC → DC_FP_END.
The phantom-plane allocation is the second case: DML calls back into dc_create_plane_state(), which is a DC function, so the wrap belongs there.

About the fix:
DC_RUN_WITH_PREEMPTION_ENABLED(plane_state = kvzalloc_obj(*plane_state, GFP_ATOMIC));

This fix looks correct to me.

Kind Regards,
Rafal