[PATCH v4 0/2] Replaced locks with lock guard macros

[PATCH v4 0/2] Replaced locks with lock guard macros

[dnbrdsky]

From: Daniel Brodsky <dnbrdsky@gmail.com>

This patch set adds:
- a fix for lock guard macros so they can be used multiple times in
the same function
- replacement of locks with lock guards where appropriate

v3 -> v4:
- removed unneeded unlocks from areas where lock guards are now used
- dropped change to lock guard in iscsi.c as it changed old functionality

v2 -> v3:
- added __COUNTER__ fix for additional lock guard macro
- added missing include header in platform.c

v1 -> v2:
- fixed whitespace churn
- added cover letter so patch set referenced correctly

Daniel Brodsky (2):
  lockable: fix __COUNTER__ macro to be referenced properly
  lockable: replaced locks with lock guard macros where appropriate

 block/iscsi.c           |  7 ++----
 block/nfs.c             | 51 +++++++++++++++++++----------------------
 cpus-common.c           | 14 ++++-------
 hw/display/qxl.c        | 43 ++++++++++++++++------------------
 hw/vfio/platform.c      |  5 ++--
 include/qemu/lockable.h |  4 ++--
 include/qemu/rcu.h      |  2 +-
 migration/migration.c   |  3 +--
 migration/multifd.c     |  8 +++----
 migration/ram.c         |  3 +--
 monitor/misc.c          |  4 +---
 ui/spice-display.c      | 14 +++++------
 util/log.c              |  4 ++--
 util/qemu-timer.c       | 17 +++++++-------
 util/rcu.c              |  8 +++----
 util/thread-pool.c      |  3 +--
 util/vfio-helpers.c     |  5 ++--
 17 files changed, 86 insertions(+), 109 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] lockable: fix __COUNTER__ macro to be referenced properly

[dnbrdsky]

From: Daniel Brodsky <dnbrdsky@gmail.com>

- __COUNTER__ doesn't work with ## concat
- replaced ## with glue() macro so __COUNTER__ is evaluated

Fixes: 3284c3ddc4

Signed-off-by: Daniel Brodsky <dnbrdsky@gmail.com>
---
 include/qemu/lockable.h | 4 ++--
 include/qemu/rcu.h      | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 1aeb2cb1a6..71cda060e4 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -152,7 +152,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
  *   }
  */
 #define WITH_QEMU_LOCK_GUARD(x) \
-    WITH_QEMU_LOCK_GUARD_((x), qemu_lockable_auto##__COUNTER__)
+    WITH_QEMU_LOCK_GUARD_((x), glue(qemu_lockable_auto, __COUNTER__))
 
 /**
  * QEMU_LOCK_GUARD - Lock an object until the end of the scope
@@ -170,7 +170,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
  *   }
  */
 #define QEMU_LOCK_GUARD(x) \
-    g_autoptr(QemuLockable) qemu_lockable_auto##__COUNTER__ = \
+    g_autoptr(QemuLockable) glue(qemu_lockable_auto, __COUNTER__) = \
             qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))
 
 #endif
diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 9c82683e37..570aa603eb 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -170,7 +170,7 @@ static inline void rcu_read_auto_unlock(RCUReadAuto *r)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(RCUReadAuto, rcu_read_auto_unlock)
 
 #define WITH_RCU_READ_LOCK_GUARD() \
-    WITH_RCU_READ_LOCK_GUARD_(_rcu_read_auto##__COUNTER__)
+    WITH_RCU_READ_LOCK_GUARD_(glue(_rcu_read_auto, __COUNTER__))
 
 #define WITH_RCU_READ_LOCK_GUARD_(var) \
     for (g_autoptr(RCUReadAuto) var = rcu_read_auto_lock(); \
-- 
2.25.1

[PATCH v4 2/2] lockable: replaced locks with lock guard macros where appropriate

[dnbrdsky]

From: Daniel Brodsky <dnbrdsky@gmail.com>

- ran regexp "qemu_mutex_lock\(.*\).*\n.*if" to find targets
- replaced result with QEMU_LOCK_GUARD if all unlocks at function end
- replaced result with WITH_QEMU_LOCK_GUARD if unlock not at end

Signed-off-by: Daniel Brodsky <dnbrdsky@gmail.com>
---
 block/iscsi.c         |  7 ++----
 block/nfs.c           | 51 ++++++++++++++++++++-----------------------
 cpus-common.c         | 14 +++++-------
 hw/display/qxl.c      | 43 +++++++++++++++++-------------------
 hw/vfio/platform.c    |  5 ++---
 migration/migration.c |  3 +--
 migration/multifd.c   |  8 +++----
 migration/ram.c       |  3 +--
 monitor/misc.c        |  4 +---
 ui/spice-display.c    | 14 ++++++------
 util/log.c            |  4 ++--
 util/qemu-timer.c     | 17 +++++++--------
 util/rcu.c            |  8 +++----
 util/thread-pool.c    |  3 +--
 util/vfio-helpers.c   |  5 ++---
 15 files changed, 83 insertions(+), 106 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 682abd8e09..f86a53c096 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1395,20 +1395,17 @@ static void iscsi_nop_timed_event(void *opaque)
 {
     IscsiLun *iscsilun = opaque;
 
-    qemu_mutex_lock(&iscsilun->mutex);
+    QEMU_LOCK_GUARD(&iscsilun->mutex);
     if (iscsi_get_nops_in_flight(iscsilun->iscsi) >= MAX_NOP_FAILURES) {
         error_report("iSCSI: NOP timeout. Reconnecting...");
         iscsilun->request_timed_out = true;
     } else if (iscsi_nop_out_async(iscsilun->iscsi, NULL, NULL, 0, NULL) != 0) {
         error_report("iSCSI: failed to sent NOP-Out. Disabling NOP messages.");
-        goto out;
+        return;
     }
 
     timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
     iscsi_set_events(iscsilun);
-
-out:
-    qemu_mutex_unlock(&iscsilun->mutex);
 }
 
 static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
diff --git a/block/nfs.c b/block/nfs.c
index 9a6311e270..09a78aede8 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -273,15 +273,14 @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
     nfs_co_init_task(bs, &task);
     task.iov = iov;
 
-    qemu_mutex_lock(&client->mutex);
-    if (nfs_pread_async(client->context, client->fh,
-                        offset, bytes, nfs_co_generic_cb, &task) != 0) {
-        qemu_mutex_unlock(&client->mutex);
-        return -ENOMEM;
-    }
+    WITH_QEMU_LOCK_GUARD(&client->mutex) {
+        if (nfs_pread_async(client->context, client->fh,
+                            offset, bytes, nfs_co_generic_cb, &task) != 0) {
+            return -ENOMEM;
+        }
 
-    nfs_set_events(client);
-    qemu_mutex_unlock(&client->mutex);
+        nfs_set_events(client);
+    }
     while (!task.complete) {
         qemu_coroutine_yield();
     }
@@ -320,19 +319,18 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
         buf = iov->iov[0].iov_base;
     }
 
-    qemu_mutex_lock(&client->mutex);
-    if (nfs_pwrite_async(client->context, client->fh,
-                         offset, bytes, buf,
-                         nfs_co_generic_cb, &task) != 0) {
-        qemu_mutex_unlock(&client->mutex);
-        if (my_buffer) {
-            g_free(buf);
+    WITH_QEMU_LOCK_GUARD(&client->mutex) {
+        if (nfs_pwrite_async(client->context, client->fh,
+                             offset, bytes, buf,
+                             nfs_co_generic_cb, &task) != 0) {
+            if (my_buffer) {
+                g_free(buf);
+            }
+            return -ENOMEM;
         }
-        return -ENOMEM;
-    }
 
-    nfs_set_events(client);
-    qemu_mutex_unlock(&client->mutex);
+        nfs_set_events(client);
+    }
     while (!task.complete) {
         qemu_coroutine_yield();
     }
@@ -355,15 +353,14 @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs)
 
     nfs_co_init_task(bs, &task);
 
-    qemu_mutex_lock(&client->mutex);
-    if (nfs_fsync_async(client->context, client->fh, nfs_co_generic_cb,
-                        &task) != 0) {
-        qemu_mutex_unlock(&client->mutex);
-        return -ENOMEM;
-    }
+    WITH_QEMU_LOCK_GUARD(&client->mutex) {
+        if (nfs_fsync_async(client->context, client->fh, nfs_co_generic_cb,
+                            &task) != 0) {
+            return -ENOMEM;
+        }
 
-    nfs_set_events(client);
-    qemu_mutex_unlock(&client->mutex);
+        nfs_set_events(client);
+    }
     while (!task.complete) {
         qemu_coroutine_yield();
     }
diff --git a/cpus-common.c b/cpus-common.c
index eaf590cb38..55d5df8923 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -22,6 +22,7 @@
 #include "exec/cpu-common.h"
 #include "hw/core/cpu.h"
 #include "sysemu/cpus.h"
+#include "qemu/lockable.h"
 
 static QemuMutex qemu_cpu_list_lock;
 static QemuCond exclusive_cond;
@@ -71,7 +72,7 @@ static int cpu_get_free_index(void)
 
 void cpu_list_add(CPUState *cpu)
 {
-    qemu_mutex_lock(&qemu_cpu_list_lock);
+    QEMU_LOCK_GUARD(&qemu_cpu_list_lock);
     if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
         cpu->cpu_index = cpu_get_free_index();
         assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
@@ -79,15 +80,13 @@ void cpu_list_add(CPUState *cpu)
         assert(!cpu_index_auto_assigned);
     }
     QTAILQ_INSERT_TAIL_RCU(&cpus, cpu, node);
-    qemu_mutex_unlock(&qemu_cpu_list_lock);
 }
 
 void cpu_list_remove(CPUState *cpu)
 {
-    qemu_mutex_lock(&qemu_cpu_list_lock);
+    QEMU_LOCK_GUARD(&qemu_cpu_list_lock);
     if (!QTAILQ_IN_USE(cpu, node)) {
         /* there is nothing to undo since cpu_exec_init() hasn't been called */
-        qemu_mutex_unlock(&qemu_cpu_list_lock);
         return;
     }
 
@@ -95,7 +94,6 @@ void cpu_list_remove(CPUState *cpu)
 
     QTAILQ_REMOVE_RCU(&cpus, cpu, node);
     cpu->cpu_index = UNASSIGNED_CPU_INDEX;
-    qemu_mutex_unlock(&qemu_cpu_list_lock);
 }
 
 struct qemu_work_item {
@@ -237,7 +235,7 @@ void cpu_exec_start(CPUState *cpu)
      * see cpu->running == true, and it will kick the CPU.
      */
     if (unlikely(atomic_read(&pending_cpus))) {
-        qemu_mutex_lock(&qemu_cpu_list_lock);
+        QEMU_LOCK_GUARD(&qemu_cpu_list_lock);
         if (!cpu->has_waiter) {
             /* Not counted in pending_cpus, let the exclusive item
              * run.  Since we have the lock, just set cpu->running to true
@@ -252,7 +250,6 @@ void cpu_exec_start(CPUState *cpu)
              * waiter at cpu_exec_end.
              */
         }
-        qemu_mutex_unlock(&qemu_cpu_list_lock);
     }
 }
 
@@ -280,7 +277,7 @@ void cpu_exec_end(CPUState *cpu)
      * next cpu_exec_start.
      */
     if (unlikely(atomic_read(&pending_cpus))) {
-        qemu_mutex_lock(&qemu_cpu_list_lock);
+        QEMU_LOCK_GUARD(&qemu_cpu_list_lock);
         if (cpu->has_waiter) {
             cpu->has_waiter = false;
             atomic_set(&pending_cpus, pending_cpus - 1);
@@ -288,7 +285,6 @@ void cpu_exec_end(CPUState *cpu)
                 qemu_cond_signal(&exclusive_cond);
             }
         }
-        qemu_mutex_unlock(&qemu_cpu_list_lock);
     }
 }
 
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 227da69a50..d5627119ec 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -478,18 +478,19 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
                               cmd->u.surface_create.stride);
             return 1;
         }
-        qemu_mutex_lock(&qxl->track_lock);
-        if (cmd->type == QXL_SURFACE_CMD_CREATE) {
-            qxl->guest_surfaces.cmds[id] = ext->cmd.data;
-            qxl->guest_surfaces.count++;
-            if (qxl->guest_surfaces.max < qxl->guest_surfaces.count)
-                qxl->guest_surfaces.max = qxl->guest_surfaces.count;
+        WITH_QEMU_LOCK_GUARD(&qxl->track_lock) {
+            if (cmd->type == QXL_SURFACE_CMD_CREATE) {
+                qxl->guest_surfaces.cmds[id] = ext->cmd.data;
+                qxl->guest_surfaces.count++;
+                if (qxl->guest_surfaces.max < qxl->guest_surfaces.count) {
+                    qxl->guest_surfaces.max = qxl->guest_surfaces.count;
+                }
+            }
+            if (cmd->type == QXL_SURFACE_CMD_DESTROY) {
+                qxl->guest_surfaces.cmds[id] = 0;
+                qxl->guest_surfaces.count--;
+            }
         }
-        if (cmd->type == QXL_SURFACE_CMD_DESTROY) {
-            qxl->guest_surfaces.cmds[id] = 0;
-            qxl->guest_surfaces.count--;
-        }
-        qemu_mutex_unlock(&qxl->track_lock);
         break;
     }
     case QXL_CMD_CURSOR:
@@ -958,10 +959,9 @@ static void interface_update_area_complete(QXLInstance *sin,
     int i;
     int qxl_i;
 
-    qemu_mutex_lock(&qxl->ssd.lock);
+    QEMU_LOCK_GUARD(&qxl->ssd.lock);
     if (surface_id != 0 || !num_updated_rects ||
         !qxl->render_update_cookie_num) {
-        qemu_mutex_unlock(&qxl->ssd.lock);
         return;
     }
     trace_qxl_interface_update_area_complete(qxl->id, surface_id, dirty->left,
@@ -980,7 +980,6 @@ static void interface_update_area_complete(QXLInstance *sin,
          * Don't bother copying or scheduling the bh since we will flip
          * the whole area anyway on completion of the update_area async call
          */
-        qemu_mutex_unlock(&qxl->ssd.lock);
         return;
     }
     qxl_i = qxl->num_dirty_rects;
@@ -991,7 +990,6 @@ static void interface_update_area_complete(QXLInstance *sin,
     trace_qxl_interface_update_area_complete_schedule_bh(qxl->id,
                                                          qxl->num_dirty_rects);
     qemu_bh_schedule(qxl->update_area_bh);
-    qemu_mutex_unlock(&qxl->ssd.lock);
 }
 
 /* called from spice server thread context only */
@@ -1694,15 +1692,14 @@ static void ioport_write(void *opaque, hwaddr addr,
     case QXL_IO_MONITORS_CONFIG_ASYNC:
 async_common:
         async = QXL_ASYNC;
-        qemu_mutex_lock(&d->async_lock);
-        if (d->current_async != QXL_UNDEFINED_IO) {
-            qxl_set_guest_bug(d, "%d async started before last (%d) complete",
-                io_port, d->current_async);
-            qemu_mutex_unlock(&d->async_lock);
-            return;
+        WITH_QEMU_LOCK_GUARD(&d->async_lock) {
+            if (d->current_async != QXL_UNDEFINED_IO) {
+                qxl_set_guest_bug(d, "%d async started before last (%d) complete",
+                    io_port, d->current_async);
+                return;
+            }
+            d->current_async = orig_io_port;
         }
-        d->current_async = orig_io_port;
-        qemu_mutex_unlock(&d->async_lock);
         break;
     default:
         break;
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 6b2952c034..ac2cefc9b1 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -22,6 +22,7 @@
 #include "hw/vfio/vfio-platform.h"
 #include "migration/vmstate.h"
 #include "qemu/error-report.h"
+#include "qemu/lockable.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/range.h"
@@ -216,7 +217,7 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
     VFIOPlatformDevice *vdev = intp->vdev;
     bool delay_handling = false;
 
-    qemu_mutex_lock(&vdev->intp_mutex);
+    QEMU_LOCK_GUARD(&vdev->intp_mutex);
     if (intp->state == VFIO_IRQ_INACTIVE) {
         QLIST_FOREACH(tmp, &vdev->intp_list, next) {
             if (tmp->state == VFIO_IRQ_ACTIVE ||
@@ -236,7 +237,6 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
         QSIMPLEQ_INSERT_TAIL(&vdev->pending_intp_queue,
                              intp, pqnext);
         ret = event_notifier_test_and_clear(intp->interrupt);
-        qemu_mutex_unlock(&vdev->intp_mutex);
         return;
     }
 
@@ -266,7 +266,6 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
                   qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
                       vdev->mmap_timeout);
     }
-    qemu_mutex_unlock(&vdev->intp_mutex);
 }
 
 /**
diff --git a/migration/migration.c b/migration/migration.c
index c1d88ace7f..2f0bd6d8b4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1652,11 +1652,10 @@ static void migrate_fd_cleanup_bh(void *opaque)
 
 void migrate_set_error(MigrationState *s, const Error *error)
 {
-    qemu_mutex_lock(&s->error_mutex);
+    QEMU_LOCK_GUARD(&s->error_mutex);
     if (!s->error) {
         s->error = error_copy(error);
     }
-    qemu_mutex_unlock(&s->error_mutex);
 }
 
 void migrate_fd_error(MigrationState *s, const Error *error)
diff --git a/migration/multifd.c b/migration/multifd.c
index cb6a4a3ab8..9123c111a3 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -894,11 +894,11 @@ void multifd_recv_sync_main(void)
     for (i = 0; i < migrate_multifd_channels(); i++) {
         MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
-        qemu_mutex_lock(&p->mutex);
-        if (multifd_recv_state->packet_num < p->packet_num) {
-            multifd_recv_state->packet_num = p->packet_num;
+        WITH_QEMU_LOCK_GUARD(&p->mutex) {
+            if (multifd_recv_state->packet_num < p->packet_num) {
+                multifd_recv_state->packet_num = p->packet_num;
+            }
         }
-        qemu_mutex_unlock(&p->mutex);
         trace_multifd_recv_sync_main_signal(p->id);
         qemu_sem_post(&p->sem_sync);
     }
diff --git a/migration/ram.c b/migration/ram.c
index c12cfdbe26..87a670cfbf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1368,7 +1368,7 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset)
         return NULL;
     }
 
-    qemu_mutex_lock(&rs->src_page_req_mutex);
+    QEMU_LOCK_GUARD(&rs->src_page_req_mutex);
     if (!QSIMPLEQ_EMPTY(&rs->src_page_requests)) {
         struct RAMSrcPageRequest *entry =
                                 QSIMPLEQ_FIRST(&rs->src_page_requests);
@@ -1385,7 +1385,6 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset)
             migration_consume_urgent_request();
         }
     }
-    qemu_mutex_unlock(&rs->src_page_req_mutex);
 
     return block;
 }
diff --git a/monitor/misc.c b/monitor/misc.c
index 6c45fa490f..9723b466cd 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1473,7 +1473,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
     MonFdsetFd *mon_fdset_fd;
     AddfdInfo *fdinfo;
 
-    qemu_mutex_lock(&mon_fdsets_lock);
+    QEMU_LOCK_GUARD(&mon_fdsets_lock);
     if (has_fdset_id) {
         QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
             /* Break if match found or match impossible due to ordering by ID */
@@ -1494,7 +1494,6 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
             if (fdset_id < 0) {
                 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id",
                            "a non-negative value");
-                qemu_mutex_unlock(&mon_fdsets_lock);
                 return NULL;
             }
             /* Use specified fdset ID */
@@ -1545,7 +1544,6 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
     fdinfo->fdset_id = mon_fdset->id;
     fdinfo->fd = mon_fdset_fd->fd;
 
-    qemu_mutex_unlock(&mon_fdsets_lock);
     return fdinfo;
 }
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6babe24909..19632fdf6c 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -18,6 +18,7 @@
 #include "qemu/osdep.h"
 #include "ui/qemu-spice.h"
 #include "qemu/timer.h"
+#include "qemu/lockable.h"
 #include "qemu/main-loop.h"
 #include "qemu/option.h"
 #include "qemu/queue.h"
@@ -483,12 +484,12 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 {
     graphic_hw_update(ssd->dcl.con);
 
-    qemu_mutex_lock(&ssd->lock);
-    if (QTAILQ_EMPTY(&ssd->updates) && ssd->ds) {
-        qemu_spice_create_update(ssd);
-        ssd->notify++;
+    WITH_QEMU_LOCK_GUARD(&ssd->lock) {
+        if (QTAILQ_EMPTY(&ssd->updates) && ssd->ds) {
+            qemu_spice_create_update(ssd);
+            ssd->notify++;
+        }
     }
-    qemu_mutex_unlock(&ssd->lock);
 
     trace_qemu_spice_display_refresh(ssd->qxl.id, ssd->notify);
     if (ssd->notify) {
@@ -580,7 +581,7 @@ static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
     SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
     int ret;
 
-    qemu_mutex_lock(&ssd->lock);
+    QEMU_LOCK_GUARD(&ssd->lock);
     if (ssd->ptr_define) {
         *ext = ssd->ptr_define->ext;
         ssd->ptr_define = NULL;
@@ -592,7 +593,6 @@ static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
     } else {
         ret = false;
     }
-    qemu_mutex_unlock(&ssd->lock);
     return ret;
 }
 
diff --git a/util/log.c b/util/log.c
index 2da6cb31dc..bdb3d712e8 100644
--- a/util/log.c
+++ b/util/log.c
@@ -25,6 +25,7 @@
 #include "qemu/cutils.h"
 #include "trace/control.h"
 #include "qemu/thread.h"
+#include "qemu/lockable.h"
 
 static char *logfilename;
 static QemuMutex qemu_logfile_mutex;
@@ -94,7 +95,7 @@ void qemu_set_log(int log_flags)
     if (qemu_loglevel && (!is_daemonized() || logfilename)) {
         need_to_open_file = true;
     }
-    qemu_mutex_lock(&qemu_logfile_mutex);
+    QEMU_LOCK_GUARD(&qemu_logfile_mutex);
     if (qemu_logfile && !need_to_open_file) {
         logfile = qemu_logfile;
         atomic_rcu_set(&qemu_logfile, NULL);
@@ -136,7 +137,6 @@ void qemu_set_log(int log_flags)
         }
         atomic_rcu_set(&qemu_logfile, logfile);
     }
-    qemu_mutex_unlock(&qemu_logfile_mutex);
 }
 
 void qemu_log_needs_buffers(void)
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index d548d3c1ad..b6575a2cd5 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -459,17 +459,16 @@ void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time)
     QEMUTimerList *timer_list = ts->timer_list;
     bool rearm;
 
-    qemu_mutex_lock(&timer_list->active_timers_lock);
-    if (ts->expire_time == -1 || ts->expire_time > expire_time) {
-        if (ts->expire_time != -1) {
-            timer_del_locked(timer_list, ts);
+    WITH_QEMU_LOCK_GUARD(&timer_list->active_timers_lock) {
+        if (ts->expire_time == -1 || ts->expire_time > expire_time) {
+            if (ts->expire_time != -1) {
+                timer_del_locked(timer_list, ts);
+            }
+            rearm = timer_mod_ns_locked(timer_list, ts, expire_time);
+        } else {
+            rearm = false;
         }
-        rearm = timer_mod_ns_locked(timer_list, ts, expire_time);
-    } else {
-        rearm = false;
     }
-    qemu_mutex_unlock(&timer_list->active_timers_lock);
-
     if (rearm) {
         timerlist_rearm(timer_list);
     }
diff --git a/util/rcu.c b/util/rcu.c
index 177a675619..60a37f72c3 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -31,6 +31,7 @@
 #include "qemu/atomic.h"
 #include "qemu/thread.h"
 #include "qemu/main-loop.h"
+#include "qemu/lockable.h"
 #if defined(CONFIG_MALLOC_TRIM)
 #include <malloc.h>
 #endif
@@ -141,14 +142,14 @@ static void wait_for_readers(void)
 
 void synchronize_rcu(void)
 {
-    qemu_mutex_lock(&rcu_sync_lock);
+    QEMU_LOCK_GUARD(&rcu_sync_lock);
 
     /* Write RCU-protected pointers before reading p_rcu_reader->ctr.
      * Pairs with smp_mb_placeholder() in rcu_read_lock().
      */
     smp_mb_global();
 
-    qemu_mutex_lock(&rcu_registry_lock);
+    QEMU_LOCK_GUARD(&rcu_registry_lock);
     if (!QLIST_EMPTY(&registry)) {
         /* In either case, the atomic_mb_set below blocks stores that free
          * old RCU-protected pointers.
@@ -169,9 +170,6 @@ void synchronize_rcu(void)
 
         wait_for_readers();
     }
-
-    qemu_mutex_unlock(&rcu_registry_lock);
-    qemu_mutex_unlock(&rcu_sync_lock);
 }
 
 
diff --git a/util/thread-pool.c b/util/thread-pool.c
index 4ed9b89ab2..d763cea505 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -210,7 +210,7 @@ static void thread_pool_cancel(BlockAIOCB *acb)
 
     trace_thread_pool_cancel(elem, elem->common.opaque);
 
-    qemu_mutex_lock(&pool->lock);
+    QEMU_LOCK_GUARD(&pool->lock);
     if (elem->state == THREAD_QUEUED &&
         /* No thread has yet started working on elem. we can try to "steal"
          * the item from the worker if we can get a signal from the
@@ -225,7 +225,6 @@ static void thread_pool_cancel(BlockAIOCB *acb)
         elem->ret = -ECANCELED;
     }
 
-    qemu_mutex_unlock(&pool->lock);
 }
 
 static AioContext *thread_pool_get_aio_context(BlockAIOCB *acb)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index ddd9a96e76..e399e330e2 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -21,6 +21,7 @@
 #include "standard-headers/linux/pci_regs.h"
 #include "qemu/event_notifier.h"
 #include "qemu/vfio-helpers.h"
+#include "qemu/lockable.h"
 #include "trace.h"
 
 #define QEMU_VFIO_DEBUG 0
@@ -667,14 +668,12 @@ int qemu_vfio_dma_reset_temporary(QEMUVFIOState *s)
         .size = QEMU_VFIO_IOVA_MAX - s->high_water_mark,
     };
     trace_qemu_vfio_dma_reset_temporary(s);
-    qemu_mutex_lock(&s->lock);
+    QEMU_LOCK_GUARD(&s->lock);
     if (ioctl(s->container, VFIO_IOMMU_UNMAP_DMA, &unmap)) {
         error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
-        qemu_mutex_unlock(&s->lock);
         return -errno;
     }
     s->high_water_mark = QEMU_VFIO_IOVA_MAX;
-    qemu_mutex_unlock(&s->lock);
     return 0;
 }
 
-- 
2.25.1

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200320123137.1937091-1-dnbrdsky@gmail.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      util/module.o
  CC      util/host-utils.o
  CC      util/bitmap.o
/tmp/qemu-test/src/util/thread-pool.c:213:5: error: unused variable 'qemu_lockable_auto1' [-Werror,-Wunused-variable]
    QEMU_LOCK_GUARD(&pool->lock);
    ^
/tmp/qemu-test/src/include/qemu/lockable.h:173:29: note: expanded from macro 'QEMU_LOCK_GUARD'
---
qemu_lockable_auto1
^
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: util/thread-pool.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=83384554d5524a41b38a2f80c48c2f30', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-zoq4pqgi/src/docker-src.2020-03-20-09.39.56.20282:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=83384554d5524a41b38a2f80c48c2f30
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-zoq4pqgi/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    3m26.893s
user    0m8.375s


The full log is available at
http://patchew.org/logs/20200320123137.1937091-1-dnbrdsky@gmail.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On Fri, Mar 20, 2020 at 06:43:23AM -0700, no-reply@patchew.org wrote:
> /tmp/qemu-test/src/util/thread-pool.c:213:5: error: unused variable 'qemu_lockable_auto1' [-Werror,-Wunused-variable]
>     QEMU_LOCK_GUARD(&pool->lock);
>     ^
> /tmp/qemu-test/src/include/qemu/lockable.h:173:29: note: expanded from macro 'QEMU_LOCK_GUARD'

Apparently gcc suppresses "unused variable" warnings with g_autoptr() so
we didn't see this warning before.

clang does report them so let's silence the warning manually.  Please
add G_GNUC_UNUSED onto the g_autoptr variable definition in the
QEMU_LOCK_GUARD() macro:

  #define QEMU_LOCK_GUARD(x) \
      g_autoptr(QemuLockable) qemu_lockable_auto##__COUNTER__ G_GNUC_UNUSED = \
              qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))

The WITH_*_LOCK_GUARD() macros should not require changes because the
variable is both read and written.

You can test locally by building with clang (llvm) instead of gcc:

  ./configure --cc=clang

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/2] lockable: replaced locks with lock guard macros where appropriate

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

On Fri, Mar 20, 2020 at 05:31:37AM -0700, dnbrdsky@gmail.com wrote:
> From: Daniel Brodsky <dnbrdsky@gmail.com>
> 
> - ran regexp "qemu_mutex_lock\(.*\).*\n.*if" to find targets
> - replaced result with QEMU_LOCK_GUARD if all unlocks at function end
> - replaced result with WITH_QEMU_LOCK_GUARD if unlock not at end
> 
> Signed-off-by: Daniel Brodsky <dnbrdsky@gmail.com>
> ---
>  block/iscsi.c         |  7 ++----
>  block/nfs.c           | 51 ++++++++++++++++++++-----------------------
>  cpus-common.c         | 14 +++++-------
>  hw/display/qxl.c      | 43 +++++++++++++++++-------------------
>  hw/vfio/platform.c    |  5 ++---
>  migration/migration.c |  3 +--
>  migration/multifd.c   |  8 +++----
>  migration/ram.c       |  3 +--
>  monitor/misc.c        |  4 +---
>  ui/spice-display.c    | 14 ++++++------
>  util/log.c            |  4 ++--
>  util/qemu-timer.c     | 17 +++++++--------
>  util/rcu.c            |  8 +++----
>  util/thread-pool.c    |  3 +--
>  util/vfio-helpers.c   |  5 ++---
>  15 files changed, 83 insertions(+), 106 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 1/2] lockable: fix __COUNTER__ macro to be referenced properly

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

On Fri, Mar 20, 2020 at 05:31:36AM -0700, dnbrdsky@gmail.com wrote:
> From: Daniel Brodsky <dnbrdsky@gmail.com>
> 
> - __COUNTER__ doesn't work with ## concat
> - replaced ## with glue() macro so __COUNTER__ is evaluated
> 
> Fixes: 3284c3ddc4
> 
> Signed-off-by: Daniel Brodsky <dnbrdsky@gmail.com>
> ---
>  include/qemu/lockable.h | 4 ++--
>  include/qemu/rcu.h      | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Please see my comment on the Patchew CI error message in this email
thread.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Daniel Brodsky]

On Mon, Mar 23, 2020 at 6:25 AM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Fri, Mar 20, 2020 at 06:43:23AM -0700, no-reply@patchew.org wrote:
> > /tmp/qemu-test/src/util/thread-pool.c:213:5: error: unused variable 'qemu_lockable_auto1' [-Werror,-Wunused-variable]
> >     QEMU_LOCK_GUARD(&pool->lock);
> >     ^
> > /tmp/qemu-test/src/include/qemu/lockable.h:173:29: note: expanded from macro 'QEMU_LOCK_GUARD'
>
> Apparently gcc suppresses "unused variable" warnings with g_autoptr() so
> we didn't see this warning before.
>
> clang does report them so let's silence the warning manually.  Please
> add G_GNUC_UNUSED onto the g_autoptr variable definition in the
> QEMU_LOCK_GUARD() macro:
>
>   #define QEMU_LOCK_GUARD(x) \
>       g_autoptr(QemuLockable) qemu_lockable_auto##__COUNTER__ G_GNUC_UNUSED = \
>               qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))
>
> The WITH_*_LOCK_GUARD() macros should not require changes because the
> variable is both read and written.
>
> You can test locally by building with clang (llvm) instead of gcc:
>
>   ./configure --cc=clang

Using --cc=clang is causing the following error in several different places:
qemu/target/ppc/translate.c:1894:18: error: result of comparison
'target_ulong' (aka 'unsigned int') <= 4294967295
is always true [-Werror,-Wtautological-type-limit-compare]
        if (mask <= 0xffffffffu) {
            ~~~~ ^  ~~~~~~~~~~~

these errors don't come up when building with gcc. Any idea how to fix
this? Or should I just suppress it?

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Richard Henderson]

On 3/23/20 2:46 PM, Daniel Brodsky wrote:
> On Mon, Mar 23, 2020 at 6:25 AM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>>
>> On Fri, Mar 20, 2020 at 06:43:23AM -0700, no-reply@patchew.org wrote:
>>> /tmp/qemu-test/src/util/thread-pool.c:213:5: error: unused variable 'qemu_lockable_auto1' [-Werror,-Wunused-variable]
>>>     QEMU_LOCK_GUARD(&pool->lock);
>>>     ^
>>> /tmp/qemu-test/src/include/qemu/lockable.h:173:29: note: expanded from macro 'QEMU_LOCK_GUARD'
>>
>> Apparently gcc suppresses "unused variable" warnings with g_autoptr() so
>> we didn't see this warning before.
>>
>> clang does report them so let's silence the warning manually.  Please
>> add G_GNUC_UNUSED onto the g_autoptr variable definition in the
>> QEMU_LOCK_GUARD() macro:
>>
>>   #define QEMU_LOCK_GUARD(x) \
>>       g_autoptr(QemuLockable) qemu_lockable_auto##__COUNTER__ G_GNUC_UNUSED = \
>>               qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))
>>
>> The WITH_*_LOCK_GUARD() macros should not require changes because the
>> variable is both read and written.
>>
>> You can test locally by building with clang (llvm) instead of gcc:
>>
>>   ./configure --cc=clang
> 
> Using --cc=clang is causing the following error in several different places:
> qemu/target/ppc/translate.c:1894:18: error: result of comparison
> 'target_ulong' (aka 'unsigned int') <= 4294967295
> is always true [-Werror,-Wtautological-type-limit-compare]
>         if (mask <= 0xffffffffu) {
>             ~~~~ ^  ~~~~~~~~~~~
> 
> these errors don't come up when building with gcc. Any idea how to fix
> this? Or should I just suppress it?

I'm of the opinion that it should be suppressed.

This is the *correct* test for ppc64, and the warning for ppc32 is simply
because target_ulong == uint32_t.  True is the correct result for ppc32.

We simply want the compiler to DTRT: simplify this test and remove the else as
unreachable.


r~

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Eric Blake]

>>> You can test locally by building with clang (llvm) instead of gcc:
>>>
>>>    ./configure --cc=clang
>>
>> Using --cc=clang is causing the following error in several different places:
>> qemu/target/ppc/translate.c:1894:18: error: result of comparison
>> 'target_ulong' (aka 'unsigned int') <= 4294967295
>> is always true [-Werror,-Wtautological-type-limit-compare]
>>          if (mask <= 0xffffffffu) {
>>              ~~~~ ^  ~~~~~~~~~~~
>>
>> these errors don't come up when building with gcc. Any idea how to fix
>> this? Or should I just suppress it?
> 
> I'm of the opinion that it should be suppressed.
> 
> This is the *correct* test for ppc64, and the warning for ppc32 is simply
> because target_ulong == uint32_t.  True is the correct result for ppc32.
> 
> We simply want the compiler to DTRT: simplify this test and remove the else as
> unreachable.

There may be ways to rewrite that expression to avoid triggering the 
warning on a 32-bit platform.  Untested, but does this help:

if (sizeof(mask) > 4 && mask <= 0xffffffffu) {

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Daniel Brodsky]

>
> There may be ways to rewrite that expression to avoid triggering the
> warning on a 32-bit platform.  Untested, but does this help:
>
> if (sizeof(mask) > 4 && mask <= 0xffffffffu) {
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
Unfortunately, the compiler still catches that the statement is always
true for 32-bit.
An alternative might be to convert cases like this to a macro instead,
which doesn't
cause any errors.

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Richard Henderson]

On Wed, 25 Mar 2020 at 04:35, Daniel Brodsky <dnbrdsky@gmail.com> wrote:
> > There may be ways to rewrite that expression to avoid triggering the
> > warning on a 32-bit platform.  Untested, but does this help:
> >
> > if (sizeof(mask) > 4 && mask <= 0xffffffffu) {
> >
> > --
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc.           +1-919-301-3226
> > Virtualization:  qemu.org | libvirt.org
> >
> Unfortunately, the compiler still catches that the statement is always
> true for 32-bit.
> An alternative might be to convert cases like this to a macro instead,
> which doesn't
> cause any errors.

My preference is to add -Wno-tautological-type-limit-compare in
configure, so we don't have to work around this same issue elsewhere
in the code base.

r~

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Daniel Brodsky]

On Thu, Mar 26, 2020 at 11:01 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> My preference is to add -Wno-tautological-type-limit-compare in
> configure, so we don't have to work around this same issue elsewhere
> in the code base.
>
> r~

What do you think would be the best way to add this? I could change
all additions of the `-m32` flag to instead use `-m32
-Wno-tautological-type-limit-compare` or add the flag if qemu is being
compiled with clang and `-m32` already enabled.

Daniel

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Richard Henderson]

On 3/27/20 11:38 PM, Daniel Brodsky wrote:
> On Thu, Mar 26, 2020 at 11:01 AM Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> My preference is to add -Wno-tautological-type-limit-compare in
>> configure, so we don't have to work around this same issue elsewhere
>> in the code base.
>>
>> r~
> 
> What do you think would be the best way to add this? I could change
> all additions of the `-m32` flag to instead use `-m32
> -Wno-tautological-type-limit-compare` or add the flag if qemu is being
> compiled with clang and `-m32` already enabled.

I was going to add it unconditionally, with all of the other warning flags.

Except that it doesn't work -- clang-9 *still* warns.  Clearly a clang bug, but
there doesn't seem to be any workaround at all except --disable-werror.


r~

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Daniel Brodsky]

On Sat, Mar 28, 2020 at 9:12 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/27/20 11:38 PM, Daniel Brodsky wrote:
> > On Thu, Mar 26, 2020 at 11:01 AM Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> My preference is to add -Wno-tautological-type-limit-compare in
> >> configure, so we don't have to work around this same issue elsewhere
> >> in the code base.
> >>
> >> r~
> >
> > What do you think would be the best way to add this? I could change
> > all additions of the `-m32` flag to instead use `-m32
> > -Wno-tautological-type-limit-compare` or add the flag if qemu is being
> > compiled with clang and `-m32` already enabled.
>
> I was going to add it unconditionally, with all of the other warning flags.
>
> Except that it doesn't work -- clang-9 *still* warns.  Clearly a clang bug, but
> there doesn't seem to be any workaround at all except --disable-werror.
>
>
> r~
Using `#pragma clang diagnostic ignored
"-Wtautological-type-limit-compare"` suppresses the errors (on Clang
9). I could go and drop that in for the problem areas? There's only a
few so it wouldn't be a major change. I'm thinking of adding a macro
like this:
#define PRAGMA(x) _Pragma(stringify(x))
#define IF_IGNORE_TYPE_LIMIT(statement) \
        PRAGMA(clang diagnostic push) \
        PRAGMA(clang diagnostic ignored "-Wtautological-type-limit-compare") \
        if (statement) \
        PRAGMA(clang diagnostic pop)

and replacing the problem conditionals with it.

Daniel

Re: [PATCH v4 0/2] Replaced locks with lock guard macros

[Peter Maydell]

On Mon, 30 Mar 2020 at 06:54, Daniel Brodsky <dnbrdsky@gmail.com> wrote:

> Using `#pragma clang diagnostic ignored
> "-Wtautological-type-limit-compare"` suppresses the errors (on Clang
> 9). I could go and drop that in for the problem areas? There's only a
> few so it wouldn't be a major change. I'm thinking of adding a macro
> like this:
> #define PRAGMA(x) _Pragma(stringify(x))
> #define IF_IGNORE_TYPE_LIMIT(statement) \
>         PRAGMA(clang diagnostic push) \
>         PRAGMA(clang diagnostic ignored "-Wtautological-type-limit-compare") \
>         if (statement) \
>         PRAGMA(clang diagnostic pop)

This is not an in-principle objection, but we have found in the past that
various gcc/clang implementations of _Pragma() are simply buggy
when used inside macros; see the comments on this patch attempt:
https://patchwork.kernel.org/patch/10620079/
(one of which has a link to half a dozen gcc bug reports involving
_Pragma and three clang bugs). For that particular case the approach
we eventually took was to only use the _Pragma() stuff on clang because
gcc mishandled it and luckily the spurious warning was clang-only.
It's a shame, because the whole point of _Pragma() is to let you do
this kind of thing in a macro, but the actual implementations in
compilers are clearly just not fit-for-purpose.

So if you do go down this path please make sure you test it on a
wide variety of different clang and gcc versions.

thanks
-- PMM