Message ID | 20171116130610.23582-16-peterx@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
* Peter Xu (peterx@redhat.com) wrote: > Monitor code now can be run in more than one thread. Let the suspend > and resume code be thread safe. > > Reviewed-by: Fam Zheng <famz@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > monitor.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/monitor.c b/monitor.c > index ec03f1b232..30f9cd80de 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -4003,7 +4003,7 @@ int monitor_suspend(Monitor *mon) > { > if (!mon->rs) > return -ENOTTY; > - mon->suspend_cnt++; > + atomic_inc(&mon->suspend_cnt); > return 0; > } > > @@ -4011,8 +4011,9 @@ void monitor_resume(Monitor *mon) > { > if (!mon->rs) > return; > - if (--mon->suspend_cnt == 0) > + if (atomic_dec_fetch(&mon->suspend_cnt) == 0) { > readline_show_prompt(mon->rs); > + } > } > > static QObject *get_qmp_greeting(void) Do you also need to do an atomic_read() in monitor_can_read? (I guess the code in monitor_event is ok because it's only the mux path, but still it's probably better to use all atomic ops on the suspend_cnt variable to be consistent). Dave > -- > 2.13.6 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Thu, Nov 23, 2017 at 11:23:16AM +0000, Dr. David Alan Gilbert wrote: > * Peter Xu (peterx@redhat.com) wrote: > > Monitor code now can be run in more than one thread. Let the suspend > > and resume code be thread safe. > > > > Reviewed-by: Fam Zheng <famz@redhat.com> > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > monitor.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index ec03f1b232..30f9cd80de 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -4003,7 +4003,7 @@ int monitor_suspend(Monitor *mon) > > { > > if (!mon->rs) > > return -ENOTTY; > > - mon->suspend_cnt++; > > + atomic_inc(&mon->suspend_cnt); > > return 0; > > } > > > > @@ -4011,8 +4011,9 @@ void monitor_resume(Monitor *mon) > > { > > if (!mon->rs) > > return; > > - if (--mon->suspend_cnt == 0) > > + if (atomic_dec_fetch(&mon->suspend_cnt) == 0) { > > readline_show_prompt(mon->rs); > > + } > > } > > > > static QObject *get_qmp_greeting(void) > > Do you also need to do an atomic_read() in monitor_can_read? > (I guess the code in monitor_event is ok because it's only the mux > path, but still it's probably better to use all atomic ops on > the suspend_cnt variable to be consistent). Agreed.
diff --git a/monitor.c b/monitor.c index ec03f1b232..30f9cd80de 100644 --- a/monitor.c +++ b/monitor.c @@ -4003,7 +4003,7 @@ int monitor_suspend(Monitor *mon) { if (!mon->rs) return -ENOTTY; - mon->suspend_cnt++; + atomic_inc(&mon->suspend_cnt); return 0; } @@ -4011,8 +4011,9 @@ void monitor_resume(Monitor *mon) { if (!mon->rs) return; - if (--mon->suspend_cnt == 0) + if (atomic_dec_fetch(&mon->suspend_cnt) == 0) { readline_show_prompt(mon->rs); + } } static QObject *get_qmp_greeting(void)