Hi On Fri, Mar 23, 2018 at 6:50 AM, Peter Xu <pet...@redhat.com> wrote: > On Thu, Mar 22, 2018 at 01:00:19PM +0100, Marc-André Lureau wrote: >> Hi >> >> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu <pet...@redhat.com> wrote: >> > For those monitors who have enabled IO thread, we'll offload the >> > responding procedure into IO thread. The main reason is that chardev is >> > not thread safe, and we need to do all the read/write IOs in the same >> > thread. For use_io_thr=true monitors, that thread is the IO thread. >> >> Actually, the chr write path is suppose to be somewhat thread safe >> (chr_write_lock). >> >> Secondly, the function responsible to write monitor data has some >> thread-safety, it's called monitor_flush_locked(), because you need >> the mon->out_lock. >> >> I think that patch is making things more complicated than they need to >> be. You should be able to call monitor_json_emitter/monitor_puts() >> directly from any thread, it will queue the data, start writing, and >> add a watch if necessary in the appropriate context. >> >> Am I missing something? > > Yes there are the locks either in monitor code and chardev code to > protect write operations. However I don't know enough to be sure of > its safety. Considering that, having the whole chardev operations > separated into the other IOThread seems to be the only safe approach > to me for now.
I have studied the code in more details, and I'll propose to revert this patch early in 2.13. I'll send a 2.13 RFC qmp series shortly. -- Marc-André Lureau
