From: Li Hao <[email protected]>
Cancel the do_wakeup flag in hvc_struct, and change it to immediately
wake up tty when hp->n_outbuf is 0 in hvc_push().
When we receive a key input character, the interrupt handling function
hvc_handle_interrupt() will be executed, and the echo thread
flush_to_ldisc() will be added to the queue.
If the user is currently using tcsetattr(), a hang may occur. tcsetattr()
enters kernel and waits for hp->n_outbuf to become 0 via
tty_wait_until_sent(). If the echo thread finishes executing before
reaching tty_wait_until_sent (for example, put_chars() takes too long),
it will cause while meeting the wakeup condition (hp->do_wakeup = 1),
tty_wait_until_sent() cannot be woken up (missed the tty_wakeup() of
this round's tty_poll). Unless the next key input character comes,
hvc_poll will be executed, and tty_wakeup() will be performed through
the do_wakeup flag.
Signed-off-by: Li Hao <[email protected]>
---
drivers/tty/hvc/hvc_console.c | 12 +++++-------
drivers/tty/hvc/hvc_console.h | 1 -
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index cd1f657f7..2fa90d938 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -476,11 +476,13 @@ static void hvc_hangup(struct tty_struct *tty)
static int hvc_push(struct hvc_struct *hp)
{
int n;
+ struct tty_struct *tty;
n = hp->ops->put_chars(hp->vtermno, hp->outbuf, hp->n_outbuf);
+ tty = tty_port_tty_get(&hp->port);
if (n <= 0) {
if (n == 0 || n == -EAGAIN) {
- hp->do_wakeup = 1;
+ tty_wakeup(tty);
return 0;
}
/* throw away output on error; this happens when
@@ -491,7 +493,7 @@ static int hvc_push(struct hvc_struct *hp)
if (hp->n_outbuf > 0)
memmove(hp->outbuf, hp->outbuf + n, hp->n_outbuf);
else
- hp->do_wakeup = 1;
+ tty_wakeup(tty);
return n;
}
@@ -739,11 +741,7 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
poll_mask |= HVC_POLL_READ;
out:
- /* Wakeup write queue if necessary */
- if (hp->do_wakeup) {
- hp->do_wakeup = 0;
- tty_wakeup(tty);
- }
+ /* Wakeup in hvc_push */
bail:
spin_unlock_irqrestore(&hp->lock, flags);
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index cf4c1af08..6622f71ba 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -36,7 +36,6 @@ struct hvc_struct {
struct tty_port port;
spinlock_t lock;
int index;
- int do_wakeup;
int outbuf_size;
int n_outbuf;
uint32_t vtermno;
--
2.25.1
On Fri, Apr 12, 2024 at 11:38:48AM +0800, [email protected] wrote:
> From: Li Hao <[email protected]>
>
> Cancel the do_wakeup flag in hvc_struct, and change it to immediately
> wake up tty when hp->n_outbuf is 0 in hvc_push().
>
> When we receive a key input character, the interrupt handling function
> hvc_handle_interrupt() will be executed, and the echo thread
> flush_to_ldisc() will be added to the queue.
>
> If the user is currently using tcsetattr(), a hang may occur. tcsetattr()
> enters kernel and waits for hp->n_outbuf to become 0 via
> tty_wait_until_sent(). If the echo thread finishes executing before
> reaching tty_wait_until_sent (for example, put_chars() takes too long),
> it will cause while meeting the wakeup condition (hp->do_wakeup = 1),
> tty_wait_until_sent() cannot be woken up (missed the tty_wakeup() of
> this round's tty_poll). Unless the next key input character comes,
> hvc_poll will be executed, and tty_wakeup() will be performed through
> the do_wakeup flag.
>
> Signed-off-by: Li Hao <[email protected]>
> ---
> drivers/tty/hvc/hvc_console.c | 12 +++++-------
> drivers/tty/hvc/hvc_console.h | 1 -
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index cd1f657f7..2fa90d938 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -476,11 +476,13 @@ static void hvc_hangup(struct tty_struct *tty)
> static int hvc_push(struct hvc_struct *hp)
> {
> int n;
> + struct tty_struct *tty;
>
> n = hp->ops->put_chars(hp->vtermno, hp->outbuf, hp->n_outbuf);
> + tty = tty_port_tty_get(&hp->port);
> if (n <= 0) {
> if (n == 0 || n == -EAGAIN) {
> - hp->do_wakeup = 1;
> + tty_wakeup(tty);
> return 0;
> }
> /* throw away output on error; this happens when
> @@ -491,7 +493,7 @@ static int hvc_push(struct hvc_struct *hp)
> if (hp->n_outbuf > 0)
> memmove(hp->outbuf, hp->outbuf + n, hp->n_outbuf);
> else
> - hp->do_wakeup = 1;
> + tty_wakeup(tty);
>
> return n;
> }
> @@ -739,11 +741,7 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
> poll_mask |= HVC_POLL_READ;
>
> out:
> - /* Wakeup write queue if necessary */
> - if (hp->do_wakeup) {
> - hp->do_wakeup = 0;
> - tty_wakeup(tty);
> - }
> + /* Wakeup in hvc_push */
> bail:
> spin_unlock_irqrestore(&hp->lock, flags);
>
> diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
> index cf4c1af08..6622f71ba 100644
> --- a/drivers/tty/hvc/hvc_console.h
> +++ b/drivers/tty/hvc/hvc_console.h
> @@ -36,7 +36,6 @@ struct hvc_struct {
> struct tty_port port;
> spinlock_t lock;
> int index;
> - int do_wakeup;
> int outbuf_size;
> int n_outbuf;
> uint32_t vtermno;
> --
> 2.25.1
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
On 12. 04. 24, 5:38, [email protected] wrote:
> From: Li Hao <[email protected]>
>
> Cancel the do_wakeup flag in hvc_struct, and change it to immediately
> wake up tty when hp->n_outbuf is 0 in hvc_push().
>
> When we receive a key input character, the interrupt handling function
> hvc_handle_interrupt() will be executed, and the echo thread
> flush_to_ldisc() will be added to the queue.
>
> If the user is currently using tcsetattr(), a hang may occur. tcsetattr()
> enters kernel and waits for hp->n_outbuf to become 0 via
> tty_wait_until_sent(). If the echo thread finishes executing before
> reaching tty_wait_until_sent (for example, put_chars() takes too long),
> it will cause while meeting the wakeup condition (hp->do_wakeup = 1),
> tty_wait_until_sent() cannot be woken up (missed the tty_wakeup() of
> this round's tty_poll). Unless the next key input character comes,
> hvc_poll will be executed, and tty_wakeup() will be performed through
> the do_wakeup flag.
>
> Signed-off-by: Li Hao <[email protected]>
> ---
> drivers/tty/hvc/hvc_console.c | 12 +++++-------
> drivers/tty/hvc/hvc_console.h | 1 -
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index cd1f657f7..2fa90d938 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -476,11 +476,13 @@ static void hvc_hangup(struct tty_struct *tty)
> static int hvc_push(struct hvc_struct *hp)
> {
> int n;
> + struct tty_struct *tty;
>
> n = hp->ops->put_chars(hp->vtermno, hp->outbuf, hp->n_outbuf);
> + tty = tty_port_tty_get(&hp->port);
> if (n <= 0) {
> if (n == 0 || n == -EAGAIN) {
> - hp->do_wakeup = 1;
> + tty_wakeup(tty);
What if tty is NULL? Did you intent to use tty_port_tty_wakeup() instead?
thanks,
--
js
suse labs
> On 12. 04. 24, 5:38, [email protected] wrote:
> > From: Li Hao <[email protected]>
> >
> > Cancel the do_wakeup flag in hvc_struct, and change it to immediately
> > wake up tty when hp->n_outbuf is 0 in hvc_push().
> >
> > When we receive a key input character, the interrupt handling function
> > hvc_handle_interrupt() will be executed, and the echo thread
> > flush_to_ldisc() will be added to the queue.
> >
> > If the user is currently using tcsetattr(), a hang may occur. tcsetattr()
> > enters kernel and waits for hp->n_outbuf to become 0 via
> > tty_wait_until_sent(). If the echo thread finishes executing before
> > reaching tty_wait_until_sent (for example, put_chars() takes too long),
> > it will cause while meeting the wakeup condition (hp->do_wakeup = 1),
> > tty_wait_until_sent() cannot be woken up (missed the tty_wakeup() of
> > this round's tty_poll). Unless the next key input character comes,
> > hvc_poll will be executed, and tty_wakeup() will be performed through
> > the do_wakeup flag.
> >
> > Signed-off-by: Li Hao <[email protected]>
> > ---
> > drivers/tty/hvc/hvc_console.c | 12 +++++-------
> > drivers/tty/hvc/hvc_console.h | 1 -
> > 2 files changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> > index cd1f657f7..2fa90d938 100644
> > --- a/drivers/tty/hvc/hvc_console.c
> > +++ b/drivers/tty/hvc/hvc_console.c
> > @@ -476,11 +476,13 @@ static void hvc_hangup(struct tty_struct *tty)
> > static int hvc_push(struct hvc_struct *hp)
> > {
> > int n;
> > + struct tty_struct *tty;
> >
> > n = hp->ops->put_chars(hp->vtermno, hp->outbuf, hp->n_outbuf);
> > + tty = tty_port_tty_get(&hp->port);
> > if (n <= 0) {
> > if (n == 0 || n == -EAGAIN) {
> > - hp->do_wakeup = 1;
> > + tty_wakeup(tty);
>
> What if tty is NULL? Did you intent to use tty_port_tty_wakeup() instead?
>
> thanks,
> --
> js
> suse labs
Thank you for your prompt reply.
tty_port_tty_wakeup() is better, it no longer check if tty is NULL in hvc_push()
Li Hao