[PATCH 4/6] hw/scsi/virtio-scsi: Use RCU_READ macro

Philippe Mathieu-Daudé posted 6 patches 10 months, 1 week ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Greg Kurz <groug@kaod.org>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Fam Zheng <fam@euphon.net>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
[PATCH 4/6] hw/scsi/virtio-scsi: Use RCU_READ macro
Posted by Philippe Mathieu-Daudé 10 months, 1 week ago
Replace the manual rcu_read_(un)lock calls by the
WITH_RCU_READ_LOCK_GUARD macro (See commit ef46ae67ba
"docs/style: call out the use of GUARD macros").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/virtio-scsi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 690aceec45..998227404a 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -343,14 +343,14 @@ static void virtio_scsi_do_one_tmf_bh(VirtIOSCSIReq *req)
         target = req->req.tmf.lun[1];
         qatomic_inc(&s->resetting);
 
-        rcu_read_lock();
-        QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
-            SCSIDevice *d1 = SCSI_DEVICE(kid->child);
-            if (d1->channel == 0 && d1->id == target) {
-                device_cold_reset(&d1->qdev);
+        WITH_RCU_READ_LOCK_GUARD() {
+            QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
+                SCSIDevice *d1 = SCSI_DEVICE(kid->child);
+                if (d1->channel == 0 && d1->id == target) {
+                    device_cold_reset(&d1->qdev);
+                }
             }
         }
-        rcu_read_unlock();
 
         qatomic_dec(&s->resetting);
         break;
-- 
2.41.0


Re: [PATCH 4/6] hw/scsi/virtio-scsi: Use RCU_READ macro
Posted by Manos Pitsidianakis 10 months, 1 week ago
On Wed, 24 Jan 2024 09:41, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Replace the manual rcu_read_(un)lock calls by the
>WITH_RCU_READ_LOCK_GUARD macro (See commit ef46ae67ba
>"docs/style: call out the use of GUARD macros").
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/scsi/virtio-scsi.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>index 690aceec45..998227404a 100644
>--- a/hw/scsi/virtio-scsi.c
>+++ b/hw/scsi/virtio-scsi.c
>@@ -343,14 +343,14 @@ static void virtio_scsi_do_one_tmf_bh(VirtIOSCSIReq *req)
>         target = req->req.tmf.lun[1];
>         qatomic_inc(&s->resetting);
> 
>-        rcu_read_lock();
>-        QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
>-            SCSIDevice *d1 = SCSI_DEVICE(kid->child);
>-            if (d1->channel == 0 && d1->id == target) {
>-                device_cold_reset(&d1->qdev);
>+        WITH_RCU_READ_LOCK_GUARD() {
>+            QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
>+                SCSIDevice *d1 = SCSI_DEVICE(kid->child);
>+                if (d1->channel == 0 && d1->id == target) {
>+                    device_cold_reset(&d1->qdev);
>+                }
>             }
>         }
>-        rcu_read_unlock();
> 
>         qatomic_dec(&s->resetting);
>         break;
>-- 
>2.41.0
>

Unrelated to your patch: I just noticed in hw/scsi/virtio-scsi.c, 
s->resetting is used to flag whether the bus is resetting; but there's 
no check if a resetting is taking place before starting another. Is this 
single threaded code so it's not necessary?

As for the patch:

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Re: [PATCH 4/6] hw/scsi/virtio-scsi: Use RCU_READ macro
Posted by Philippe Mathieu-Daudé 10 months, 1 week ago
On 24/1/24 10:17, Manos Pitsidianakis wrote:
> On Wed, 24 Jan 2024 09:41, Philippe Mathieu-Daudé <philmd@linaro.org> 
> wrote:
>> Replace the manual rcu_read_(un)lock calls by the
>> WITH_RCU_READ_LOCK_GUARD macro (See commit ef46ae67ba
>> "docs/style: call out the use of GUARD macros").
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/scsi/virtio-scsi.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)

> Unrelated to your patch: I just noticed in hw/scsi/virtio-scsi.c, 
> s->resetting is used to flag whether the bus is resetting; but there's 
> no check if a resetting is taking place before starting another. Is this 
> single threaded code so it's not necessary?

No clue about this device, I'll let the maintainers answer you :)