[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] chardev: report blocked write to chardev backend
From: |
Nicholas Piggin |
Subject: |
Re: [PATCH 2/3] chardev: report blocked write to chardev backend |
Date: |
Mon, 20 Nov 2023 23:35:50 +1000 |
On Mon Nov 20, 2023 at 10:06 PM AEST, Marc-André Lureau wrote:
> Hi
>
> On Thu, Nov 16, 2023 at 3:54 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> >
> > If a chardev socket is not read, it will eventually fill and QEMU
> > can block attempting to write to it. A difficult bug in avocado
> > tests where the console socket was not being read from caused this
> > hang.
> >
> > warn if a chardev write is blocked for 100ms.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > This is not necessary for the fix but it does trigger in the
> > failing avocado test without the previous patch applied. Maybe
> > it would be helpful?
> >
> > Thanks,
> > Nick
> >
> > chardev/char.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/chardev/char.c b/chardev/char.c
> > index 996a024c7a..7c375e3cc4 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -114,6 +114,8 @@ static int qemu_chr_write_buffer(Chardev *s,
> > {
> > ChardevClass *cc = CHARDEV_GET_CLASS(s);
> > int res = 0;
> > + int nr_retries = 0;
> > +
> > *offset = 0;
> >
> > qemu_mutex_lock(&s->chr_write_lock);
> > @@ -126,6 +128,10 @@ static int qemu_chr_write_buffer(Chardev *s,
> > } else {
> > g_usleep(100);
> > }
> > + if (++nr_retries == 1000) { /* 100ms */
> > + warn_report("Chardev '%s' write blocked for > 100ms, "
> > + "socket buffer full?", s->label);
> > + }
>
> That shouldn't happen, the frontend should poll and only write when it
> can. What is the qemu command being used here?
You can follow it through the thread here
ZVT-bY9YOr69QTPX@redhat.com/">https://lore.kernel.org/qemu-devel/ZVT-bY9YOr69QTPX@redhat.com/
In short, a console device is attached to a socket pair and nothing
ever reads from it. It eventually fills, and writing to it fails
indefinitely here.
It can be reproduced with:
make check-avocado
AVOCADO_TESTS=tests/avocado/reverse_debugging.py:test_ppc64_pseries
> I think this change can be worth for debugging though.
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Thanks,
Nick
- [PATCH 1/3] tests/avocado: reverse_debugging drain console to prevent hang, Nicholas Piggin, 2023/11/16
- [PATCH 2/3] chardev: report blocked write to chardev backend, Nicholas Piggin, 2023/11/16
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Marc-André Lureau, 2023/11/20
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend,
Nicholas Piggin <=
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Marc-André Lureau, 2023/11/21
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Daniel P . Berrangé, 2023/11/21
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Thomas Huth, 2023/11/21
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Marc-André Lureau, 2023/11/21
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Alex Bennée, 2023/11/22
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Thomas Huth, 2023/11/22
- Re: [PATCH 2/3] chardev: report blocked write to chardev backend, Daniel P . Berrangé, 2023/11/22
[PATCH 3/3] tests/avocado: Enable reverse_debugging.py tests in gitlab CI, Nicholas Piggin, 2023/11/16