fbcon: Avoid deleting a timer in IRQ context

Started by Thierry Reding on May 21, 2015 · 4 messages from 3 authors · Last reply May 27, 2015

Subsystems (3)

Maintainers (3)

STALE4187d REVIEWED · 2 reviews, 0 by maintainers

Status: Reviewed.

2 review trailers.

open the first message on its own page

Thierry Reding

Message details and related content
Open message details and related content
From: Thierry Reding <redacted>

Commit 27a4c827c34a ("fbcon: use the cursor blink interval provided by
vt") unconditionally removes the cursor blink timer. Unfortunately that
wreaks havoc under some circumstances. An easily reproducible way is to
use both the framebuffer console and a debug serial port as the console
output for kernel messages (e.g. "console=ttyS0 console=tty1" on the
kernel command-line. Upon boot this triggers a warning from within the
del_timer_sync() function because it is called from IRQ context:

	[    5.070096] ------------[ cut here ]------------
	[    5.070110] WARNING: CPU: 0 PID: 0 at ../kernel/time/timer.c:1098 del_timer_sync+0x4c/0x54()
	[    5.070115] Modules linked in:
	[    5.070120] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150519 #1
	[    5.070123] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
	[    5.070142] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
	[    5.070156] [] (show_stack) from [] (dump_stack+0x70/0xbc)
	[    5.070164] [] (dump_stack) from [] (warn_slowpath_common+0x74/0xb0)
	[    5.070169] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24)
	[    5.070174] [] (warn_slowpath_null) from [] (del_timer_sync+0x4c/0x54)
	[    5.070183] [] (del_timer_sync) from [] (fbcon_del_cursor_timer+0x2c/0x40)
	[    5.070190] [] (fbcon_del_cursor_timer) from [] (fbcon_cursor+0x9c/0x180)
	[    5.070198] [] (fbcon_cursor) from [] (hide_cursor+0x30/0x98)
	[    5.070204] [] (hide_cursor) from [] (vt_console_print+0x2a8/0x340)
	[    5.070212] [] (vt_console_print) from [] (call_console_drivers.constprop.23+0xc8/0xec)
	[    5.070218] [] (call_console_drivers.constprop.23) from [] (console_unlock+0x498/0x4f0)
	[    5.070223] [] (console_unlock) from [] (vprintk_emit+0x1f0/0x508)
	[    5.070228] [] (vprintk_emit) from [] (vprintk_default+0x24/0x2c)
	[    5.070234] [] (vprintk_default) from [] (printk+0x70/0x88)

After which the system starts spewing all kinds of weird and seemingly
unrelated error messages.

This commit fixes this by restoring the condition under which the call
to fbcon_del_cursor_timer() happens.

Reported-by: Daniel Stone <redacted>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <redacted>
Tested-by: Scot Doyle <redacted>

5 more lines

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 		return;
 
 	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-	fbcon_del_cursor_timer(info);
-	if (!(vc->vc_cursor_type & 0x10))
+	if (vc->vc_cursor_type & 0x10)
+		fbcon_del_cursor_timer(info);
+	else
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode = CM_ERASE) ? 0 : 1;
-- 
2.4.1

Andrey Wagin replied to Thierry Reding

Message details and related content
Open message details and related content
2015-05-21 10:58 GMT+03:00 Thierry Reding [off-list ref]:
From: Thierry Reding <redacted>

Commit 27a4c827c34a ("fbcon: use the cursor blink interval provided by
vt") unconditionally removes the cursor blink timer. Unfortunately that
wreaks havoc under some circumstances. An easily reproducible way is to
use both the framebuffer console and a debug serial port as the console
output for kernel messages (e.g. "console=ttyS0 console=tty1" on the
kernel command-line. Upon boot this triggers a warning from within the
del_timer_sync() function because it is called from IRQ context:

        [    5.070096] ------------[ cut here ]------------
        [    5.070110] WARNING: CPU: 0 PID: 0 at ../kernel/time/timer.c:1098 del_timer_sync+0x4c/0x54()
        [    5.070115] Modules linked in:
        [    5.070120] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150519 #1
        [    5.070123] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
        [    5.070142] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
        [    5.070156] [] (show_stack) from [] (dump_stack+0x70/0xbc)
        [    5.070164] [] (dump_stack) from [] (warn_slowpath_common+0x74/0xb0)
        [    5.070169] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24)
        [    5.070174] [] (warn_slowpath_null) from [] (del_timer_sync+0x4c/0x54)
        [    5.070183] [] (del_timer_sync) from [] (fbcon_del_cursor_timer+0x2c/0x40)
        [    5.070190] [] (fbcon_del_cursor_timer) from [] (fbcon_cursor+0x9c/0x180)
        [    5.070198] [] (fbcon_cursor) from [] (hide_cursor+0x30/0x98)
        [    5.070204] [] (hide_cursor) from [] (vt_console_print+0x2a8/0x340)
        [    5.070212] [] (vt_console_print) from [] (call_console_drivers.constprop.23+0xc8/0xec)
        [    5.070218] [] (call_console_drivers.constprop.23) from [] (console_unlock+0x498/0x4f0)
        [    5.070223] [] (console_unlock) from [] (vprintk_emit+0x1f0/0x508)
        [    5.070228] [] (vprintk_emit) from [] (vprintk_default+0x24/0x2c)
        [    5.070234] [] (vprintk_default) from [] (printk+0x70/0x88)

After which the system starts spewing all kinds of weird and seemingly
unrelated error messages.

This commit fixes this by restoring the condition under which the call
to fbcon_del_cursor_timer() happens.
Tested-by: Andrew Vagin <redacted>

It would be good to push this patch into linux-next asap. Without this
patch I can't execute tests on linux-next. Thanks.
quoted hunk
Reported-by: Daniel Stone <redacted>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <redacted>
Tested-by: Scot Doyle <redacted>
Signed-off-by: Thierry Reding <redacted>
---
 drivers/video/console/fbcon.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
                return;

        ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-       fbcon_del_cursor_timer(info);
-       if (!(vc->vc_cursor_type & 0x10))
+       if (vc->vc_cursor_type & 0x10)
+               fbcon_del_cursor_timer(info);
+       else
                fbcon_add_cursor_timer(info);

        ops->cursor_flash = (mode = CM_ERASE) ? 0 : 1;
--
2.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Tomeu Vizoso replied to Andrey Wagin

Message details and related content
Open message details and related content
On 27 May 2015 at 13:19, Andrey Wagin [off-list ref] wrote:
2015-05-21 10:58 GMT+03:00 Thierry Reding [off-list ref]:
quoted
From: Thierry Reding <redacted>

Commit 27a4c827c34a ("fbcon: use the cursor blink interval provided by
vt") unconditionally removes the cursor blink timer. Unfortunately that
wreaks havoc under some circumstances. An easily reproducible way is to
use both the framebuffer console and a debug serial port as the console
output for kernel messages (e.g. "console=ttyS0 console=tty1" on the
kernel command-line. Upon boot this triggers a warning from within the
del_timer_sync() function because it is called from IRQ context:

        [    5.070096] ------------[ cut here ]------------
        [    5.070110] WARNING: CPU: 0 PID: 0 at ../kernel/time/timer.c:1098 del_timer_sync+0x4c/0x54()
        [    5.070115] Modules linked in:
        [    5.070120] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150519 #1
        [    5.070123] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
        [    5.070142] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
        [    5.070156] [] (show_stack) from [] (dump_stack+0x70/0xbc)
        [    5.070164] [] (dump_stack) from [] (warn_slowpath_common+0x74/0xb0)
        [    5.070169] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24)
        [    5.070174] [] (warn_slowpath_null) from [] (del_timer_sync+0x4c/0x54)
        [    5.070183] [] (del_timer_sync) from [] (fbcon_del_cursor_timer+0x2c/0x40)
        [    5.070190] [] (fbcon_del_cursor_timer) from [] (fbcon_cursor+0x9c/0x180)
        [    5.070198] [] (fbcon_cursor) from [] (hide_cursor+0x30/0x98)
        [    5.070204] [] (hide_cursor) from [] (vt_console_print+0x2a8/0x340)
        [    5.070212] [] (vt_console_print) from [] (call_console_drivers.constprop.23+0xc8/0xec)
        [    5.070218] [] (call_console_drivers.constprop.23) from [] (console_unlock+0x498/0x4f0)
        [    5.070223] [] (console_unlock) from [] (vprintk_emit+0x1f0/0x508)
        [    5.070228] [] (vprintk_emit) from [] (vprintk_default+0x24/0x2c)
        [    5.070234] [] (vprintk_default) from [] (printk+0x70/0x88)

After which the system starts spewing all kinds of weird and seemingly
unrelated error messages.

This commit fixes this by restoring the condition under which the call
to fbcon_del_cursor_timer() happens.
Tested-by: Andrew Vagin <redacted>

It would be good to push this patch into linux-next asap. Without this

1 more quoted lines

Tested-by: Tomeu Vizoso <redacted>

Same here, please apply.

Thanks,

Tomeu
quoted
Reported-by: Daniel Stone <redacted>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <redacted>
Tested-by: Scot Doyle <redacted>
Signed-off-by: Thierry Reding <redacted>
---
 drivers/video/console/fbcon.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
                return;

        ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-       fbcon_del_cursor_timer(info);
-       if (!(vc->vc_cursor_type & 0x10))
+       if (vc->vc_cursor_type & 0x10)
+               fbcon_del_cursor_timer(info);
+       else
                fbcon_add_cursor_timer(info);

        ops->cursor_flash = (mode = CM_ERASE) ? 0 : 1;
--
2.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Thierry Reding replied to Thierry Reding

Message details and related content
Open message details and related content
On Thu, May 21, 2015 at 09:58:03AM +0200, Thierry Reding wrote:
From: Thierry Reding <redacted>

Commit 27a4c827c34a ("fbcon: use the cursor blink interval provided by
vt") unconditionally removes the cursor blink timer. Unfortunately that
wreaks havoc under some circumstances. An easily reproducible way is to
use both the framebuffer console and a debug serial port as the console
output for kernel messages (e.g. "console=ttyS0 console=tty1" on the
kernel command-line. Upon boot this triggers a warning from within the
del_timer_sync() function because it is called from IRQ context:

	[    5.070096] ------------[ cut here ]------------
	[    5.070110] WARNING: CPU: 0 PID: 0 at ../kernel/time/timer.c:1098 del_timer_sync+0x4c/0x54()
	[    5.070115] Modules linked in:
	[    5.070120] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150519 #1
	[    5.070123] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
	[    5.070142] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
	[    5.070156] [] (show_stack) from [] (dump_stack+0x70/0xbc)
	[    5.070164] [] (dump_stack) from [] (warn_slowpath_common+0x74/0xb0)
	[    5.070169] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24)
	[    5.070174] [] (warn_slowpath_null) from [] (del_timer_sync+0x4c/0x54)
	[    5.070183] [] (del_timer_sync) from [] (fbcon_del_cursor_timer+0x2c/0x40)
	[    5.070190] [] (fbcon_del_cursor_timer) from [] (fbcon_cursor+0x9c/0x180)
	[    5.070198] [] (fbcon_cursor) from [] (hide_cursor+0x30/0x98)
	[    5.070204] [] (hide_cursor) from [] (vt_console_print+0x2a8/0x340)
	[    5.070212] [] (vt_console_print) from [] (call_console_drivers.constprop.23+0xc8/0xec)
	[    5.070218] [] (call_console_drivers.constprop.23) from [] (console_unlock+0x498/0x4f0)
	[    5.070223] [] (console_unlock) from [] (vprintk_emit+0x1f0/0x508)
	[    5.070228] [] (vprintk_emit) from [] (vprintk_default+0x24/0x2c)
	[    5.070234] [] (vprintk_default) from [] (printk+0x70/0x88)

After which the system starts spewing all kinds of weird and seemingly
unrelated error messages.

This commit fixes this by restoring the condition under which the call
to fbcon_del_cursor_timer() happens.

Reported-by: Daniel Stone <redacted>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <redacted>
Tested-by: Scot Doyle <redacted>

4 more quoted lines

Ping? This is an important fix, can we get this into linux-next sometime
soon?

Thierry
quoted hunk
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 		return;
 
 	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-	fbcon_del_cursor_timer(info);
-	if (!(vc->vc_cursor_type & 0x10))
+	if (vc->vc_cursor_type & 0x10)
+		fbcon_del_cursor_timer(info);
+	else
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
-- 
2.4.1






    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread / next series-diff position

          
kprevious message in thread / previous series-diff position

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help