* [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment
@ 2021-10-05 18:58 Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 1/4] block/aio: Add automatically released aio_context variants Philippe Mathieu-Daudé
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-05 18:58 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Dr . David Alan Gilbert, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé
Experiment to use glib g_autoptr/autofree features with
AIO context.
Since this is a RFC, only few examples are provided.
TODO: Document the macros in docs/devel/multiple-iothreads.txt
Philippe Mathieu-Daudé (4):
block/aio: Add automatically released aio_context variants
hw/scsi/scsi-disk: Use automatic AIO context lock
hw/scsi/scsi-generic: Use automatic AIO context lock
hw/block/virtio-blk: Use automatic AIO context lock
include/block/aio.h | 24 ++++++++++++++++++++++++
hw/block/virtio-blk.c | 26 ++++++++++++--------------
hw/scsi/scsi-disk.c | 13 ++++---------
hw/scsi/scsi-generic.c | 6 +++---
4 files changed, 43 insertions(+), 26 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/4] block/aio: Add automatically released aio_context variants
2021-10-05 18:58 [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Philippe Mathieu-Daudé
@ 2021-10-05 18:58 ` Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock Philippe Mathieu-Daudé
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-05 18:58 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Dr . David Alan Gilbert, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé
Similarly to commit 5626f8c6d46 ("rcu: Add automatically
released rcu_read_lock variants"):
AIO_CONTEXT_ACQUIRE_GUARD() acquires the aio context and then uses
glib's g_auto infrastructure (and thus whatever the compiler's hooks
are) to release it on all exits of the block.
WITH_AIO_CONTEXT_ACQUIRE_GUARD() is similar but is used as a wrapper
for the lock, i.e.:
WITH_AIO_CONTEXT_ACQUIRE_GUARD() {
stuff with context acquired
}
Inspired-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/block/aio.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/include/block/aio.h b/include/block/aio.h
index 47fbe9d81f2..4fa5a5c2720 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -294,6 +294,30 @@ void aio_context_acquire(AioContext *ctx);
/* Relinquish ownership of the AioContext. */
void aio_context_release(AioContext *ctx);
+static inline AioContext *aio_context_auto_acquire(AioContext *ctx)
+{
+ aio_context_acquire(ctx);
+ return ctx;
+}
+
+static inline void aio_context_auto_release(AioContext *ctx)
+{
+ aio_context_release(ctx);
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(AioContext, aio_context_auto_release)
+
+#define WITH_AIO_CONTEXT_ACQUIRE_GUARD(ctx) \
+ WITH_AIO_CONTEXT_ACQUIRE_GUARD_(glue(_aio_context_auto, __COUNTER__), ctx)
+
+#define WITH_AIO_CONTEXT_ACQUIRE_GUARD_(var, ctx) \
+ for (g_autoptr(AioContext) var = aio_context_auto_acquire(ctx); \
+ (var); aio_context_auto_release(var), (var) = NULL)
+
+#define AIO_CONTEXT_ACQUIRE_GUARD(ctx) \
+ g_autoptr(AioContext) _aio_context_auto __attribute__((unused)) \
+ = aio_context_auto_acquire(ctx)
+
/**
* aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will
* run only once and as soon as possible.
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock
2021-10-05 18:58 [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 1/4] block/aio: Add automatically released aio_context variants Philippe Mathieu-Daudé
@ 2021-10-05 18:58 ` Philippe Mathieu-Daudé
2021-10-06 16:51 ` Dr. David Alan Gilbert
2021-10-05 18:58 ` [RFC PATCH 3/4] hw/scsi/scsi-generic: " Philippe Mathieu-Daudé
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-05 18:58 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Dr . David Alan Gilbert, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé
Use the automatic AIO context acquire/release in scsi_block_realize().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/scsi/scsi-disk.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e8a547dbb7d..fa2d8543718 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2605,7 +2605,6 @@ static int get_device_type(SCSIDiskState *s)
static void scsi_block_realize(SCSIDevice *dev, Error **errp)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
- AioContext *ctx;
int sg_version;
int rc;
@@ -2620,8 +2619,7 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
"be removed in a future version");
}
- ctx = blk_get_aio_context(s->qdev.conf.blk);
- aio_context_acquire(ctx);
+ AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(dev->conf.blk));
/* check we are using a driver managing SG_IO (version 3 and after) */
rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version);
@@ -2630,18 +2628,18 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
if (rc != -EPERM) {
error_append_hint(errp, "Is this a SCSI device?\n");
}
- goto out;
+ return;
}
if (sg_version < 30000) {
error_setg(errp, "scsi generic interface too old");
- goto out;
+ return;
}
/* get device type from INQUIRY data */
rc = get_device_type(s);
if (rc < 0) {
error_setg(errp, "INQUIRY failed");
- goto out;
+ return;
}
/* Make a guess for the block size, we'll fix it when the guest sends.
@@ -2661,9 +2659,6 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
scsi_realize(&s->qdev, errp);
scsi_generic_read_device_inquiry(&s->qdev);
-
-out:
- aio_context_release(ctx);
}
typedef struct SCSIBlockReq {
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 3/4] hw/scsi/scsi-generic: Use automatic AIO context lock
2021-10-05 18:58 [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 1/4] block/aio: Add automatically released aio_context variants Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock Philippe Mathieu-Daudé
@ 2021-10-05 18:58 ` Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 4/4] hw/block/virtio-blk: " Philippe Mathieu-Daudé
2021-10-07 13:15 ` [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Stefan Hajnoczi
4 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-05 18:58 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Dr . David Alan Gilbert, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé
Use the automatic AIO context acquire/release in
scsi_command_complete().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/scsi/scsi-generic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 665baf900e4..08ef623c030 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -114,9 +114,9 @@ static void scsi_command_complete(void *opaque, int ret)
assert(r->req.aiocb != NULL);
r->req.aiocb = NULL;
- aio_context_acquire(blk_get_aio_context(s->conf.blk));
- scsi_command_complete_noio(r, ret);
- aio_context_release(blk_get_aio_context(s->conf.blk));
+ WITH_AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(s->conf.blk)) {
+ scsi_command_complete_noio(r, ret);
+ }
}
static int execute_command(BlockBackend *blk,
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 4/4] hw/block/virtio-blk: Use automatic AIO context lock
2021-10-05 18:58 [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-10-05 18:58 ` [RFC PATCH 3/4] hw/scsi/scsi-generic: " Philippe Mathieu-Daudé
@ 2021-10-05 18:58 ` Philippe Mathieu-Daudé
2021-10-07 13:15 ` [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Stefan Hajnoczi
4 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-05 18:58 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Dr . David Alan Gilbert, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé
Use the automatic AIO context acquire/release in virtio_blk_reset().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/virtio-blk.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f139cd7cc9c..2dd6428e7b3 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -896,24 +896,22 @@ static void virtio_blk_dma_restart_cb(void *opaque, bool running,
static void virtio_blk_reset(VirtIODevice *vdev)
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
- AioContext *ctx;
VirtIOBlockReq *req;
- ctx = blk_get_aio_context(s->blk);
- aio_context_acquire(ctx);
- blk_drain(s->blk);
-
- /* We drop queued requests after blk_drain() because blk_drain() itself can
- * produce them. */
- while (s->rq) {
- req = s->rq;
- s->rq = req->next;
- virtqueue_detach_element(req->vq, &req->elem, 0);
- virtio_blk_free_request(req);
+ WITH_AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(s->blk)) {
+ blk_drain(s->blk);
+ /*
+ * We drop queued requests after blk_drain() because
+ * blk_drain() itself can produce them.
+ */
+ while (s->rq) {
+ req = s->rq;
+ s->rq = req->next;
+ virtqueue_detach_element(req->vq, &req->elem, 0);
+ virtio_blk_free_request(req);
+ }
}
- aio_context_release(ctx);
-
assert(!s->dataplane_started);
blk_set_enable_write_cache(s->blk, s->original_wce);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock
2021-10-05 18:58 ` [RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock Philippe Mathieu-Daudé
@ 2021-10-06 16:51 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-06 16:51 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, qemu-devel, Hanna Reitz, Stefan Hajnoczi,
Paolo Bonzini
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Use the automatic AIO context acquire/release in scsi_block_realize().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
I'll let the AIO people reply more thoroughly to this; but this patch is
the case I really like this mechanism for; removing the need for the
goto's and cleanup.
Dave
> ---
> hw/scsi/scsi-disk.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index e8a547dbb7d..fa2d8543718 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2605,7 +2605,6 @@ static int get_device_type(SCSIDiskState *s)
> static void scsi_block_realize(SCSIDevice *dev, Error **errp)
> {
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> - AioContext *ctx;
> int sg_version;
> int rc;
>
> @@ -2620,8 +2619,7 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
> "be removed in a future version");
> }
>
> - ctx = blk_get_aio_context(s->qdev.conf.blk);
> - aio_context_acquire(ctx);
> + AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(dev->conf.blk));
>
> /* check we are using a driver managing SG_IO (version 3 and after) */
> rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version);
> @@ -2630,18 +2628,18 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
> if (rc != -EPERM) {
> error_append_hint(errp, "Is this a SCSI device?\n");
> }
> - goto out;
> + return;
> }
> if (sg_version < 30000) {
> error_setg(errp, "scsi generic interface too old");
> - goto out;
> + return;
> }
>
> /* get device type from INQUIRY data */
> rc = get_device_type(s);
> if (rc < 0) {
> error_setg(errp, "INQUIRY failed");
> - goto out;
> + return;
> }
>
> /* Make a guess for the block size, we'll fix it when the guest sends.
> @@ -2661,9 +2659,6 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
>
> scsi_realize(&s->qdev, errp);
> scsi_generic_read_device_inquiry(&s->qdev);
> -
> -out:
> - aio_context_release(ctx);
> }
>
> typedef struct SCSIBlockReq {
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment
2021-10-05 18:58 [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-10-05 18:58 ` [RFC PATCH 4/4] hw/block/virtio-blk: " Philippe Mathieu-Daudé
@ 2021-10-07 13:15 ` Stefan Hajnoczi
2021-10-07 14:16 ` Philippe Mathieu-Daudé
4 siblings, 1 reply; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-10-07 13:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Emanuele Giuseppe Esposito, qemu-devel,
Dr . David Alan Gilbert, Hanna Reitz, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
On Tue, Oct 05, 2021 at 08:58:03PM +0200, Philippe Mathieu-Daudé wrote:
> Experiment to use glib g_autoptr/autofree features with
> AIO context.
> Since this is a RFC, only few examples are provided.
>
> TODO: Document the macros in docs/devel/multiple-iothreads.txt
>
> Philippe Mathieu-Daudé (4):
> block/aio: Add automatically released aio_context variants
> hw/scsi/scsi-disk: Use automatic AIO context lock
> hw/scsi/scsi-generic: Use automatic AIO context lock
> hw/block/virtio-blk: Use automatic AIO context lock
>
> include/block/aio.h | 24 ++++++++++++++++++++++++
> hw/block/virtio-blk.c | 26 ++++++++++++--------------
> hw/scsi/scsi-disk.c | 13 ++++---------
> hw/scsi/scsi-generic.c | 6 +++---
> 4 files changed, 43 insertions(+), 26 deletions(-)
This is nice. Two things:
1. Emanuele is working on eliminating aio_context_acquire/release(), so
a big effort to convert existing code to using guards could be wasted
energy and cause conflicts with his patches.
2. A few callers anticipate that the AioContext of their BDS may change
between acquire/release. Care needs to be taken when converting to
preserve the semantics but most instances should be safe to convert.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment
2021-10-07 13:15 ` [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Stefan Hajnoczi
@ 2021-10-07 14:16 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-07 14:16 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy, qemu-block,
Michael S. Tsirkin, Emanuele Giuseppe Esposito, qemu-devel,
Dr . David Alan Gilbert, Hanna Reitz, Paolo Bonzini
On 10/7/21 15:15, Stefan Hajnoczi wrote:
> On Tue, Oct 05, 2021 at 08:58:03PM +0200, Philippe Mathieu-Daudé wrote:
>> Experiment to use glib g_autoptr/autofree features with
>> AIO context.
>> Since this is a RFC, only few examples are provided.
>>
>> TODO: Document the macros in docs/devel/multiple-iothreads.txt
>>
>> Philippe Mathieu-Daudé (4):
>> block/aio: Add automatically released aio_context variants
>> hw/scsi/scsi-disk: Use automatic AIO context lock
>> hw/scsi/scsi-generic: Use automatic AIO context lock
>> hw/block/virtio-blk: Use automatic AIO context lock
>>
>> include/block/aio.h | 24 ++++++++++++++++++++++++
>> hw/block/virtio-blk.c | 26 ++++++++++++--------------
>> hw/scsi/scsi-disk.c | 13 ++++---------
>> hw/scsi/scsi-generic.c | 6 +++---
>> 4 files changed, 43 insertions(+), 26 deletions(-)
>
> This is nice. Two things:
>
> 1. Emanuele is working on eliminating aio_context_acquire/release(), so
> a big effort to convert existing code to using guards could be wasted
> energy and cause conflicts with his patches.
Thanks for the update, I'll wait Emanuele effort to land.
> 2. A few callers anticipate that the AioContext of their BDS may change
> between acquire/release. Care needs to be taken when converting to
> preserve the semantics but most instances should be safe to convert.
>
> Stefan
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-07 14:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 18:58 [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 1/4] block/aio: Add automatically released aio_context variants Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock Philippe Mathieu-Daudé
2021-10-06 16:51 ` Dr. David Alan Gilbert
2021-10-05 18:58 ` [RFC PATCH 3/4] hw/scsi/scsi-generic: " Philippe Mathieu-Daudé
2021-10-05 18:58 ` [RFC PATCH 4/4] hw/block/virtio-blk: " Philippe Mathieu-Daudé
2021-10-07 13:15 ` [RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment Stefan Hajnoczi
2021-10-07 14:16 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.