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.lur...@gmail.com>: > Hi > > On Mon, Oct 29, 2018 at 5:35 PM Yury Kotov <yury-ko...@yandex-team.ru> wrote: >> 29.10.2018, 09:46, "Marc-André Lureau" <marcandre.lur...@gmail.com>: >> > Hi >> > >> > On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov <yury-ko...@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
