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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <da1d1533798a6bdb9930f6d59201aeab@cl.cam.ac.uk>
Date:	Tue, 18 Jul 2006 11:31:42 +0100
From:	Keir Fraser <Keir.Fraser@...cam.ac.uk>
To:	Arjan van de Ven <arjan@...radead.org>
Cc:	virtualization@...ts.osdl.org,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	xen-devel@...ts.xensource.com, Ian Pratt <ian.pratt@...source.com>,
	linux-kernel@...r.kernel.org, Chris Wright <chrisw@...s-sol.org>,
	Andrew Morton <akpm@...l.org>
Subject: Re: [RFC PATCH 27/33] Add the Xen virtual console driver.


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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/