[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] vhost-user devices work with chardev from different thr
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] vhost-user devices work with chardev from different threads |
Date: |
Tue, 30 Oct 2018 21:12:18 +0400 |
Hi
On Mon, Oct 29, 2018 at 5:35 PM Yury Kotov <address@hidden> wrote:
>
>
>
> 29.10.2018, 09:46, "Marc-André Lureau" <address@hidden>:
> > Hi
> >
> > On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov <address@hidden> wrote:
> >
> >> Hi,
> >>
> >> I examined vhost-user devices and found some chardev using strangeness.
> >>
> >> Is it ok, that vhost-user's set_status do sync chardev io ops from KVM
> >> thread?
> >> It seems that chardev doesn't support working with different threads.
> >>
> >> For example, I think such race is possible (two simultaneous events):
> >> 1) Vhost-user server was killed (main thread will handle G_IO_HUP),
> >> 2) Virtio-pci bus handles guest driver's set_status command.
> >> Some io op from vhost-user backend will fail because of 1).
> >>
> >> So, it is possible to enter tcp_chr_disconnect twice from the main thread
> >> and
> >> from KVM thread.
> >>
> >> Backtrace examples:
> >>
> >> KVM thread (handle set_status of guest):
> >> tcp_chr_disconnect
> >> tcp_chr_write (cc->chr_write)
> >> qemu_chr_write_buffer
> >> qemu_chr_write
> >> qemu_chr_fe_write_all
> >> vhost_user_write
> >> vhost_user_set_mem_table (hdev->vhost_ops->vhost_set_mem_table)
> >> vhost_dev_start
> >> vhost_user_blk_start (or another vhost-user device)
> >> vhost_user_blk_set_status (vdc->set_status)
> >>
> >> Main thread (handle vhost server disconnect):
> >> tcp_chr_disconnect
> >> tcp_chr_hup
> >> g_main_context_dispatch
> >> glib_pollfds_poll
> >> os_host_main_loop_wait
> >> main_loop_wait
> >> main_loop
> >> main
> >>
> >> Is it really a problem or do I misunderstand?
> >
> > I think you are correct, it's a problem. Are you working on a
> > solution? (either using the chardev lock, or perhaps relying on main
> > thread HUP handler only, or a combination of both approaches)
>
> So, chardev is thread safe by design? I thought it is vhost's problem...
> Anyway I plan to fix that.
chardev is not thread-safe in general, only the write path (assuming
everything else, like lifecycle, is done safely by the user).
>
> >
> > thanks
> >
> > --
> > Marc-André Lureau
--
Marc-André Lureau