From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Milton Miller <miltonm@bga.com>
Cc: Joe Peterson <joe@skyrush.com>,
lkml <linux-kernel@vger.kernel.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
linuxppc-dev list <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH 1/4] hvc_console: do not set low_latency
Date: Tue, 13 Jan 2009 10:04:22 +0100 [thread overview]
Message-ID: <200901131004.22609.borntraeger@de.ibm.com> (raw)
In-Reply-To: <hvc-console-29-3-1@bga.com>
Am Donnerstag 08 Januar 2009 schrieb Milton Miller:
> hvc_console is setting low_latency unconditionally, but some clients are
> interrupt driven and will call hvc_poll from irq context. This will cause
> tty_flip_buffer_push to be called from irq context, and it very clearly
> states it must not be called from IRQ when low_latency is specified.
>
> Looking back through history:
> v2.6.16-rc1 via 33f0f88f1c51ae5c2d593d26960c760ea154c2e2
> [PATCH] TTY layer buffering revamp
>
> added this new api.
>
> v2.6.16-rc3 via 8977d929e49021d9a6e031310aab01fa72f849c2
> [PATCH] tty buffering stall fix
>
> claims to fix a stall discovered with hvc_console
>
> v2.6.16-rc5 via fb5c594c2acc441f0d2d8f457484a0e0e9285db3
> [PATCH] Fix race condition in hvc console.
>
> said set this flag to avoid a stall problem, and was merged through
> the powerpc arch tree.
>
> Without searching for email discussions, it would appear to be an
> overlapping "fix", but one that did not consider all users.
>
> ---
> This version continues to set low_latency if irqs are not flagged to
> be in use, as requested by paulus. Do all hvc drivers identify the
> interrupt this way? (A quick look at hvc_iucv says they lock to bh
> and are not irq driven, the rest would have used the irq before that
> patch).
>
> Having the flag set for purely polled drivers will save delaying
> the work when receiving input for 1 jiffie.
>
>
> Index: work.git/drivers/char/hvc_console.c
> ===================================================================
> --- work.git.orig/drivers/char/hvc_console.c 2009-01-08
03:01:24.000000000 -0600
> +++ work.git/drivers/char/hvc_console.c 2009-01-08 03:01:51.000000000 -0600
> @@ -318,7 +318,8 @@ static int hvc_open(struct tty_struct *t
> } /* else count == 0 */
>
> tty->driver_data = hp;
> - tty->low_latency = 1; /* Makes flushes to ldisc synchronous. */
> + if (!hp->irq_requested)
> + tty->low_latency = 1; /* Makes flushes to ldisc synchronous. */
>
> hp->tty = tty;
>
>
This wont work, since the call to notifier_add is done later:
What about:
---
drivers/char/hvc_console.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/char/hvc_console.c
===================================================================
--- linux-2.6.orig/drivers/char/hvc_console.c
+++ linux-2.6/drivers/char/hvc_console.c
@@ -318,8 +318,6 @@ static int hvc_open(struct tty_struct *t
} /* else count == 0 */
tty->driver_data = hp;
- tty->low_latency = 1; /* Makes flushes to ldisc synchronous. */
-
hp->tty = tty;
spin_unlock_irqrestore(&hp->lock, flags);
@@ -327,6 +325,9 @@ static int hvc_open(struct tty_struct *t
if (hp->ops->notifier_add)
rc = hp->ops->notifier_add(hp, hp->data);
+ if (!hp->irq_requested)
+ tty->low_latency = 1; /* Makes flushes to ldisc synchronous. */
+
/*
* If the notifier fails we return an error. The tty layer
* will call hvc_close() after a failed open but we don't want to clean
next prev parent reply other threads:[~2009-01-13 9:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-08 5:18 [BUG] hvc_console WARN() on current upstream Benjamin Herrenschmidt
2009-01-08 7:57 ` Christian Borntraeger
2009-01-08 8:42 ` Benjamin Herrenschmidt
2009-01-08 11:11 ` Alan Cox
2009-01-08 12:12 ` [PATCH 0/5] hvc_console updates was " Milton Miller
2009-01-08 12:14 ` [PATCH 4/4] hvc_console: comment mb and make it an smp_ one Milton Miller
2009-01-08 12:14 ` [PATCH 3/4] hvc_console: free_irq only if request_irq was successful Milton Miller
2009-01-08 16:50 ` Christian Borntraeger
2009-01-08 12:14 ` [PATCH 1/4] hvc_console: do not set low_latency Milton Miller
2009-01-08 12:36 ` Alan Cox
2009-01-08 13:25 ` Milton Miller
2009-01-13 9:04 ` Christian Borntraeger [this message]
2009-01-13 11:28 ` Christian Borntraeger
2009-01-13 11:35 ` Hendrik Brueckner
2009-01-13 16:03 ` Milton Miller
2009-01-13 21:04 ` Benjamin Herrenschmidt
2009-01-15 9:15 ` [PATCH 1/4 v2] hvc_console: remove tty->low_latency Hendrik Brueckner
2009-01-15 9:17 ` Christian Borntraeger
2009-01-08 12:14 ` [PATCH 2/4] hvc_console: use kzalloc Milton Miller
2009-01-08 20:36 ` [BUG] hvc_console WARN() on current upstream Benjamin Herrenschmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200901131004.22609.borntraeger@de.ibm.com \
--to=borntraeger@de.ibm.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=joe@skyrush.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=miltonm@bga.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).