On Tue, 2020-09-29 at 14:04 -0400, John Snow wrote:
> On 9/29/20 9:42 AM, Elena Afanasova wrote:
> > Signed-off-by: Elena Afanasova <eafanas...@gmail.com>
>
> Hi, can I have a commit message here, please?
>
> > ---
> > job.c | 46 +++++++++++++++++-----------------------------
> > 1 file changed, 17 insertions(+), 29 deletions(-)
> >
> > diff --git a/job.c b/job.c
> > index 8fecf38960..89ceb53434 100644
> > --- a/job.c
> > +++ b/job.c
> > @@ -79,16 +79,6 @@ struct JobTxn {
> > * job_enter. */
> > static QemuMutex job_mutex;
> >
> > -static void job_lock(void)
> > -{
> > - qemu_mutex_lock(&job_mutex);
> > -}
> > -
> > -static void job_unlock(void)
> > -{
> > - qemu_mutex_unlock(&job_mutex);
> > -}
> > -
> > static void __attribute__((__constructor__)) job_init(void)
> > {
> > qemu_mutex_init(&job_mutex);
> > @@ -437,21 +427,19 @@ void job_enter_cond(Job *job, bool(*fn)(Job
> > *job))
> > return;
> > }
> >
> > - job_lock();
> > - if (job->busy) {
> > - job_unlock();
> > - return;
> > - }
> > + WITH_QEMU_LOCK_GUARD(&job_mutex) {
> > + if (job->busy) {
> > + return;
> > + }
> >
> > - if (fn && !fn(job)) {
> > - job_unlock();
> > - return;
> > - }
> > + if (fn && !fn(job)) {
> > + return;
> > + }
> >
> > - assert(!job->deferred_to_main_loop);
> > - timer_del(&job->sleep_timer);
> > - job->busy = true;
> > - job_unlock();
> > + assert(!job->deferred_to_main_loop);
> > + timer_del(&job->sleep_timer);
> > + job->busy = true;
> > + }
> > aio_co_enter(job->aio_context, job->co);
> > }
> >
> > @@ -468,13 +456,13 @@ void job_enter(Job *job)
> > * called explicitly. */
> > static void coroutine_fn job_do_yield(Job *job, uint64_t ns)
> > {
> > - job_lock();
> > - if (ns != -1) {
> > - timer_mod(&job->sleep_timer, ns);
> > + WITH_QEMU_LOCK_GUARD(&job_mutex) {
> > + if (ns != -1) {
> > + timer_mod(&job->sleep_timer, ns);
> > + }
> > + job->busy = false;
> > + job_event_idle(job);
>
> Is this new macro safe to use in a coroutine context?
Hi, I suppose it's safe. It would be nice to get some more opinions
here.
> > }
> > - job->busy = false;
> > - job_event_idle(job);
> > - job_unlock();
> > qemu_coroutine_yield();
> >
> > /* Set by job_enter_cond() before re-entering the
> > coroutine. */
> >
>
> I haven't looked into WITH_QEMU_LOCK_GUARD before, I assume it's new.
> If
> it works like I think it does, this change seems good.
>
> (I'm assuming it works like a Python context manager and it drops
> the
> lock when it leaves the scope of the macro using GCC/Clang language
> extensions.)
>
