Re: [PATCH 2/8] block-backend: enable_write_cache should be atomic

[Emanuele Giuseppe Esposito]

Am 08/11/2022 um 22:19 schrieb Stefan Hajnoczi:
> From: Emanuele Giuseppe Esposito 
>
> It is read from IO_CODE and written with BQL held,
> so setting it as atomic should be enough.
>
> Also remove the aiocontext lock that was sporadically
> taken around the set.
>
> Signed-off-by: Emanuele Giuseppe Esposito 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefan Hajnoczi 
> Message-Id: <20220609143727.1151816-3-eespo...@redhat.com>
>


Reviewed-by: Emanuele Giuseppe Esposito 

[PATCH 2/8] block-backend: enable_write_cache should be atomic

[Stefan Hajnoczi]

From: Emanuele Giuseppe Esposito 

It is read from IO_CODE and written with BQL held,
so setting it as atomic should be enough.

Also remove the aiocontext lock that was sporadically
taken around the set.

Signed-off-by: Emanuele Giuseppe Esposito 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Stefan Hajnoczi 
Message-Id: <20220609143727.1151816-3-eespo...@redhat.com>
---
 block/block-backend.c | 6 +++---
 hw/block/virtio-blk.c | 4 
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index c0c7d56c8d..949418cad4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -60,7 +60,7 @@ struct BlockBackend {
  * can be used to restore those options in the new BDS on insert) */
 BlockBackendRootState root_state;
 
-bool enable_write_cache;
+bool enable_write_cache; /* Atomic */
 
 /* I/O stats (display with "info blockstats"). */
 BlockAcctStats stats;
@@ -1939,13 +1939,13 @@ bool blk_is_sg(BlockBackend *blk)
 bool blk_enable_write_cache(BlockBackend *blk)
 {
 IO_CODE();
-return blk->enable_write_cache;
+return qatomic_read(>enable_write_cache);
 }
 
 void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
 {
 IO_CODE();
-blk->enable_write_cache = wce;
+qatomic_set(>enable_write_cache, wce);
 }
 
 void blk_activate(BlockBackend *blk, Error **errp)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cdc6fd5979..96d00103a4 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -961,9 +961,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const 
uint8_t *config)
 
 memcpy(, config, s->config_size);
 
-aio_context_acquire(blk_get_aio_context(s->blk));
 blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
-aio_context_release(blk_get_aio_context(s->blk));
 }
 
 static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,
@@ -1031,11 +1029,9 @@ static void virtio_blk_set_status(VirtIODevice *vdev, 
uint8_t status)
  * s->blk would erroneously be placed in writethrough mode.
  */
 if (!virtio_vdev_has_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE)) {
-aio_context_acquire(blk_get_aio_context(s->blk));
 blk_set_enable_write_cache(s->blk,
virtio_vdev_has_feature(vdev,
VIRTIO_BLK_F_WCE));
-aio_context_release(blk_get_aio_context(s->blk));
 }
 }
 
-- 
2.38.1

Re: [PATCH 2/8] block-backend: enable_write_cache should be atomic

[Stefan Hajnoczi]

On Thu, Jun 09, 2022 at 10:37:21AM -0400, Emanuele Giuseppe Esposito wrote:
> It is read from IO_CODE and written with BQL held,
> so setting it as atomic should be enough.
> 
> Also remove the aiocontext lock that was sporadically
> taken around the set.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  block/block-backend.c | 6 +++---
>  hw/block/virtio-blk.c | 4 
>  2 files changed, 3 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

[PATCH 2/8] block-backend: enable_write_cache should be atomic

[Emanuele Giuseppe Esposito]

It is read from IO_CODE and written with BQL held,
so setting it as atomic should be enough.

Also remove the aiocontext lock that was sporadically
taken around the set.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 block/block-backend.c | 6 +++---
 hw/block/virtio-blk.c | 4 
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index f425b00793..384e52d564 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -60,7 +60,7 @@ struct BlockBackend {
  * can be used to restore those options in the new BDS on insert) */
 BlockBackendRootState root_state;
 
-bool enable_write_cache;
+bool enable_write_cache; /* Atomic */
 
 /* I/O stats (display with "info blockstats"). */
 BlockAcctStats stats;
@@ -1972,13 +1972,13 @@ bool blk_is_sg(BlockBackend *blk)
 bool blk_enable_write_cache(BlockBackend *blk)
 {
 IO_CODE();
-return blk->enable_write_cache;
+return qatomic_read(>enable_write_cache);
 }
 
 void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
 {
 GLOBAL_STATE_CODE();
-blk->enable_write_cache = wce;
+qatomic_set(>enable_write_cache, wce);
 }
 
 void blk_activate(BlockBackend *blk, Error **errp)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8d0590cc76..191f75ce25 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -988,9 +988,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const 
uint8_t *config)
 
 memcpy(, config, s->config_size);
 
-aio_context_acquire(blk_get_aio_context(s->blk));
 blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
-aio_context_release(blk_get_aio_context(s->blk));
 }
 
 static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,
@@ -1058,11 +1056,9 @@ static void virtio_blk_set_status(VirtIODevice *vdev, 
uint8_t status)
  * s->blk would erroneously be placed in writethrough mode.
  */
 if (!virtio_vdev_has_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE)) {
-aio_context_acquire(blk_get_aio_context(s->blk));
 blk_set_enable_write_cache(s->blk,
virtio_vdev_has_feature(vdev,
VIRTIO_BLK_F_WCE));
-aio_context_release(blk_get_aio_context(s->blk));
 }
 }
 
-- 
2.31.1