| Message ID | 20240412113848167egmP7kBg1Qm5sxfwGALG-@zte.com.cn (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | tty: hvc: wakeup hvc console immediately when needed | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
On Fri, Apr 12, 2024 at 11:38:48AM +0800, li.hao40@zte.com.cn wrote: > From: Li Hao <li.hao40@zte.com.cn> > > 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 <li.hao40@zte.com.cn> > --- > 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, li.hao40@zte.com.cn wrote: > From: Li Hao <li.hao40@zte.com.cn> > > 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 <li.hao40@zte.com.cn> > --- > 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,
> On 12. 04. 24, 5:38, li.hao40@zte.com.cn wrote: > > From: Li Hao <li.hao40@zte.com.cn> > > > > 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 <li.hao40@zte.com.cn> > > --- > > 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
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;