* Re: [PATCH] fbcon: Avoid cancel_delayed_work_sync() in atomic context
2026-10-26 14:59 [PATCH] fbcon: Avoid cancel_delayed_work_sync() in atomic context Yao Yiqi
@ 2026-10-26 15:24 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-26 15:24 UTC (permalink / raw)
To: Yao Yiqi; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] fbcon: fbcon_del_cursor_work() still sleeps in atomic context on PREEMPT_RT
- [Medium] fbcon: fb_flashcursor() re-queues cursor_work after cancel_delayed_work()
--
commit 81badb044cb0837eebe19d72924931a878fcf8e9
Author: Yao Yiqi <yaoyiqi3@huawei.com>
fbcon: Avoid cancel_delayed_work_sync() in atomic context
This commit updates fbcon_del_cursor_work() to use cancel_delayed_work()
instead of cancel_delayed_work_sync() when in_interrupt() or irqs_disabled()
is true, avoiding sleeping when fbcon_cursor() is called from the console
printing path with interrupts disabled.
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 7720dfc80c165..14611be17cb19 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
[ ... ]
> @@ -441,7 +444,14 @@ static void fbcon_del_cursor_work(struct fb_info *info)
> {
> struct fbcon_par *par = info->fbcon_par;
>
> - cancel_delayed_work_sync(&par->cursor_work);
> + /*
> + * fbcon_cursor() can be reached from the console printing path with
> + * interrupts disabled. Only synchronize when sleeping is allowed.
> + */
> + if (in_interrupt() || irqs_disabled())
> + cancel_delayed_work(&par->cursor_work);
> + else
> + cancel_delayed_work_sync(&par->cursor_work);
> }
[Severity: High]
Can fbcon_del_cursor_work() still call cancel_delayed_work_sync() from an
atomic context when hardware interrupts are enabled?
On CONFIG_PREEMPT_RT, force_legacy_kthread() is true, so
console_emit_next_record() invokes con->write() (vt_console_print()) in task
context without disabling interrupts:
kernel/printk/printk.c:console_emit_next_record() {
...
if (force_legacy_kthread() && !panic_in_progress()) {
con->write(con, outbuf, pmsg.outbuf_len);
con->seq = pmsg.seq + 1;
}
...
}
Inside vt_console_print(), spin_trylock(&printing_lock) calls
__rt_spin_trylock(), which acquires printing_lock (an LD_WAIT_CONFIG lock),
enters rcu_read_lock(), and calls migrate_disable() while leaving hardware
interrupts enabled:
drivers/tty/vt/vt.c:vt_console_print() {
...
if (!spin_trylock(&printing_lock))
return;
...
if (con_is_fg(vc))
hide_cursor(vc);
...
}
When vc->vc_cursor_type & CUR_SW is set, hide_cursor(), lf() -> con_scroll() ->
fbcon_scroll(), and set_cursor() call into fbcon_cursor() ->
fbcon_del_cursor_work(). Because both in_interrupt() and irqs_disabled() are
false, fbcon_del_cursor_work() still calls cancel_delayed_work_sync() ->
__cancel_work_sync() while holding printing_lock and rcu_read_lock().
Similarly, in_interrupt() is marked deprecated in include/linux/preempt.h and
only checks irq_count(), so it does not detect preempt_disable(), spin_lock(),
or rcu_read_lock() critical sections (for example during an oops via
bust_spinlocks(0) -> console_unblank() -> do_unblank_screen() ->
fbcon_cursor()).
Since fbcon_cursor() only stops cursor blinking and does not free par, while
teardown paths such as fbcon_release() and fbcon_deinit() need synchronous
draining, should fbcon_cursor() use a non-synchronizing cancel directly instead
of checking in_interrupt() || irqs_disabled()?
[Severity: Medium]
Also, when fbcon_del_cursor_work() uses cancel_delayed_work() instead of
cancel_delayed_work_sync(), can an in-flight fb_flashcursor() work item
immediately re-arm par->cursor_work after it was canceled?
In fb_flashcursor(), queue_delayed_work() is called outside console_lock and
without checking whether CUR_SW is set or whether the work was canceled:
drivers/video/fbdev/core/fbcon.c:fb_flashcursor() {
...
par->bitops->cursor(vc, info, enable,
get_fg_color(vc, info, c),
get_bg_color(vc, info, c));
console_unlock();
queue_delayed_work(system_power_efficient_wq, &par->cursor_work,
par->cur_blink_jiffies);
}
If par->cursor_work was armed (for instance via fbcon_startup(),
fbcon_switch(), fbcon_unblank(), cursor_blink_store(), or when switching to
CUR_SW with CUR_NONE) and fb_flashcursor() is running, vt_console_print() ->
hide_cursor() / set_cursor() -> fbcon_cursor() -> fbcon_del_cursor_work() can
run during or right after fb_flashcursor()'s console_unlock() call.
Because cancel_delayed_work() does not wait for the running fb_flashcursor()
callback to finish, fb_flashcursor() will proceed to queue_delayed_work() and
re-arm par->cursor_work indefinitely.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026145916.414396-1-yyaqia@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread