* [PATCH 0/3] scsi: fixes for targets with many LUNs
@ 2023-06-02 16:38 mwilck
2023-06-02 16:38 ` [PATCH 1/3] bsg: increase number of devices mwilck
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: mwilck @ 2023-06-02 16:38 UTC (permalink / raw)
To: Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block,
Hannes Reinecke, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
This patch series addresses some issues we saw in a test setup
with a large number of SCSI LUNs. The first two patches simply
increase the number of available sg and bsg devices. The last one
fixes an large delay we encountered between blocking a Fibre Channel
remote port and the dev_loss_tmo.
Hannes Reinecke (3):
bsg: increase number of devices
scsi: sg: increase number of devices
scsi: simplify scsi_stop_queue()
block/bsg.c | 2 +-
drivers/scsi/scsi_lib.c | 29 +++++++++++++++--------------
drivers/scsi/sg.c | 2 +-
3 files changed, 17 insertions(+), 16 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] bsg: increase number of devices
2023-06-02 16:38 [PATCH 0/3] scsi: fixes for targets with many LUNs mwilck
@ 2023-06-02 16:38 ` mwilck
2023-06-02 16:38 ` [PATCH 2/3] scsi: sg: " mwilck
2023-06-02 16:38 ` [PATCH 3/3] scsi: simplify scsi_stop_queue() mwilck
2 siblings, 0 replies; 7+ messages in thread
From: mwilck @ 2023-06-02 16:38 UTC (permalink / raw)
To: Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block,
Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
Larger setups may need to allocate more than 32k bsg devices, so
increase the number of devices to the full range of minor device
numbers.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
block/bsg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/bsg.c b/block/bsg.c
index 7eca43f33d7f..c53f24243bf2 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -36,7 +36,7 @@ static inline struct bsg_device *to_bsg_device(struct inode *inode)
}
#define BSG_DEFAULT_CMDS 64
-#define BSG_MAX_DEVS 32768
+#define BSG_MAX_DEVS (1 << MINORBITS)
static DEFINE_IDA(bsg_minor_ida);
static struct class *bsg_class;
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] scsi: sg: increase number of devices
2023-06-02 16:38 [PATCH 0/3] scsi: fixes for targets with many LUNs mwilck
2023-06-02 16:38 ` [PATCH 1/3] bsg: increase number of devices mwilck
@ 2023-06-02 16:38 ` mwilck
2023-06-04 1:45 ` Douglas Gilbert
2023-06-02 16:38 ` [PATCH 3/3] scsi: simplify scsi_stop_queue() mwilck
2 siblings, 1 reply; 7+ messages in thread
From: mwilck @ 2023-06-02 16:38 UTC (permalink / raw)
To: Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block,
Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
Larger setups may need to allocate more than 32k sg devices, so
increase the number of devices to the full range of minor device
numbers.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/sg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 037f8c98a6d3..6c04cf941dac 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -71,7 +71,7 @@ static int sg_proc_init(void);
#define SG_ALLOW_DIO_DEF 0
-#define SG_MAX_DEVS 32768
+#define SG_MAX_DEVS (1 << MINORBITS)
/* SG_MAX_CDB_SIZE should be 260 (spc4r37 section 3.1.30) however the type
* of sg_io_hdr::cmd_len can only represent 255. All SCSI commands greater
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] scsi: simplify scsi_stop_queue()
2023-06-02 16:38 [PATCH 0/3] scsi: fixes for targets with many LUNs mwilck
2023-06-02 16:38 ` [PATCH 1/3] bsg: increase number of devices mwilck
2023-06-02 16:38 ` [PATCH 2/3] scsi: sg: " mwilck
@ 2023-06-02 16:38 ` mwilck
2023-06-04 13:54 ` Bart Van Assche
2 siblings, 1 reply; 7+ messages in thread
From: mwilck @ 2023-06-02 16:38 UTC (permalink / raw)
To: Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block,
Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
scsi_target_block() calls scsi_stop_queue() for each scsi_device
and scsi_stop_queue() calls blk_mq_wait_quiesce_done() for each LUN.
As blk_mq_wait_quiesce_done() comes down to synchronize_rcu() for
SCSI queues, this can cause substantial delay for scsi_target_block()
on a target with a lot of logical units (we measured more than 100s
delay for blocking a FC rport with 2048 LUNs).
Simplify scsi_stop_queue(), which is only called in this code path, to
never wait for the quiescing to finish. Rather call synchronize_rcu()
from scsi_target_block() after iterating over all devices.
Also, move the call to scsi_stop_queue() in scsi_internal_device_block()
out of the code section where the state_mutex is held.
This patch uses the same basic idea as commit f983622ae605 ("scsi: core:
Avoid calling synchronize_rcu() for each device in scsi_host_block()").
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
drivers/scsi/scsi_lib.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b7c569a42aa4..ed5bc9e1dbed 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2725,24 +2725,18 @@ void scsi_start_queue(struct scsi_device *sdev)
blk_mq_unquiesce_queue(sdev->request_queue);
}
-static void scsi_stop_queue(struct scsi_device *sdev, bool nowait)
+static void scsi_stop_queue(struct scsi_device *sdev)
{
/*
* The atomic variable of ->queue_stopped covers that
* blk_mq_quiesce_queue* is balanced with blk_mq_unquiesce_queue.
*
* However, we still need to wait until quiesce is done
- * in case that queue has been stopped.
+ * in case that queue has been stopped. This is done in
+ * scsi_target_block() for all devices of the target.
*/
- if (!cmpxchg(&sdev->queue_stopped, 0, 1)) {
- if (nowait)
- blk_mq_quiesce_queue_nowait(sdev->request_queue);
- else
- blk_mq_quiesce_queue(sdev->request_queue);
- } else {
- if (!nowait)
- blk_mq_wait_quiesce_done(sdev->request_queue->tag_set);
- }
+ if (!cmpxchg(&sdev->queue_stopped, 0, 1))
+ blk_mq_quiesce_queue_nowait(sdev->request_queue);
}
/**
@@ -2769,7 +2763,7 @@ int scsi_internal_device_block_nowait(struct scsi_device *sdev)
* request queue.
*/
if (!ret)
- scsi_stop_queue(sdev, true);
+ scsi_stop_queue(sdev);
return ret;
}
EXPORT_SYMBOL_GPL(scsi_internal_device_block_nowait);
@@ -2795,9 +2789,9 @@ static int scsi_internal_device_block(struct scsi_device *sdev)
mutex_lock(&sdev->state_mutex);
err = __scsi_internal_device_block_nowait(sdev);
- if (err == 0)
- scsi_stop_queue(sdev, false);
mutex_unlock(&sdev->state_mutex);
+ if (err == 0)
+ scsi_stop_queue(sdev);
return err;
}
@@ -2910,6 +2904,13 @@ scsi_target_block(struct device *dev)
device_block);
else
device_for_each_child(dev, NULL, target_block);
+
+ /*
+ * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag,
+ * so blk_mq_wait_quiesce_done() comes down to just synchronize_rcu().
+ * Just calling it once is enough.
+ */
+ synchronize_rcu();
}
EXPORT_SYMBOL_GPL(scsi_target_block);
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] scsi: sg: increase number of devices
2023-06-02 16:38 ` [PATCH 2/3] scsi: sg: " mwilck
@ 2023-06-04 1:45 ` Douglas Gilbert
0 siblings, 0 replies; 7+ messages in thread
From: Douglas Gilbert @ 2023-06-04 1:45 UTC (permalink / raw)
To: mwilck, Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block,
Hannes Reinecke
On 2023-06-02 12:38, mwilck@suse.com wrote:
> From: Hannes Reinecke <hare@suse.de>
>
> Larger setups may need to allocate more than 32k sg devices, so
> increase the number of devices to the full range of minor device
> numbers.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Thanks.
> ---
> drivers/scsi/sg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 037f8c98a6d3..6c04cf941dac 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -71,7 +71,7 @@ static int sg_proc_init(void);
>
> #define SG_ALLOW_DIO_DEF 0
>
> -#define SG_MAX_DEVS 32768
> +#define SG_MAX_DEVS (1 << MINORBITS)
>
> /* SG_MAX_CDB_SIZE should be 260 (spc4r37 section 3.1.30) however the type
> * of sg_io_hdr::cmd_len can only represent 255. All SCSI commands greater
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] scsi: simplify scsi_stop_queue()
2023-06-02 16:38 ` [PATCH 3/3] scsi: simplify scsi_stop_queue() mwilck
@ 2023-06-04 13:54 ` Bart Van Assche
2023-06-05 7:31 ` Hannes Reinecke
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2023-06-04 13:54 UTC (permalink / raw)
To: mwilck, Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block,
Hannes Reinecke
On 6/2/23 09:38, mwilck@suse.com wrote:
> @@ -2910,6 +2904,13 @@ scsi_target_block(struct device *dev)
> device_block);
> else
> device_for_each_child(dev, NULL, target_block);
> +
> + /*
> + * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag,
> + * so blk_mq_wait_quiesce_done() comes down to just synchronize_rcu().
> + * Just calling it once is enough.
> + */
> + synchronize_rcu();
> }
> EXPORT_SYMBOL_GPL(scsi_target_block);
The above comment is wrong. See also commit b125bb99559e ("scsi:
core: Support setting BLK_MQ_F_BLOCKING").
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] scsi: simplify scsi_stop_queue()
2023-06-04 13:54 ` Bart Van Assche
@ 2023-06-05 7:31 ` Hannes Reinecke
0 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2023-06-05 7:31 UTC (permalink / raw)
To: Bart Van Assche, mwilck, Martin K. Petersen, Christoph Hellwig, Ming Lei
Cc: James Bottomley, Bart Van Assche, linux-scsi, linux-block
On 6/4/23 15:54, Bart Van Assche wrote:
> On 6/2/23 09:38, mwilck@suse.com wrote:
>> @@ -2910,6 +2904,13 @@ scsi_target_block(struct device *dev)
>> device_block);
>> else
>> device_for_each_child(dev, NULL, target_block);
>> +
>> + /*
>> + * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag,
>> + * so blk_mq_wait_quiesce_done() comes down to just
>> synchronize_rcu().
>> + * Just calling it once is enough.
>> + */
>> + synchronize_rcu();
>> }
>> EXPORT_SYMBOL_GPL(scsi_target_block);
>
> The above comment is wrong. See also commit b125bb99559e ("scsi:
> core: Support setting BLK_MQ_F_BLOCKING").
>
Well, this patch got written before your patchset has been posted.
But yeah, we'll be updating it.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-05 7:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-02 16:38 [PATCH 0/3] scsi: fixes for targets with many LUNs mwilck
2023-06-02 16:38 ` [PATCH 1/3] bsg: increase number of devices mwilck
2023-06-02 16:38 ` [PATCH 2/3] scsi: sg: " mwilck
2023-06-04 1:45 ` Douglas Gilbert
2023-06-02 16:38 ` [PATCH 3/3] scsi: simplify scsi_stop_queue() mwilck
2023-06-04 13:54 ` Bart Van Assche
2023-06-05 7:31 ` Hannes Reinecke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.