FAILED: patch "[PATCH] block: mq-deadline: Fix dd_finish_request() for zoned devices" failed to apply to 5.15-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Possible dependencies:
2820e5d0820a ("block: mq-deadline: Fix dd_finish_request() for zoned devices")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 2820e5d0820ac4daedff1272616a53d9c7682fd2 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <[email protected]>
Date: Thu, 24 Nov 2022 11:12:07 +0900
Subject: [PATCH] block: mq-deadline: Fix dd_finish_request() for zoned devices
dd_finish_request() tests if the per prio fifo_list is not empty to determine if request dispatching must be restarted for handling blocked write requests to zoned devices with a call to blk_mq_sched_mark_restart_hctx(). While simple, this implementation has 2 problems:
1) Only the priority level of the completed request is considered. However, writes to a zone may be blocked due to other writes to the same zone using a different priority level. While this is unlikely to happen in practice, as writing a zone with different IO priorirites does not make sense, nothing in the code prevents this from happening.
2) The use of list_empty() is dangerous as dd_finish_request() does not take dd->lock and may run concurrently with the insert and dispatch code.
Fix these 2 problems by testing the write fifo list of all priority levels using the new helper dd_has_write_work(), and by testing each fifo list using list_empty_careful().
block/mq-deadline.c +15 -2
@@ -789,6 +789,18 @@ static void dd_prepare_request(struct request *rq) rq->elv.priv[0 ] =NULL ; } +static bool dd_has_write_work (struct blk_mq_hw_ctx *hctx) +{ +struct deadline_data *dd = hctx->queue->elevator->elevator_data; +enum dd_prio p; + +for (p =0 ; p <=DD_PRIO_MAX ; p++) +if (!list_empty_careful (&dd->per_prio[p].fifo_list[DD_WRITE ])) +return true; + +return false; +} +/* * Callback from inside blk_mq_free_request(). *
@@ -828,9 +840,10 @@ static void dd_finish_request(struct request *rq)spin_lock_irqsave (&dd->zone_lock, flags);blk_req_zone_write_unlock (rq); -if (!list_empty (&per_prio->fifo_list[DD_WRITE ])) -blk_mq_sched_mark_restart_hctx (rq->mq_hctx);spin_unlock_irqrestore (&dd->zone_lock, flags); + +if (dd_has_write_work (rq->mq_hctx)) +blk_mq_sched_mark_restart_hctx (rq->mq_hctx); } }
PATCH 1/1 block: mq-deadline: Fix dd_finish_request() for zoned devices
commit 2820e5d0820ac4daedff1272616a53d9c7682fd2 upstream.
dd_finish_request() tests if the per prio fifo_list is not empty to determine if request dispatching must be restarted for handling blocked write requests to zoned devices with a call to blk_mq_sched_mark_restart_hctx(). While simple, this implementation has 2 problems:
1) Only the priority level of the completed request is considered. However, writes to a zone may be blocked due to other writes to the same zone using a different priority level. While this is unlikely to happen in practice, as writing a zone with different IO priorirites does not make sense, nothing in the code prevents this from happening.
2) The use of list_empty() is dangerous as dd_finish_request() does not take dd->lock and may run concurrently with the insert and dispatch code.
Fix these 2 problems by testing the write fifo list of all priority levels using the new helper dd_has_write_work(), and by testing each fifo list using list_empty_careful().
--- block/mq-deadline.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
block/mq-deadline.c +15 -3
@@ -733,6 +733,18 @@ static void dd_prepare_request(struct request *rq) rq->elv.priv[0 ] =NULL ; } +static bool dd_has_write_work (struct blk_mq_hw_ctx *hctx) +{ +struct deadline_data *dd = hctx->queue->elevator->elevator_data; +enum dd_prio p; + +for (p =0 ; p <=DD_PRIO_MAX ; p++) +if (!list_empty_careful (&dd->per_prio[p].fifo_list[DD_WRITE ])) +return true; + +return false; +} +/* * Callback from inside blk_mq_free_request(). *
@@ -755,7 +767,6 @@ static void dd_finish_request(struct request *rq)struct deadline_data *dd = q->elevator->elevator_data;const u8 ioprio_class =dd_rq_ioclass (rq);const enum dd_prio prio = ioprio_class_to_prio[ioprio_class]; -struct dd_per_prio *per_prio = &dd->per_prio[prio];/* * The block layer core may call dd_finish_request() without having
@@ -771,9 +782,10 @@ static void dd_finish_request(struct request *rq)spin_lock_irqsave (&dd->zone_lock, flags);blk_req_zone_write_unlock (rq); -if (!list_empty (&per_prio->fifo_list[DD_WRITE ])) -blk_mq_sched_mark_restart_hctx (rq->mq_hctx);spin_unlock_irqrestore (&dd->zone_lock, flags); + +if (dd_has_write_work (rq->mq_hctx)) +blk_mq_sched_mark_restart_hctx (rq->mq_hctx); } }
Signature · 1 line
2.39.0
Greg Kroah-Hartman
maintainerRe: [PATCH] block: mq-deadline: Fix dd_finish_request() for zoned devices
Quoted 28 lines · Damien Le Moal
commit
2820e5d0820ac4daedff1272616a53d9c7682fd2upstream.
dd_finish_request()tests if the per priofifo_listis not empty to determine if request dispatching must be restarted for handling blocked write requests to zoned devices with a call toblk_mq_sched_mark_restart_hctx(). While simple, this implementation has 2 problems:1) Only the priority level of the completed request is considered. However, writes to a zone may be blocked due to other writes to the same zone using a different priority level. While this is unlikely to happen in practice, as writing a zone with different IO priorirites does not make sense, nothing in the code prevents this from happening.
2) The use oflist_empty()is dangerous asdd_finish_request()does not takedd->lockand may run concurrently with the insert and dispatch code.Fix these 2 problems by testing the write fifo list of all priority levels using the new helper
dd_has_write_work(), and by testing each fifo list usinglist_empty_careful().Fixes c807ab520fc3 ("block/mq-deadline: Add I/O priority support")Cc <[email protected]>Signed-off-by Damien Le Moal <[email protected]>Reviewed-by Johannes Thumshirn <[email protected]>Signed-off-by Jens Axboe <[email protected]>
Quoted 3 lines
--- block/mq-deadline.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
Now queued up, thanks.
greg k-h
Quoted 5 lines · On 1/4/23 23:21, [email protected] wrote:
The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Greg,
I sent you a backport in reply to your email.
Thanks !
Quoted 80 lines
Possible dependencies:
2820e5d0820a("block: mq-deadline: Fixdd_finish_request()for zoned devices")thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From
2820e5d0820ac4daedff1272616a53d9c7682fd2Mon Sep 17 00:00:00 2001
From: Damien Le Moal <[email protected]>
Date: Thu, 24 Nov 2022 11:12:07 +0900
Subject: [PATCH] block: mq-deadline: Fixdd_finish_request()for zoned devices
dd_finish_request()tests if the per priofifo_listis not empty to determine if request dispatching must be restarted for handling blocked write requests to zoned devices with a call toblk_mq_sched_mark_restart_hctx(). While simple, this implementation has 2 problems:1) Only the priority level of the completed request is considered. However, writes to a zone may be blocked due to other writes to the same zone using a different priority level. While this is unlikely to happen in practice, as writing a zone with different IO priorirites does not make sense, nothing in the code prevents this from happening.
2) The use oflist_empty()is dangerous asdd_finish_request()does not takedd->lockand may run concurrently with the insert and dispatch code.Fix these 2 problems by testing the write fifo list of all priority levels using the new helper
dd_has_write_work(), and by testing each fifo list usinglist_empty_careful().Fixes c807ab520fc3 ("block/mq-deadline: Add I/O priority support")Cc <[email protected]>Signed-off-by Damien Le Moal <[email protected]>Reviewed-by Johannes Thumshirn <[email protected]>Signed-off-by Jens Axboe <[email protected]>block/mq-deadline.c +15
-2@@ -789,6 +789,18 @@ static void dd_prepare_request(struct request *rq) rq->elv.priv[0 ] =NULL ; } +static bool dd_has_write_work (struct blk_mq_hw_ctx *hctx) +{ +struct deadline_data *dd = hctx->queue->elevator->elevator_data; +enum dd_prio p; + +for (p =0 ; p <=DD_PRIO_MAX ; p++) +if (!list_empty_careful (&dd->per_prio[p].fifo_list[DD_WRITE ])) +return true; + +return false; +} +/* * Callback from inside blk_mq_free_request(). * @@ -828,9 +840,10 @@ static void dd_finish_request(struct request *rq)spin_lock_irqsave (&dd->zone_lock, flags);blk_req_zone_write_unlock (rq); -if (!list_empty (&per_prio->fifo_list[DD_WRITE ])) -blk_mq_sched_mark_restart_hctx (rq->mq_hctx);spin_unlock_irqrestore (&dd->zone_lock, flags); + +if (dd_has_write_work (rq->mq_hctx)) +blk_mq_sched_mark_restart_hctx (rq->mq_hctx); } }
Signature · 2 lines
Damien Le Moal Western Digital Research