Skip to contentSkip to conversation

Avoid a race in complete_change_console()

PATCH In review Chris Ahna 2 participants 2 messages Oct 26, 2001 Char and misc drivers

lore
Share & tools
lore

Chris Ahna

author
#0 lore raw

PATCH 1/1 Avoid a race in complete_change_console()

Currently drivers/char/vt.c:complete_change_console() calls do_blank_screen() when switching from text to graphics mode after sending acqsig to the controlling process of the new console.

On an SMP system, acqsig can wake the controlling process up and get it working before do_blank_screen completes. At least on my ia64 systems, this causes accesses to the VGA range from the kernel and accesses to framebuffer from X to overlap with one another causing system lockups.

Appended is a patch which ensures that do_blank_screen() is called before delivering acqsig instead of after. do_blank_screen() is still called if signal delivery fails (as far as I could tell, handling this case was the reason screen blanking was done after signal delivery in the first place).

Can someone familiar with this code comment on the correctness of this patch? The patch is against vanilla 2.4.13. Thanks,

Chris

drivers/char/vt.c +26 -12
@@ -1184,6 +1184,24 @@
 	switch_screen(new_console);
 
 	/*
+	 * This can't appear below a successful kill_proc().  If it did,
+	 * then the *blank_screen operation could occur while X, having
+	 * received acqsig, is waking up on another processor.  This
+	 * condition can lead to overlapping accesses to the VGA range
+	 * and the framebuffer (causing system lockups).
+	 *
+	 * To account for this we duplicate this code below only if the
+	 * controlling process is gone and we've called reset_vc.
+	 */
+	if (old_vc_mode != vt_cons[new_console]->vc_mode)
+	{
+		if (vt_cons[new_console]->vc_mode == KD_TEXT)
+			unblank_screen();
+		else
+			do_blank_screen(1);
+	}
+
+	/*
 	 * If this new console is under process control, send it a signal
 	 * telling it that it has acquired. Also check if it has died and
 	 * clean up (similar to logic employed in change_console())
@@ -1209,19 +1227,15 @@
 		 * to account for and tracking tty count may be undesirable.
 		 */
 		        reset_vc(new_console);
-		}
-	}
 
-	/*
-	 * We do this here because the controlling process above may have
-	 * gone, and so there is now a new vc_mode
-	 */
-	if (old_vc_mode != vt_cons[new_console]->vc_mode)
-	{
-		if (vt_cons[new_console]->vc_mode == KD_TEXT)
-			unblank_screen();
-		else
-			do_blank_screen(1);
+			if (old_vc_mode != vt_cons[new_console]->vc_mode)
+			{
+				if (vt_cons[new_console]->vc_mode == KD_TEXT)
+					unblank_screen();
+				else
+					do_blank_screen(1);
+			}
+		}
 	}
 
 	/*
#1 lore raw
Quoted 2 lines · Chris Ahna

Can someone familiar with this code comment on the correctness of this patch? The patch is against vanilla 2.4.13. Thanks,

I'm not very familiar with it, but it seems sane fix. Only detail is that the vc_mode can only be KD_TEXT after the reset_vc but the additional check doesn't hurt and it makes it indipendent by the reset_vc details.

Andrea