virtio-scsi: fix 100% CPU consumption in IOThread

[PATCH 6/6] virtio-scsi: move request-related items from .h to .c

Posted by Stefan Hajnoczi 3 years, 2 months ago

There is no longer a need to expose the request and related APIs in
virtio-scsi.h since there are no callers outside virtio-scsi.c.

Note the block comment in VirtIOSCSIReq has been adjusted to meet the
coding style.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/virtio/virtio-scsi.h | 40 -----------------------------
 hw/scsi/virtio-scsi.c           | 45 ++++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 2497530064..abdda2cbd0 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -94,42 +94,6 @@ struct VirtIOSCSI {
     uint32_t host_features;
 };
 
-typedef struct VirtIOSCSIReq {
-    /* Note:
-     * - fields up to resp_iov are initialized by virtio_scsi_init_req;
-     * - fields starting at vring are zeroed by virtio_scsi_init_req.
-     * */
-    VirtQueueElement elem;
-
-    VirtIOSCSI *dev;
-    VirtQueue *vq;
-    QEMUSGList qsgl;
-    QEMUIOVector resp_iov;
-
-    union {
-        /* Used for two-stage request submission */
-        QTAILQ_ENTRY(VirtIOSCSIReq) next;
-
-        /* Used for cancellation of request during TMFs */
-        int remaining;
-    };
-
-    SCSIRequest *sreq;
-    size_t resp_size;
-    enum SCSIXferMode mode;
-    union {
-        VirtIOSCSICmdResp     cmd;
-        VirtIOSCSICtrlTMFResp tmf;
-        VirtIOSCSICtrlANResp  an;
-        VirtIOSCSIEvent       event;
-    } resp;
-    union {
-        VirtIOSCSICmdReq      cmd;
-        VirtIOSCSICtrlTMFReq  tmf;
-        VirtIOSCSICtrlANReq   an;
-    } req;
-} VirtIOSCSIReq;
-
 static inline void virtio_scsi_acquire(VirtIOSCSI *s)
 {
     if (s->ctx) {
@@ -151,10 +115,6 @@ void virtio_scsi_common_realize(DeviceState *dev,
                                 Error **errp);
 
 void virtio_scsi_common_unrealize(DeviceState *dev);
-void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req);
-void virtio_scsi_free_req(VirtIOSCSIReq *req);
-void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
-                            uint32_t event, uint32_t reason);
 
 void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp);
 int virtio_scsi_dataplane_start(VirtIODevice *s);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 12c6a21202..db54d104be 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -29,6 +29,43 @@
 #include "hw/virtio/virtio-access.h"
 #include "trace.h"
 
+typedef struct VirtIOSCSIReq {
+    /*
+     * Note:
+     * - fields up to resp_iov are initialized by virtio_scsi_init_req;
+     * - fields starting at vring are zeroed by virtio_scsi_init_req.
+     */
+    VirtQueueElement elem;
+
+    VirtIOSCSI *dev;
+    VirtQueue *vq;
+    QEMUSGList qsgl;
+    QEMUIOVector resp_iov;
+
+    union {
+        /* Used for two-stage request submission */
+        QTAILQ_ENTRY(VirtIOSCSIReq) next;
+
+        /* Used for cancellation of request during TMFs */
+        int remaining;
+    };
+
+    SCSIRequest *sreq;
+    size_t resp_size;
+    enum SCSIXferMode mode;
+    union {
+        VirtIOSCSICmdResp     cmd;
+        VirtIOSCSICtrlTMFResp tmf;
+        VirtIOSCSICtrlANResp  an;
+        VirtIOSCSIEvent       event;
+    } resp;
+    union {
+        VirtIOSCSICmdReq      cmd;
+        VirtIOSCSICtrlTMFReq  tmf;
+        VirtIOSCSICtrlANReq   an;
+    } req;
+} VirtIOSCSIReq;
+
 static inline int virtio_scsi_get_lun(uint8_t *lun)
 {
     return ((lun[2] << 8) | lun[3]) & 0x3FFF;
@@ -45,7 +82,7 @@ static inline SCSIDevice *virtio_scsi_device_get(VirtIOSCSI *s, uint8_t *lun)
     return scsi_device_get(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
 }
 
-void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
+static void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
     const size_t zero_skip =
@@ -58,7 +95,7 @@ void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
     memset((uint8_t *)req + zero_skip, 0, sizeof(*req) - zero_skip);
 }
 
-void virtio_scsi_free_req(VirtIOSCSIReq *req)
+static void virtio_scsi_free_req(VirtIOSCSIReq *req)
 {
     qemu_iovec_destroy(&req->resp_iov);
     qemu_sglist_destroy(&req->qsgl);
@@ -801,8 +838,8 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
     s->events_dropped = false;
 }
 
-void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
-                            uint32_t event, uint32_t reason)
+static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
+                                   uint32_t event, uint32_t reason)
 {
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
     VirtIOSCSIReq *req;
-- 
2.35.1

Re: [PATCH 6/6] virtio-scsi: move request-related items from .h to .c

Posted by Paolo Bonzini 3 years, 2 months ago

On 4/27/22 16:35, Stefan Hajnoczi wrote:
> There is no longer a need to expose the request and related APIs in
> virtio-scsi.h since there are no callers outside virtio-scsi.c.
> 
> Note the block comment in VirtIOSCSIReq has been adjusted to meet the
> coding style.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   include/hw/virtio/virtio-scsi.h | 40 -----------------------------
>   hw/scsi/virtio-scsi.c           | 45 ++++++++++++++++++++++++++++++---
>   2 files changed, 41 insertions(+), 44 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 2497530064..abdda2cbd0 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -94,42 +94,6 @@ struct VirtIOSCSI {
>       uint32_t host_features;
>   };
>   
> -typedef struct VirtIOSCSIReq {
> -    /* Note:
> -     * - fields up to resp_iov are initialized by virtio_scsi_init_req;
> -     * - fields starting at vring are zeroed by virtio_scsi_init_req.
> -     * */
> -    VirtQueueElement elem;
> -
> -    VirtIOSCSI *dev;
> -    VirtQueue *vq;
> -    QEMUSGList qsgl;
> -    QEMUIOVector resp_iov;
> -
> -    union {
> -        /* Used for two-stage request submission */
> -        QTAILQ_ENTRY(VirtIOSCSIReq) next;
> -
> -        /* Used for cancellation of request during TMFs */
> -        int remaining;
> -    };
> -
> -    SCSIRequest *sreq;
> -    size_t resp_size;
> -    enum SCSIXferMode mode;
> -    union {
> -        VirtIOSCSICmdResp     cmd;
> -        VirtIOSCSICtrlTMFResp tmf;
> -        VirtIOSCSICtrlANResp  an;
> -        VirtIOSCSIEvent       event;
> -    } resp;
> -    union {
> -        VirtIOSCSICmdReq      cmd;
> -        VirtIOSCSICtrlTMFReq  tmf;
> -        VirtIOSCSICtrlANReq   an;
> -    } req;
> -} VirtIOSCSIReq;
> -
>   static inline void virtio_scsi_acquire(VirtIOSCSI *s)
>   {
>       if (s->ctx) {
> @@ -151,10 +115,6 @@ void virtio_scsi_common_realize(DeviceState *dev,
>                                   Error **errp);
>   
>   void virtio_scsi_common_unrealize(DeviceState *dev);
> -void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req);
> -void virtio_scsi_free_req(VirtIOSCSIReq *req);
> -void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
> -                            uint32_t event, uint32_t reason);
>   
>   void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp);
>   int virtio_scsi_dataplane_start(VirtIODevice *s);
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 12c6a21202..db54d104be 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -29,6 +29,43 @@
>   #include "hw/virtio/virtio-access.h"
>   #include "trace.h"
>   
> +typedef struct VirtIOSCSIReq {
> +    /*
> +     * Note:
> +     * - fields up to resp_iov are initialized by virtio_scsi_init_req;
> +     * - fields starting at vring are zeroed by virtio_scsi_init_req.
> +     */
> +    VirtQueueElement elem;
> +
> +    VirtIOSCSI *dev;
> +    VirtQueue *vq;
> +    QEMUSGList qsgl;
> +    QEMUIOVector resp_iov;
> +
> +    union {
> +        /* Used for two-stage request submission */
> +        QTAILQ_ENTRY(VirtIOSCSIReq) next;
> +
> +        /* Used for cancellation of request during TMFs */
> +        int remaining;
> +    };
> +
> +    SCSIRequest *sreq;
> +    size_t resp_size;
> +    enum SCSIXferMode mode;
> +    union {
> +        VirtIOSCSICmdResp     cmd;
> +        VirtIOSCSICtrlTMFResp tmf;
> +        VirtIOSCSICtrlANResp  an;
> +        VirtIOSCSIEvent       event;
> +    } resp;
> +    union {
> +        VirtIOSCSICmdReq      cmd;
> +        VirtIOSCSICtrlTMFReq  tmf;
> +        VirtIOSCSICtrlANReq   an;
> +    } req;
> +} VirtIOSCSIReq;
> +
>   static inline int virtio_scsi_get_lun(uint8_t *lun)
>   {
>       return ((lun[2] << 8) | lun[3]) & 0x3FFF;
> @@ -45,7 +82,7 @@ static inline SCSIDevice *virtio_scsi_device_get(VirtIOSCSI *s, uint8_t *lun)
>       return scsi_device_get(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
>   }
>   
> -void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
> +static void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
>   {
>       VirtIODevice *vdev = VIRTIO_DEVICE(s);
>       const size_t zero_skip =
> @@ -58,7 +95,7 @@ void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
>       memset((uint8_t *)req + zero_skip, 0, sizeof(*req) - zero_skip);
>   }
>   
> -void virtio_scsi_free_req(VirtIOSCSIReq *req)
> +static void virtio_scsi_free_req(VirtIOSCSIReq *req)
>   {
>       qemu_iovec_destroy(&req->resp_iov);
>       qemu_sglist_destroy(&req->qsgl);
> @@ -801,8 +838,8 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
>       s->events_dropped = false;
>   }
>   
> -void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
> -                            uint32_t event, uint32_t reason)
> +static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
> +                                   uint32_t event, uint32_t reason)
>   {
>       VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
>       VirtIOSCSIReq *req;

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

[PATCH 1/6] virtio-scsi: fix ctrl and event handler functions in dataplane mode
[PATCH 2/6] virtio-scsi: don't waste CPU polling the event virtqueue
[PATCH 3/6] virtio-scsi: clean up virtio_scsi_handle_event_vq()
[PATCH 4/6] virtio-scsi: clean up virtio_scsi_handle_ctrl_vq()
[PATCH 5/6] virtio-scsi: clean up virtio_scsi_handle_cmd_vq()
[PATCH 6/6] virtio-scsi: move request-related items from .h to .c