* [PATCH] mq-deadline: Make sure to always unlock zones
@ 2018-02-28 17:35 Bart Van Assche
2018-03-01 1:36 ` Ming Lei
2018-03-01 15:39 ` Jens Axboe
0 siblings, 2 replies; 3+ messages in thread
From: Bart Van Assche @ 2018-02-28 17:35 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Damien Le Moal, Bart Van Assche,
Hannes Reinecke, Ming Lei
From: Damien Le Moal <damien.lemoal@wdc.com>
In case of a failed write request (all retries failed) and when using
libata, the SCSI error handler calls scsi_finish_command(). In the
case of blk-mq this means that scsi_mq_done() does not get called,
that blk_mq_complete_request() does not get called and also that the
mq-deadline .completed_request() method is not called. This results in
the target zone of the failed write request being left in a locked
state, preventing that any new write requests are issued to the same
zone.
Fix this by replacing the .completed_request() method with the
.finish_request() method as this method is always called whether or
not a request completes successfully. Since the .finish_request()
method is only called by the blk-mq core if a .prepare_request()
method exists, add a dummy .prepare_request() method.
Fixes: 5700f69178e9 ("mq-deadline: Introduce zone locking support")
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
[ bvanassche: edited patch description ]
Cc: Hannes Reinecke <hare@suse.com>
Cc: Ming Lei <ming.lei@redhat.com>
---
block/mq-deadline.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index c56f211c8440..8ec0ba9f5386 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -535,13 +535,22 @@ static void dd_insert_requests(struct blk_mq_hw_ctx *hctx,
spin_unlock(&dd->lock);
}
+/*
+ * Nothing to do here. This is defined only to ensure that .finish_request
+ * method is called upon request completion.
+ */
+static void dd_prepare_request(struct request *rq, struct bio *bio)
+{
+}
+
/*
* For zoned block devices, write unlock the target zone of
* completed write requests. Do this while holding the zone lock
* spinlock so that the zone is never unlocked while deadline_fifo_request()
- * while deadline_next_request() are executing.
+ * or deadline_next_request() are executing. This function is called for
+ * all requests, whether or not these requests complete successfully.
*/
-static void dd_completed_request(struct request *rq)
+static void dd_finish_request(struct request *rq)
{
struct request_queue *q = rq->q;
@@ -756,7 +765,8 @@ static struct elevator_type mq_deadline = {
.ops.mq = {
.insert_requests = dd_insert_requests,
.dispatch_request = dd_dispatch_request,
- .completed_request = dd_completed_request,
+ .prepare_request = dd_prepare_request,
+ .finish_request = dd_finish_request,
.next_request = elv_rb_latter_request,
.former_request = elv_rb_former_request,
.bio_merge = dd_bio_merge,
--
2.16.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mq-deadline: Make sure to always unlock zones
2018-02-28 17:35 [PATCH] mq-deadline: Make sure to always unlock zones Bart Van Assche
@ 2018-03-01 1:36 ` Ming Lei
2018-03-01 15:39 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Ming Lei @ 2018-03-01 1:36 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Damien Le Moal,
Hannes Reinecke
On Wed, Feb 28, 2018 at 09:35:29AM -0800, Bart Van Assche wrote:
> From: Damien Le Moal <damien.lemoal@wdc.com>
>
> In case of a failed write request (all retries failed) and when using
> libata, the SCSI error handler calls scsi_finish_command(). In the
> case of blk-mq this means that scsi_mq_done() does not get called,
> that blk_mq_complete_request() does not get called and also that the
> mq-deadline .completed_request() method is not called. This results in
> the target zone of the failed write request being left in a locked
> state, preventing that any new write requests are issued to the same
> zone.
>
> Fix this by replacing the .completed_request() method with the
> .finish_request() method as this method is always called whether or
> not a request completes successfully. Since the .finish_request()
> method is only called by the blk-mq core if a .prepare_request()
> method exists, add a dummy .prepare_request() method.
>
> Fixes: 5700f69178e9 ("mq-deadline: Introduce zone locking support")
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> [ bvanassche: edited patch description ]
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> ---
> block/mq-deadline.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> index c56f211c8440..8ec0ba9f5386 100644
> --- a/block/mq-deadline.c
> +++ b/block/mq-deadline.c
> @@ -535,13 +535,22 @@ static void dd_insert_requests(struct blk_mq_hw_ctx *hctx,
> spin_unlock(&dd->lock);
> }
>
> +/*
> + * Nothing to do here. This is defined only to ensure that .finish_request
> + * method is called upon request completion.
> + */
> +static void dd_prepare_request(struct request *rq, struct bio *bio)
> +{
> +}
> +
> /*
> * For zoned block devices, write unlock the target zone of
> * completed write requests. Do this while holding the zone lock
> * spinlock so that the zone is never unlocked while deadline_fifo_request()
> - * while deadline_next_request() are executing.
> + * or deadline_next_request() are executing. This function is called for
> + * all requests, whether or not these requests complete successfully.
> */
> -static void dd_completed_request(struct request *rq)
> +static void dd_finish_request(struct request *rq)
> {
> struct request_queue *q = rq->q;
>
> @@ -756,7 +765,8 @@ static struct elevator_type mq_deadline = {
> .ops.mq = {
> .insert_requests = dd_insert_requests,
> .dispatch_request = dd_dispatch_request,
> - .completed_request = dd_completed_request,
> + .prepare_request = dd_prepare_request,
> + .finish_request = dd_finish_request,
> .next_request = elv_rb_latter_request,
> .former_request = elv_rb_former_request,
> .bio_merge = dd_bio_merge,
> --
> 2.16.2
>
Looks fine:
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mq-deadline: Make sure to always unlock zones
2018-02-28 17:35 [PATCH] mq-deadline: Make sure to always unlock zones Bart Van Assche
2018-03-01 1:36 ` Ming Lei
@ 2018-03-01 15:39 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2018-03-01 15:39 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-block, Christoph Hellwig, Damien Le Moal, Hannes Reinecke,
Ming Lei
On 2/28/18 10:35 AM, Bart Van Assche wrote:
> From: Damien Le Moal <damien.lemoal@wdc.com>
>
> In case of a failed write request (all retries failed) and when using
> libata, the SCSI error handler calls scsi_finish_command(). In the
> case of blk-mq this means that scsi_mq_done() does not get called,
> that blk_mq_complete_request() does not get called and also that the
> mq-deadline .completed_request() method is not called. This results in
> the target zone of the failed write request being left in a locked
> state, preventing that any new write requests are issued to the same
> zone.
>
> Fix this by replacing the .completed_request() method with the
> .finish_request() method as this method is always called whether or
> not a request completes successfully. Since the .finish_request()
> method is only called by the blk-mq core if a .prepare_request()
> method exists, add a dummy .prepare_request() method.
Thanks, applied for 4.16.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-01 15:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-28 17:35 [PATCH] mq-deadline: Make sure to always unlock zones Bart Van Assche
2018-03-01 1:36 ` Ming Lei
2018-03-01 15:39 ` Jens Axboe
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.