Messages in this thread Patches in this message | | | Date | Fri, 11 Jun 1999 12:52:30 +0200 | From | Petr Vandrovec <> | Subject | reentrancy of console drivers - source of problems found (?) |
| |
Hi, on Monday I announced that there is reentrancy bug in console subsystem - - putc procedure gets reentered. After some investigation I found that this comes from problem that '*_cursor' can now cause screen redraw after JJ scrollback patches. So I applied this fix:
diff -urN linux/drivers/char/console.c linux/drivers/char/console.c --- linux/drivers/char/console.c Tue May 11 23:37:40 1999 +++ linux/drivers/char/console.c Thu Jun 10 22:45:32 1999 @@ -503,6 +503,9 @@ { if (!IS_FG || console_blanked || vcmode == KD_GRAPHICS) return; + + disable_bh(CONSOLE_BH); + if (deccm) { if (currcons == sel_cons) clear_selection(); @@ -511,6 +514,8 @@ sw->con_cursor(vc_cons[currcons].d,CM_DRAW); } else hide_cursor(currcons); + + enable_bh(CONSOLE_BH); } static void set_origin(int currcons) @@ -1809,11 +1814,12 @@ himask = hi_font_mask; charmask = himask ? 0x1ff : 0xff; + disable_bh(CONSOLE_BH); + /* undraw cursor first */ if (IS_FG) hide_cursor(currcons); - disable_bh(CONSOLE_BH); while (!tty->stopped && count) { enable_bh(CONSOLE_BH); if (from_user) \ End over my 2.3.5 kernel... After that I was not able to cause reentrancy of matroxfb - but I cannot say that drivers/char/console.c is now safe - there is couple of calls to *_cursor, some of them are guarded by different console locks and by disable_bh, some of them maybe are not - Martin, could you verify that all calls to hide_cursor() are guarded by something on upper level?
When I was debugging it, I got one strange reentrancy:
CPU: 0 EIP: 0010:[printstate+9/48] EFLAGS: 00000202 eax: 00000001 ebx: 00000001 ecx: 00000008 edx: 0000007f esi: 00000000 edi: 00000010 ebp: 00000010 esp: c4ed5c3c ds: 0018 es: 0018 ss: 0018 Process x (pid: 259, process nr: 27, stackpage=c4ed5000) Stack: c01a6de1 c01a8049 c0231d00 00aaaaaa c7fe0000 c0008000 00000010 00000050 00000008 c0008000 00000001 00160000 0000007f 00000010 00500010 00000001 c01a83d3 00aaaaaa 00000000 c0231d00 c7fe0000 0000000a 00000005 00000000 Call Trace: [guard_start+33/48] [matrox_cfbX_putcs+201/720] [matrox_cfb32_putcs+99/112] [fbcon_redraw_softback+503/720] [fbcon_scrolldelta+355/704] [do_8259A_IRQ+141/208] [console_bh+246/272] [do_bottom_half+153/192] [do_IRQ+82/96] [ret_from_intr+0/32] [vfat:vfat_lookup+-8754800/288] [vfat:vfat_lookup+-8754800/288] [vfat:vfat_lookup+-8754776/288] [matrox_cfbX_putc+272/880] [matrox_cfb32_putc+90/112] [fbcon_putc+192/224] [add_softcursor+198/208] [set_cursor+79/128] [con_flush_chars+18/32] [con_write+35/48] [opost_block+395/416] [console_bh+186/272] [do_bottom_half+153/192] [do_IRQ+82/96] [set_cursor+79/128] [write_chan+604/880] [tty_write+550/752] [write_chan+0/880] [sys_write+315/432] [tty_write+0/752] [system_call+52/64] Code: 50 1e 06 50 55 57 56 52 51 53 89 e0 50 e8 65 fd ff ff 83 c4 I see console_bh TWICE in this list - but it was UNIPROCESSOR machine (with SMP kernel), so I cannot imagine, what could cause that. After I added disable_bh() to set_cursor, I cannot repeat it. I added couple of checks (cpu_bh_endlock test whether local_cpu_count == 1, check whether smp_processor_id() returns always 0 on my uni box...) but they were not trigerred, so I do not know, what happened.
I'm sure that patch is correct, but maybe that it is not complete. It should be applied to both 2.3.6 and 2.2.10, I think. Best regards, Petr Vandrovec vandrove@vc.cvut.cz
diff -urN linux/drivers/char/console.c linux/drivers/char/console.c --- linux/drivers/char/console.c Tue May 11 23:37:40 1999 +++ linux/drivers/char/console.c Thu Jun 10 22:45:32 1999 @@ -503,6 +503,9 @@ { if (!IS_FG || console_blanked || vcmode == KD_GRAPHICS) return; + + disable_bh(CONSOLE_BH); + if (deccm) { if (currcons == sel_cons) clear_selection(); @@ -511,6 +514,8 @@ sw->con_cursor(vc_cons[currcons].d,CM_DRAW); } else hide_cursor(currcons); + + enable_bh(CONSOLE_BH); } static void set_origin(int currcons) @@ -1809,11 +1814,12 @@ himask = hi_font_mask; charmask = himask ? 0x1ff : 0xff; + disable_bh(CONSOLE_BH); + /* undraw cursor first */ if (IS_FG) hide_cursor(currcons); - disable_bh(CONSOLE_BH); while (!tty->stopped && count) { enable_bh(CONSOLE_BH); if (from_user) | |