On Tue, 2006-07-18 at 00:00 -0700, Chris Wright wrote:
> + } else if (sysrq_requested) {
> + unsigned long sysrq_timeout =
> + sysrq_requested + HZ*2;
> + sysrq_requested = 0;
> + if (time_before(jiffies, sysrq_timeout)) {
> + spin_unlock_irqrestore(
> + &xencons_lock, flags);
> + handle_sysrq(
> + buf[i], regs, xencons_tty);
> + spin_lock_irqsave(
> + &xencons_lock, flags);
> + continue;
> + }
Lindent can be harmful...
> +static void xencons_wait_until_sent(struct tty_struct *tty, int timeout)
> +{
> + unsigned long orig_jiffies = jiffies;
> +
> + if (TTY_INDEX(tty) != 0)
> + return;
> +
> + while (DRV(tty->driver)->chars_in_buffer(tty)) {
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule_timeout(1);
> + if (signal_pending(current))
> + break;
> + if (timeout && time_after(jiffies, orig_jiffies + timeout))
> + break;
> + }
> +
> + set_current_state(TASK_RUNNING);
> +}
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..
-
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]
