* [Qemu-devel] vhost-user devices work with chardev from different threads
@ 2018-10-22 13:22 Yury Kotov
2018-10-29 6:46 ` Marc-André Lureau
0 siblings, 1 reply; 5+ messages in thread
From: Yury Kotov @ 2018-10-22 13:22 UTC (permalink / raw
To: Paolo Bonzini, Marc-André Lureau, Michael S. Tsirkin,
qemu-devel
Cc: Евгений Яковлев
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?
Thanks,
Yury
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] vhost-user devices work with chardev from different threads
2018-10-22 13:22 [Qemu-devel] vhost-user devices work with chardev from different threads Yury Kotov
@ 2018-10-29 6:46 ` Marc-André Lureau
2018-10-29 13:35 ` Yury Kotov
0 siblings, 1 reply; 5+ messages in thread
From: Marc-André Lureau @ 2018-10-29 6:46 UTC (permalink / raw
To: yury-kotov; +Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU, wrfsh
Hi
On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov <yury-kotov@yandex-team.ru> 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)
thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] vhost-user devices work with chardev from different threads
2018-10-29 6:46 ` Marc-André Lureau
@ 2018-10-29 13:35 ` Yury Kotov
2018-10-30 17:12 ` Marc-André Lureau
0 siblings, 1 reply; 5+ messages in thread
From: Yury Kotov @ 2018-10-29 13:35 UTC (permalink / raw
To: Marc-André Lureau
Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU, wrfsh@yandex-team.ru
29.10.2018, 09:46, "Marc-André Lureau" <marcandre.lureau@gmail.com>:
> Hi
>
> On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov <yury-kotov@yandex-team.ru> 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.
>
> thanks
>
> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] vhost-user devices work with chardev from different threads
2018-10-29 13:35 ` Yury Kotov
@ 2018-10-30 17:12 ` Marc-André Lureau
2018-11-07 17:03 ` Yury Kotov
0 siblings, 1 reply; 5+ messages in thread
From: Marc-André Lureau @ 2018-10-30 17:12 UTC (permalink / raw
To: yury-kotov; +Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU, wrfsh
Hi
On Mon, Oct 29, 2018 at 5:35 PM Yury Kotov <yury-kotov@yandex-team.ru> wrote:
>
>
>
> 29.10.2018, 09:46, "Marc-André Lureau" <marcandre.lureau@gmail.com>:
> > Hi
> >
> > On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov <yury-kotov@yandex-team.ru> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] vhost-user devices work with chardev from different threads
2018-10-30 17:12 ` Marc-André Lureau
@ 2018-11-07 17:03 ` Yury Kotov
0 siblings, 0 replies; 5+ messages in thread
From: Yury Kotov @ 2018-11-07 17:03 UTC (permalink / raw
To: Marc-André Lureau
Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU, wrfsh@yandex-team.ru
Hi,
I was wrong... Described problem with vhost-user is not possible.
It cannot enter tcp_chr_disconnect twice because of qemu global mutex (BQL).
Thus chardev event handler (main loop) and vhost_user_blk_set_status (VCPU) are
working under BQL.
30.10.2018, 20:12, "Marc-André Lureau" <marcandre.lureau@gmail.com>:
> Hi
>
> On Mon, Oct 29, 2018 at 5:35 PM Yury Kotov <yury-kotov@yandex-team.ru> wrote:
>> 29.10.2018, 09:46, "Marc-André Lureau" <marcandre.lureau@gmail.com>:
>> > Hi
>> >
>> > On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov <yury-kotov@yandex-team.ru> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-07 17:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-22 13:22 [Qemu-devel] vhost-user devices work with chardev from different threads Yury Kotov
2018-10-29 6:46 ` Marc-André Lureau
2018-10-29 13:35 ` Yury Kotov
2018-10-30 17:12 ` Marc-André Lureau
2018-11-07 17:03 ` Yury Kotov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.