Re: [PATCH 06/12] scsi: remove AioContext locking

2023-12-04 Thread Stefan Hajnoczi
On Mon, Dec 04, 2023 at 01:23:09PM +0100, Kevin Wolf wrote:
> Am 29.11.2023 um 20:55 hat Stefan Hajnoczi geschrieben:
> > The AioContext lock no longer has any effect. Remove it.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  include/hw/virtio/virtio-scsi.h | 14 --
> >  hw/scsi/scsi-bus.c  |  2 --
> >  hw/scsi/scsi-disk.c | 28 
> >  hw/scsi/virtio-scsi.c   | 18 --
> >  4 files changed, 4 insertions(+), 58 deletions(-)
> 
> > @@ -2531,13 +2527,11 @@ static void scsi_unrealize(SCSIDevice *dev)
> >  static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
> >  {
> >  SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> > -AioContext *ctx = NULL;
> > +
> >  /* can happen for devices without drive. The error message for missing
> >   * backend will be issued in scsi_realize
> >   */
> >  if (s->qdev.conf.blk) {
> > -ctx = blk_get_aio_context(s->qdev.conf.blk);
> > -aio_context_acquire(ctx);
> >  if (!blkconf_blocksizes(&s->qdev.conf, errp)) {
> >  goto out;
> >  }
> > @@ -2549,15 +2543,11 @@ static void scsi_hd_realize(SCSIDevice *dev, Error 
> > **errp)
> >  }
> >  scsi_realize(&s->qdev, errp);
> >  out:
> > -if (ctx) {
> > -aio_context_release(ctx);
> > -}
> >  }
> 
> This doesn't build for me:
> 
> ../hw/scsi/scsi-disk.c:2545:1: error: label at end of compound statement is a 
> C2x extension [-Werror,-Wc2x-extensions]
> }
> ^
> 1 error generated.

Will fix in v2. Thanks!

Stefan


signature.asc
Description: PGP signature


Re: [PATCH 06/12] scsi: remove AioContext locking

2023-12-04 Thread Kevin Wolf
Am 29.11.2023 um 20:55 hat Stefan Hajnoczi geschrieben:
> The AioContext lock no longer has any effect. Remove it.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  include/hw/virtio/virtio-scsi.h | 14 --
>  hw/scsi/scsi-bus.c  |  2 --
>  hw/scsi/scsi-disk.c | 28 
>  hw/scsi/virtio-scsi.c   | 18 --
>  4 files changed, 4 insertions(+), 58 deletions(-)

> @@ -2531,13 +2527,11 @@ static void scsi_unrealize(SCSIDevice *dev)
>  static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
>  {
>  SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> -AioContext *ctx = NULL;
> +
>  /* can happen for devices without drive. The error message for missing
>   * backend will be issued in scsi_realize
>   */
>  if (s->qdev.conf.blk) {
> -ctx = blk_get_aio_context(s->qdev.conf.blk);
> -aio_context_acquire(ctx);
>  if (!blkconf_blocksizes(&s->qdev.conf, errp)) {
>  goto out;
>  }
> @@ -2549,15 +2543,11 @@ static void scsi_hd_realize(SCSIDevice *dev, Error 
> **errp)
>  }
>  scsi_realize(&s->qdev, errp);
>  out:
> -if (ctx) {
> -aio_context_release(ctx);
> -}
>  }

This doesn't build for me:

../hw/scsi/scsi-disk.c:2545:1: error: label at end of compound statement is a 
C2x extension [-Werror,-Wc2x-extensions]
}
^
1 error generated.

Kevin




Re: [PATCH 06/12] scsi: remove AioContext locking

2023-11-30 Thread Eric Blake
On Wed, Nov 29, 2023 at 02:55:47PM -0500, Stefan Hajnoczi wrote:
> The AioContext lock no longer has any effect. Remove it.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  include/hw/virtio/virtio-scsi.h | 14 --
>  hw/scsi/scsi-bus.c  |  2 --
>  hw/scsi/scsi-disk.c | 28 
>  hw/scsi/virtio-scsi.c   | 18 --
>  4 files changed, 4 insertions(+), 58 deletions(-)
>

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




[PATCH 06/12] scsi: remove AioContext locking

2023-11-29 Thread Stefan Hajnoczi
The AioContext lock no longer has any effect. Remove it.

Signed-off-by: Stefan Hajnoczi 
---
 include/hw/virtio/virtio-scsi.h | 14 --
 hw/scsi/scsi-bus.c  |  2 --
 hw/scsi/scsi-disk.c | 28 
 hw/scsi/virtio-scsi.c   | 18 --
 4 files changed, 4 insertions(+), 58 deletions(-)

diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index da8cb928d9..7f0573b1bf 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -101,20 +101,6 @@ struct VirtIOSCSI {
 uint32_t host_features;
 };
 
-static inline void virtio_scsi_acquire(VirtIOSCSI *s)
-{
-if (s->ctx) {
-aio_context_acquire(s->ctx);
-}
-}
-
-static inline void virtio_scsi_release(VirtIOSCSI *s)
-{
-if (s->ctx) {
-aio_context_release(s->ctx);
-}
-}
-
 void virtio_scsi_common_realize(DeviceState *dev,
 VirtIOHandleOutput ctrl,
 VirtIOHandleOutput evt,
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index b8bfde9565..0031164cc3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1725,9 +1725,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev, 
SCSISense sense)
 {
 scsi_device_for_each_req_async(sdev, scsi_device_purge_one_req, NULL);
 
-aio_context_acquire(blk_get_aio_context(sdev->conf.blk));
 blk_drain(sdev->conf.blk);
-aio_context_release(blk_get_aio_context(sdev->conf.blk));
 scsi_device_set_ua(sdev, sense);
 }
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2c1bbb3530..f1bd5f5c6e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2325,14 +2325,10 @@ static void scsi_disk_reset(DeviceState *dev)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
 uint64_t nb_sectors;
-AioContext *ctx;
 
 scsi_device_purge_requests(&s->qdev, SENSE_CODE(RESET));
 
-ctx = blk_get_aio_context(s->qdev.conf.blk);
-aio_context_acquire(ctx);
 blk_get_geometry(s->qdev.conf.blk, &nb_sectors);
-aio_context_release(ctx);
 
 nb_sectors /= s->qdev.blocksize / BDRV_SECTOR_SIZE;
 if (nb_sectors) {
@@ -2531,13 +2527,11 @@ static void scsi_unrealize(SCSIDevice *dev)
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-AioContext *ctx = NULL;
+
 /* can happen for devices without drive. The error message for missing
  * backend will be issued in scsi_realize
  */
 if (s->qdev.conf.blk) {
-ctx = blk_get_aio_context(s->qdev.conf.blk);
-aio_context_acquire(ctx);
 if (!blkconf_blocksizes(&s->qdev.conf, errp)) {
 goto out;
 }
@@ -2549,15 +2543,11 @@ static void scsi_hd_realize(SCSIDevice *dev, Error 
**errp)
 }
 scsi_realize(&s->qdev, errp);
 out:
-if (ctx) {
-aio_context_release(ctx);
-}
 }
 
 static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-AioContext *ctx;
 int ret;
 uint32_t blocksize = 2048;
 
@@ -2573,8 +2563,6 @@ static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
 blocksize = dev->conf.physical_block_size;
 }
 
-ctx = blk_get_aio_context(dev->conf.blk);
-aio_context_acquire(ctx);
 s->qdev.blocksize = blocksize;
 s->qdev.type = TYPE_ROM;
 s->features |= 1 << SCSI_DISK_F_REMOVABLE;
@@ -2582,7 +2570,6 @@ static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
 s->product = g_strdup("QEMU CD-ROM");
 }
 scsi_realize(&s->qdev, errp);
-aio_context_release(ctx);
 }
 
 
@@ -2713,7 +2700,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;
 
@@ -2728,9 +2714,6 @@ 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);
-
 /* 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);
 if (rc < 0) {
@@ -2738,18 +2721,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 < 3) {
 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.
@@ -276