* [PATCH v2 1/5] blk-mq: Export blk_mq_sched_restart_hctx()
2017-04-03 23:22 [PATCH v2 0/5] Avoid that scsi-mq queue processing stalls Bart Van Assche
@ 2017-04-03 23:22 ` Bart Van Assche
2017-04-04 6:40 ` Christoph Hellwig
2017-04-03 23:22 ` [PATCH v2 2/5] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list Bart Van Assche
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2017-04-03 23:22 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Martin K . Petersen, James Bottomley,
Bart Van Assche, Christoph Hellwig, Hannes Reinecke
Since a later patch will add a call to this function from the
SCSI core, export this function. Move the BLK_MQ_S_SCHED_RESTART
bit test from blk_mq_sched_restart_hctx() into the callers of
this function. This leads to some code duplication but that will
be addressed in another patch in this series.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
block/blk-mq-sched.c | 17 +++++++++--------
include/linux/blk-mq.h | 1 +
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 09af8ff18719..414ed4b3d266 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -317,14 +317,13 @@ static bool blk_mq_sched_bypass_insert(struct blk_mq_hw_ctx *hctx,
return true;
}
-static void blk_mq_sched_restart_hctx(struct blk_mq_hw_ctx *hctx)
+void blk_mq_sched_restart_hctx(struct blk_mq_hw_ctx *hctx)
{
- if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) {
- clear_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state);
- if (blk_mq_hctx_has_pending(hctx))
- blk_mq_run_hw_queue(hctx, true);
- }
+ clear_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state);
+ if (blk_mq_hctx_has_pending(hctx))
+ blk_mq_run_hw_queue(hctx, true);
}
+EXPORT_SYMBOL(blk_mq_sched_restart_hctx);
void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx *hctx)
{
@@ -334,9 +333,11 @@ void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx *hctx)
if (test_bit(QUEUE_FLAG_RESTART, &q->queue_flags)) {
if (test_and_clear_bit(QUEUE_FLAG_RESTART, &q->queue_flags)) {
queue_for_each_hw_ctx(q, hctx, i)
- blk_mq_sched_restart_hctx(hctx);
+ if (test_bit(BLK_MQ_S_SCHED_RESTART,
+ &hctx->state))
+ blk_mq_sched_restart_hctx(hctx);
}
- } else {
+ } else if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) {
blk_mq_sched_restart_hctx(hctx);
}
}
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index ea2e9dcd3aef..f62f3ce2dc65 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -237,6 +237,7 @@ void blk_mq_stop_hw_queues(struct request_queue *q);
void blk_mq_start_hw_queues(struct request_queue *q);
void blk_mq_start_stopped_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);
+void blk_mq_sched_restart_hctx(struct blk_mq_hw_ctx *hctx);
void blk_mq_run_hw_queues(struct request_queue *q, bool async);
void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs);
void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
--
2.12.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/5] blk-mq: Export blk_mq_sched_restart_hctx()
2017-04-03 23:22 ` [PATCH v2 1/5] blk-mq: Export blk_mq_sched_restart_hctx() Bart Van Assche
@ 2017-04-04 6:40 ` Christoph Hellwig
0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-04-04 6:40 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Christoph Hellwig, Hannes Reinecke
> +void blk_mq_sched_restart_hctx(struct blk_mq_hw_ctx *hctx)
> {
> + clear_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state);
> + if (blk_mq_hctx_has_pending(hctx))
> + blk_mq_run_hw_queue(hctx, true);
> }
> +EXPORT_SYMBOL(blk_mq_sched_restart_hctx);
_GPL export like the other _hctx functions, please.
Otherwise looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 2/5] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list
2017-04-03 23:22 [PATCH v2 0/5] Avoid that scsi-mq queue processing stalls Bart Van Assche
2017-04-03 23:22 ` [PATCH v2 1/5] blk-mq: Export blk_mq_sched_restart_hctx() Bart Van Assche
@ 2017-04-03 23:22 ` Bart Van Assche
2017-04-04 6:40 ` Christoph Hellwig
2017-04-03 23:22 ` [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx Bart Van Assche
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2017-04-03 23:22 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Martin K . Petersen, James Bottomley,
Bart Van Assche, Christoph Hellwig, Hannes Reinecke
A later patch in this series will namely use RCU to iterate over
this list.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
block/blk-mq.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 061fc2cc88d3..8fb983e6e2e4 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2093,6 +2093,8 @@ static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, bool shared)
{
struct request_queue *q;
+ lockdep_assert_held(&set->tag_list_lock);
+
list_for_each_entry(q, &set->tag_list, tag_set_list) {
blk_mq_freeze_queue(q);
queue_set_hctx_shared(q, shared);
@@ -2113,6 +2115,8 @@ static void blk_mq_del_queue_tag_set(struct request_queue *q)
blk_mq_update_tag_set_depth(set, false);
}
mutex_unlock(&set->tag_list_lock);
+
+ synchronize_rcu();
}
static void blk_mq_add_queue_tag_set(struct blk_mq_tag_set *set,
@@ -2618,6 +2622,8 @@ void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
{
struct request_queue *q;
+ lockdep_assert_held(&set->tag_list_lock);
+
if (nr_hw_queues > nr_cpu_ids)
nr_hw_queues = nr_cpu_ids;
if (nr_hw_queues < 1 || nr_hw_queues == set->nr_hw_queues)
--
2.12.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/5] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list
2017-04-03 23:22 ` [PATCH v2 2/5] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list Bart Van Assche
@ 2017-04-04 6:40 ` Christoph Hellwig
2017-04-04 15:55 ` Bart Van Assche
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2017-04-04 6:40 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Christoph Hellwig, Hannes Reinecke
On Mon, Apr 03, 2017 at 04:22:25PM -0700, Bart Van Assche wrote:
> A later patch in this series will namely use RCU to iterate over
> this list.
It also adds a couple lockdep_assert_held calls, which might be worth
mentionining.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/5] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list
2017-04-04 6:40 ` Christoph Hellwig
@ 2017-04-04 15:55 ` Bart Van Assche
0 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2017-04-04 15:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Hannes Reinecke
On 04/03/2017 11:40 PM, Christoph Hellwig wrote:=0A=
> On Mon, Apr 03, 2017 at 04:22:25PM -0700, Bart Van Assche wrote:=0A=
>> A later patch in this series will namely use RCU to iterate over=0A=
>> this list.=0A=
> =0A=
> It also adds a couple lockdep_assert_held calls, which might be worth=0A=
> mentioning.=0A=
> =0A=
> Otherwise looks good:=0A=
> =0A=
> Reviewed-by: Christoph Hellwig <hch@lst.de>=0A=
=0A=
Thanks for the review. I will update the patch description.=0A=
=0A=
Bart.=0A=
=0A=
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx
2017-04-03 23:22 [PATCH v2 0/5] Avoid that scsi-mq queue processing stalls Bart Van Assche
2017-04-03 23:22 ` [PATCH v2 1/5] blk-mq: Export blk_mq_sched_restart_hctx() Bart Van Assche
2017-04-03 23:22 ` [PATCH v2 2/5] blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list Bart Van Assche
@ 2017-04-03 23:22 ` Bart Van Assche
2017-04-04 6:42 ` Christoph Hellwig
2017-04-05 20:41 ` Omar Sandoval
2017-04-03 23:22 ` [PATCH v2 4/5] scsi: Add scsi_restart_hctx() Bart Van Assche
2017-04-03 23:22 ` [PATCH v2 5/5] scsi: Ensure that scsi_run_queue() runs all hardware queues Bart Van Assche
4 siblings, 2 replies; 17+ messages in thread
From: Bart Van Assche @ 2017-04-03 23:22 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Martin K . Petersen, James Bottomley,
Bart Van Assche, Christoph Hellwig, Hannes Reinecke
If a tag set is shared among multiple hardware queues, leave
it to the block driver to rerun hardware queues. Hence remove
QUEUE_FLAG_RESTART and introduce blk_mq_ops.restart_hctx.
Remove blk_mq_sched_mark_restart_queue() because this
function has no callers.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
block/blk-mq-sched.c | 15 ++++-----------
block/blk-mq-sched.h | 14 --------------
include/linux/blk-mq.h | 7 +++++++
include/linux/blkdev.h | 1 -
4 files changed, 11 insertions(+), 26 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 414ed4b3d266..f0c691a3ef9e 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -328,18 +328,11 @@ EXPORT_SYMBOL(blk_mq_sched_restart_hctx);
void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx *hctx)
{
struct request_queue *q = hctx->queue;
- unsigned int i;
-
- if (test_bit(QUEUE_FLAG_RESTART, &q->queue_flags)) {
- if (test_and_clear_bit(QUEUE_FLAG_RESTART, &q->queue_flags)) {
- queue_for_each_hw_ctx(q, hctx, i)
- if (test_bit(BLK_MQ_S_SCHED_RESTART,
- &hctx->state))
- blk_mq_sched_restart_hctx(hctx);
- }
- } else if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) {
+
+ if (q->mq_ops->restart_hctx)
+ q->mq_ops->restart_hctx(q, hctx);
+ else if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state))
blk_mq_sched_restart_hctx(hctx);
- }
}
/*
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index a75b16b123f7..fe62b1eccf4c 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -131,20 +131,6 @@ static inline void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx)
set_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state);
}
-/*
- * Mark a hardware queue and the request queue it belongs to as needing a
- * restart.
- */
-static inline void blk_mq_sched_mark_restart_queue(struct blk_mq_hw_ctx *hctx)
-{
- struct request_queue *q = hctx->queue;
-
- if (!test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state))
- set_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state);
- if (!test_bit(QUEUE_FLAG_RESTART, &q->queue_flags))
- set_bit(QUEUE_FLAG_RESTART, &q->queue_flags);
-}
-
static inline bool blk_mq_sched_needs_restart(struct blk_mq_hw_ctx *hctx)
{
return test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index f62f3ce2dc65..ebeea36ff9cd 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -86,6 +86,7 @@ struct blk_mq_queue_data {
};
typedef int (queue_rq_fn)(struct blk_mq_hw_ctx *, const struct blk_mq_queue_data *);
+typedef void (restart_fn)(struct request_queue *q, struct blk_mq_hw_ctx *hctx);
typedef enum blk_eh_timer_return (timeout_fn)(struct request *, bool);
typedef int (init_hctx_fn)(struct blk_mq_hw_ctx *, void *, unsigned int);
typedef void (exit_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
@@ -109,6 +110,12 @@ struct blk_mq_ops {
queue_rq_fn *queue_rq;
/*
+ * Called upon request completion to rerun all hctxs that share a tag
+ * set with the hctx for which a request finished.
+ */
+ restart_fn *restart_hctx;
+
+ /*
* Called on request timeout
*/
timeout_fn *timeout;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index a2dc6b390d48..a80543ec8be7 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -615,7 +615,6 @@ struct request_queue {
#define QUEUE_FLAG_FLUSH_NQ 25 /* flush not queueuable */
#define QUEUE_FLAG_DAX 26 /* device supports DAX */
#define QUEUE_FLAG_STATS 27 /* track rq completion times */
-#define QUEUE_FLAG_RESTART 28 /* queue needs restart at completion */
#define QUEUE_FLAG_POLL_STATS 29 /* collecting stats for hybrid polling */
#define QUEUE_FLAG_REGISTERED 30 /* queue has been registered to a disk */
--
2.12.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx
2017-04-03 23:22 ` [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx Bart Van Assche
@ 2017-04-04 6:42 ` Christoph Hellwig
2017-04-05 20:41 ` Omar Sandoval
1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-04-04 6:42 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Christoph Hellwig, Hannes Reinecke
On Mon, Apr 03, 2017 at 04:22:26PM -0700, Bart Van Assche wrote:
> If a tag set is shared among multiple hardware queues, leave
> it to the block driver to rerun hardware queues. Hence remove
> QUEUE_FLAG_RESTART and introduce blk_mq_ops.restart_hctx.
> Remove blk_mq_sched_mark_restart_queue() because this
> function has no callers.
This looks fine, but I think it needs to also actually implement at
least a dummy restart_hctx method for SCSI and NVMe to keep the
existing functionality.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx
2017-04-03 23:22 ` [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx Bart Van Assche
2017-04-04 6:42 ` Christoph Hellwig
@ 2017-04-05 20:41 ` Omar Sandoval
2017-04-05 20:51 ` Bart Van Assche
1 sibling, 1 reply; 17+ messages in thread
From: Omar Sandoval @ 2017-04-05 20:41 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Christoph Hellwig, Hannes Reinecke
On Mon, Apr 03, 2017 at 04:22:26PM -0700, Bart Van Assche wrote:
> If a tag set is shared among multiple hardware queues, leave
> it to the block driver to rerun hardware queues. Hence remove
> QUEUE_FLAG_RESTART and introduce blk_mq_ops.restart_hctx.
> Remove blk_mq_sched_mark_restart_queue() because this
> function has no callers.
Hi, Bart,
Kyber uses blk_mq_sched_mark_restart_queue() and the QUEUE_FLAG_RESTART
bit. If it's not too much trouble, it'd make things easier for me if you
left it in place. If it's a pain, it's fine if you get rid of it, I can
reintroduce it in my series.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx
2017-04-05 20:41 ` Omar Sandoval
@ 2017-04-05 20:51 ` Bart Van Assche
2017-04-05 20:54 ` Omar Sandoval
0 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2017-04-05 20:51 UTC (permalink / raw)
To: osandov; +Cc: hch, James.Bottomley, linux-block, hare, martin.petersen, axboe
On Wed, 2017-04-05 at 13:41 -0700, Omar Sandoval wrote:
> On Mon, Apr 03, 2017 at 04:22:26PM -0700, Bart Van Assche wrote:
> > If a tag set is shared among multiple hardware queues, leave
> > it to the block driver to rerun hardware queues. Hence remove
> > QUEUE_FLAG_RESTART and introduce blk_mq_ops.restart_hctx.
> > Remove blk_mq_sched_mark_restart_queue() because this
> > function has no callers.
>=20
> Kyber uses blk_mq_sched_mark_restart_queue() and the QUEUE_FLAG_RESTART
> bit. If it's not too much trouble, it'd make things easier for me if you
> left it in place. If it's a pain, it's fine if you get rid of it, I can
> reintroduce it in my series.
Hello Omar,
Would it be OK for you to reintroduce blk_mq_sched_mark_restart_queue()?
Since that function does not yet have any users I can't test any changes
I make to that function ...
Thanks,
Bart.=
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx
2017-04-05 20:51 ` Bart Van Assche
@ 2017-04-05 20:54 ` Omar Sandoval
0 siblings, 0 replies; 17+ messages in thread
From: Omar Sandoval @ 2017-04-05 20:54 UTC (permalink / raw)
To: Bart Van Assche
Cc: hch, James.Bottomley, linux-block, hare, martin.petersen, axboe
On Wed, Apr 05, 2017 at 08:51:51PM +0000, Bart Van Assche wrote:
> On Wed, 2017-04-05 at 13:41 -0700, Omar Sandoval wrote:
> > On Mon, Apr 03, 2017 at 04:22:26PM -0700, Bart Van Assche wrote:
> > > If a tag set is shared among multiple hardware queues, leave
> > > it to the block driver to rerun hardware queues. Hence remove
> > > QUEUE_FLAG_RESTART and introduce blk_mq_ops.restart_hctx.
> > > Remove blk_mq_sched_mark_restart_queue() because this
> > > function has no callers.
> >
> > Kyber uses blk_mq_sched_mark_restart_queue() and the QUEUE_FLAG_RESTART
> > bit. If it's not too much trouble, it'd make things easier for me if you
> > left it in place. If it's a pain, it's fine if you get rid of it, I can
> > reintroduce it in my series.
>
> Hello Omar,
>
> Would it be OK for you to reintroduce blk_mq_sched_mark_restart_queue()?
> Since that function does not yet have any users I can't test any changes
> I make to that function ...
>
> Thanks,
>
> Bart.
Yeah, that's fine.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 4/5] scsi: Add scsi_restart_hctx()
2017-04-03 23:22 [PATCH v2 0/5] Avoid that scsi-mq queue processing stalls Bart Van Assche
` (2 preceding siblings ...)
2017-04-03 23:22 ` [PATCH v2 3/5] blk-mq: Introduce blk_mq_ops.restart_hctx Bart Van Assche
@ 2017-04-03 23:22 ` Bart Van Assche
2017-04-04 6:42 ` Christoph Hellwig
2017-04-03 23:22 ` [PATCH v2 5/5] scsi: Ensure that scsi_run_queue() runs all hardware queues Bart Van Assche
4 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2017-04-03 23:22 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Martin K . Petersen, James Bottomley,
Bart Van Assche, Christoph Hellwig, Hannes Reinecke
This patch avoids that if multiple SCSI devices are associated with
a SCSI host that a queue can get stuck if scsi_queue_rq() returns
"busy".
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/scsi_lib.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c1519660824b..0e240aebc150 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -555,6 +555,21 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
scsi_run_queue(sdev->request_queue);
}
+static void scsi_restart_hctx(struct request_queue *q,
+ struct blk_mq_hw_ctx *hctx)
+{
+ struct blk_mq_tags *tags = hctx->tags;
+ struct blk_mq_tag_set *set = q->tag_set;
+ int i;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(q, &set->tag_list, tag_set_list)
+ queue_for_each_hw_ctx(q, hctx, i)
+ if (hctx->tags == tags)
+ blk_mq_sched_restart_hctx(hctx);
+ rcu_read_unlock();
+}
+
static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
{
if (!blk_rq_is_passthrough(cmd->request)) {
@@ -2156,6 +2171,7 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
static const struct blk_mq_ops scsi_mq_ops = {
.queue_rq = scsi_queue_rq,
+ .restart_hctx = scsi_restart_hctx,
.complete = scsi_softirq_done,
.timeout = scsi_timeout,
.init_request = scsi_init_request,
--
2.12.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] scsi: Add scsi_restart_hctx()
2017-04-03 23:22 ` [PATCH v2 4/5] scsi: Add scsi_restart_hctx() Bart Van Assche
@ 2017-04-04 6:42 ` Christoph Hellwig
2017-04-04 15:56 ` Bart Van Assche
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2017-04-04 6:42 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Christoph Hellwig, Hannes Reinecke
> +static void scsi_restart_hctx(struct request_queue *q,
> + struct blk_mq_hw_ctx *hctx)
> +{
> + struct blk_mq_tags *tags = hctx->tags;
> + struct blk_mq_tag_set *set = q->tag_set;
> + int i;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(q, &set->tag_list, tag_set_list)
> + queue_for_each_hw_ctx(q, hctx, i)
> + if (hctx->tags == tags)
> + blk_mq_sched_restart_hctx(hctx);
> + rcu_read_unlock();
> +}
This looks like generic block layer code, why is it in SCSI?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] scsi: Add scsi_restart_hctx()
2017-04-04 6:42 ` Christoph Hellwig
@ 2017-04-04 15:56 ` Bart Van Assche
2017-04-05 6:20 ` Christoph Hellwig
0 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2017-04-04 15:56 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Hannes Reinecke
On 04/03/2017 11:42 PM, Christoph Hellwig wrote:=0A=
>> +static void scsi_restart_hctx(struct request_queue *q,=0A=
>> + struct blk_mq_hw_ctx *hctx)=0A=
>> +{=0A=
>> + struct blk_mq_tags *tags =3D hctx->tags;=0A=
>> + struct blk_mq_tag_set *set =3D q->tag_set;=0A=
>> + int i;=0A=
>> +=0A=
>> + rcu_read_lock();=0A=
>> + list_for_each_entry_rcu(q, &set->tag_list, tag_set_list)=0A=
>> + queue_for_each_hw_ctx(q, hctx, i)=0A=
>> + if (hctx->tags =3D=3D tags)=0A=
>> + blk_mq_sched_restart_hctx(hctx);=0A=
>> + rcu_read_unlock();=0A=
>> +}=0A=
> =0A=
> This looks like generic block layer code, why is it in SCSI?=0A=
=0A=
Hello Christoph,=0A=
=0A=
That's an excellent question. I assume that you are fine with moving=0A=
this code to the block layer?=0A=
=0A=
Bart.=0A=
=0A=
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] scsi: Add scsi_restart_hctx()
2017-04-04 15:56 ` Bart Van Assche
@ 2017-04-05 6:20 ` Christoph Hellwig
2017-04-05 17:36 ` Bart Van Assche
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2017-04-05 6:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: Christoph Hellwig, Jens Axboe, linux-block, Martin K . Petersen,
James Bottomley, Hannes Reinecke
On Tue, Apr 04, 2017 at 03:56:34PM +0000, Bart Van Assche wrote:
> > This looks like generic block layer code, why is it in SCSI?
>
> Hello Christoph,
>
> That's an excellent question. I assume that you are fine with moving
> this code to the block layer?
Yes. In fact I wonder if we need the blk_mq_ops method at all now
that you're using RCU locking and don't need the scsi host_lock.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] scsi: Add scsi_restart_hctx()
2017-04-05 6:20 ` Christoph Hellwig
@ 2017-04-05 17:36 ` Bart Van Assche
0 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2017-04-05 17:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Martin K . Petersen, James Bottomley,
Hannes Reinecke
On 04/04/2017 11:20 PM, Christoph Hellwig wrote:
> On Tue, Apr 04, 2017 at 03:56:34PM +0000, Bart Van Assche wrote:
>>> This looks like generic block layer code, why is it in SCSI?
>>
>> That's an excellent question. I assume that you are fine with moving
>> this code to the block layer?
>
> Yes. In fact I wonder if we need the blk_mq_ops method at all now
> that you're using RCU locking and don't need the scsi host_lock.
Hello Christoph,
My plan is to remove the new blk_mq_ops method again before I post v3 of
this patch series.
Bart.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 5/5] scsi: Ensure that scsi_run_queue() runs all hardware queues
2017-04-03 23:22 [PATCH v2 0/5] Avoid that scsi-mq queue processing stalls Bart Van Assche
` (3 preceding siblings ...)
2017-04-03 23:22 ` [PATCH v2 4/5] scsi: Add scsi_restart_hctx() Bart Van Assche
@ 2017-04-03 23:22 ` Bart Van Assche
4 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2017-04-03 23:22 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Martin K . Petersen, James Bottomley,
Bart Van Assche, Christoph Hellwig
commit 52d7f1b5c2f3 ("blk-mq: Avoid that requeueing starts stopped
queues") removed the blk_mq_stop_hw_queue() call from scsi_queue_rq()
for the BLK_MQ_RQ_QUEUE_BUSY case. blk_mq_start_stopped_hw_queues()
only runs queues that had been stopped. Hence change the
blk_mq_start_stopped_hw_queues() call in scsi_run_queue() into
blk_mq_run_hw_queues(). Remove the blk_mq_start_stopped_hw_queues()
call from scsi_end_request() because __blk_mq_finish_request()
already runs all hardware queues if needed.
Fixes: commit 52d7f1b5c2f3 ("blk-mq: Avoid that requeueing starts stopped queues")
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/scsi_lib.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0e240aebc150..4459b18f62a1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -496,7 +496,7 @@ static void scsi_run_queue(struct request_queue *q)
scsi_starved_list_run(sdev->host);
if (q->mq_ops)
- blk_mq_start_stopped_hw_queues(q, false);
+ blk_mq_run_hw_queues(q, false);
else
blk_run_queue(q);
}
@@ -681,8 +681,6 @@ static bool scsi_end_request(struct request *req, int error,
if (scsi_target(sdev)->single_lun ||
!list_empty(&sdev->host->starved_list))
kblockd_schedule_work(&sdev->requeue_work);
- else
- blk_mq_start_stopped_hw_queues(q, true);
} else {
unsigned long flags;
--
2.12.0
^ permalink raw reply related [flat|nested] 17+ messages in thread