Re: [RFC PATCH 27/33] Add the Xen virtual console driver.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 18 Jul 2006, at 11:24, Arjan van de Ven wrote:

hmm somehow I find this code scary; we had similar code recently
elsewhere where this turned out to be a real issue; you now sleep for
"1" time, so you sleep for a fixed time if you aren't getting wakeups,
but if you are getting wakeups your code is upside down, I would expect
it to look like

+	set_current_state(TASK_INTERRUPTIBLE);
+	while (DRV(tty->driver)->chars_in_buffer(tty))
+		schedule_timeout(1);
+		if (signal_pending(current))
+			break;
+		if (timeout && time_after(jiffies, orig_jiffies + timeout))
+			break;
+		set_current_state(TASK_INTERRUPTIBLE);
+	}

instead, so that you don't have the wakeup race..

There's no wakeup signal, so no possibility of a wakeup race. That's why we schedule_timeout() instead of wait_event() or similar. This code is only used to flush the console when the kernel crashes, so we can get the full oops, so waiting a little bit too long is acceptable.

Your suggested change is perhaps more idiomatic though, and less jarring for reviewers. :-)

Thanks for your comments by the way. Reviewing lots of patches isn't much fun.

 -- Keir

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

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux