2019-09-06 02:19:10

by Matt Lupfer

[permalink] [raw]
Subject: [PATCH v2] scsi: virtio_scsi: unplug LUNs when events missed

The event handler calls scsi_scan_host() when events are missed, which
will hotplug new LUNs. However, this function won't remove any
unplugged LUNs. The result is that hotunplug doesn't work properly when
the number of unplugged LUNs exceeds the event queue size (currently 8).

Scan existing LUNs when events are missed to check if they are still
present. If not, remove them.

Signed-off-by: Matt Lupfer <[email protected]>
---
drivers/scsi/virtio_scsi.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 297e1076e571..13a82b94b27b 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -30,6 +30,8 @@
#include <linux/seqlock.h>
#include <linux/blk-mq-virtio.h>

+#include "sd.h"
+
#define VIRTIO_SCSI_MEMPOOL_SZ 64
#define VIRTIO_SCSI_EVENT_LEN 8
#define VIRTIO_SCSI_VQ_BASE 2
@@ -324,6 +326,36 @@ static void virtscsi_handle_param_change(struct virtio_scsi *vscsi,
scsi_device_put(sdev);
}

+static void virtscsi_rescan_hotunplug(struct virtio_scsi *vscsi)
+{
+ struct scsi_device *sdev;
+ struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
+ unsigned char scsi_cmd[MAX_COMMAND_SIZE];
+ int result, inquiry_len, inq_result_len = 256;
+ char *inq_result = kmalloc(inq_result_len, GFP_KERNEL);
+
+ shost_for_each_device(sdev, shost) {
+ inquiry_len = sdev->inquiry_len ? sdev->inquiry_len : 36;
+
+ memset(scsi_cmd, 0, sizeof(scsi_cmd));
+ scsi_cmd[0] = INQUIRY;
+ scsi_cmd[4] = (unsigned char) inquiry_len;
+
+ memset(inq_result, 0, inq_result_len);
+
+ result = scsi_execute_req(sdev, scsi_cmd, DMA_FROM_DEVICE,
+ inq_result, inquiry_len, NULL,
+ SD_TIMEOUT, SD_MAX_RETRIES, NULL);
+
+ if (result == 0 && inq_result[0] >> 5) {
+ /* PQ indicates the LUN is not attached */
+ scsi_remove_device(sdev);
+ }
+ }
+
+ kfree(inq_result);
+}
+
static void virtscsi_handle_event(struct work_struct *work)
{
struct virtio_scsi_event_node *event_node =
@@ -335,6 +367,7 @@ static void virtscsi_handle_event(struct work_struct *work)
cpu_to_virtio32(vscsi->vdev, VIRTIO_SCSI_T_EVENTS_MISSED)) {
event->event &= ~cpu_to_virtio32(vscsi->vdev,
VIRTIO_SCSI_T_EVENTS_MISSED);
+ virtscsi_rescan_hotunplug(vscsi);
scsi_scan_host(virtio_scsi_host(vscsi->vdev));
}

--
2.23.0


2019-09-06 11:11:39

by Stefan Hajnoczi

[permalink] [raw]
Subject: Re: [PATCH v2] scsi: virtio_scsi: unplug LUNs when events missed

On Thu, Sep 05, 2019 at 06:19:28PM +0000, Matt Lupfer wrote:
> The event handler calls scsi_scan_host() when events are missed, which
> will hotplug new LUNs. However, this function won't remove any
> unplugged LUNs. The result is that hotunplug doesn't work properly when
> the number of unplugged LUNs exceeds the event queue size (currently 8).
>
> Scan existing LUNs when events are missed to check if they are still
> present. If not, remove them.
>
> Signed-off-by: Matt Lupfer <[email protected]>
> ---
> drivers/scsi/virtio_scsi.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)

Please include a changelog in future patch revisions. For example:

Signed-off-by: ...
---
v2:
* Replaced magic constants with sd.h constants [Michael]

Just C and virtio code review, no SCSI specifics:

Reviewed-by: Stefan Hajnoczi <[email protected]>


Attachments:
(No filename) (909.00 B)
signature.asc (499.00 B)
Download all attachments

2019-09-10 09:06:01

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2] scsi: virtio_scsi: unplug LUNs when events missed

On 06/09/19 10:54, Stefan Hajnoczi wrote:
> On Thu, Sep 05, 2019 at 06:19:28PM +0000, Matt Lupfer wrote:
>> The event handler calls scsi_scan_host() when events are missed, which
>> will hotplug new LUNs. However, this function won't remove any
>> unplugged LUNs. The result is that hotunplug doesn't work properly when
>> the number of unplugged LUNs exceeds the event queue size (currently 8).
>>
>> Scan existing LUNs when events are missed to check if they are still
>> present. If not, remove them.
>>
>> Signed-off-by: Matt Lupfer <[email protected]>
>> ---
>> drivers/scsi/virtio_scsi.c | 33 +++++++++++++++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>
> Please include a changelog in future patch revisions. For example:
>
> Signed-off-by: ...
> ---
> v2:
> * Replaced magic constants with sd.h constants [Michael]
>
> Just C and virtio code review, no SCSI specifics:
>
> Reviewed-by: Stefan Hajnoczi <[email protected]>
>

Acked-by: Paolo Bonzini <[email protected]>


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2019-09-11 02:17:05

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2] scsi: virtio_scsi: unplug LUNs when events missed


Matt,

> The event handler calls scsi_scan_host() when events are missed, which
> will hotplug new LUNs. However, this function won't remove any
> unplugged LUNs. The result is that hotunplug doesn't work properly
> when the number of unplugged LUNs exceeds the event queue size
> (currently 8).
>
> Scan existing LUNs when events are missed to check if they are still
> present. If not, remove them.

Applied to 5.4/scsi-queue, thanks!

--
Martin K. Petersen Oracle Linux Engineering