Re: [PATCH] job: delete job_{lock, unlock} functions and replace them with lock guard

[John Snow]

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?

      }
-    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.)