[RFC PATCH] block: defer flush request no matter whether we have elevator

[RFC PATCH] block: defer flush request no matter whether we have elevator

[Yufen Yu]

Commit 7520872c0cf4 ("block: don't defer flushes on blk-mq + scheduling")
tried to fix deadlock for cycled wait between flush requests and data
request into flush_data_in_flight. The former holded all driver tags
and wait for data request completion, but the latter can not complete
for waiting free driver tags.

After commit 923218f6166a ("blk-mq: don't allocate driver tag upfront
for flush rq"), flush requests will not get driver tag before queuing
into flush queue.

* With elevator, flush request just get sched_tags before inserting
  flush queue. It will not get driver tag until issue them to driver.
  data request on list fq->flush_data_in_flight will complete in
  the end.

* Without elevator, each flush request will get a driver tag when
  allocate request. Then data request on fq->flush_data_in_flight
  don't worry about lacking driver tag.

In both of these cases, cycled wait cannot be true. So we may allow
to defer flush request.

Signed-off-by: Yufen Yu <yuyufen@huawei.com>
---
 block/blk-flush.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 15ae0155ec07..24c208d21793 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -286,13 +286,8 @@ static void blk_kick_flush(struct request_queue *q, struct blk_flush_queue *fq,
 	if (fq->flush_pending_idx != fq->flush_running_idx || list_empty(pending))
 		return;
 
-	/* C2 and C3
-	 *
-	 * For blk-mq + scheduling, we can risk having all driver tags
-	 * assigned to empty flushes, and we deadlock if we are expecting
-	 * other requests to make progress. Don't defer for that case.
-	 */
-	if (!list_empty(&fq->flush_data_in_flight) && q->elevator &&
+	/* C2 and C3 */
+	if (!list_empty(&fq->flush_data_in_flight) &&
 	    time_before(jiffies,
 			fq->flush_pending_since + FLUSH_PENDING_TIMEOUT))
 		return;
-- 
2.25.4

Re: [RFC PATCH] block: defer flush request no matter whether we have elevator

[Ming Lei]

On Thu, Jul 16, 2020 at 02:52:01AM -0400, Yufen Yu wrote:
> Commit 7520872c0cf4 ("block: don't defer flushes on blk-mq + scheduling")
> tried to fix deadlock for cycled wait between flush requests and data
> request into flush_data_in_flight. The former holded all driver tags
> and wait for data request completion, but the latter can not complete
> for waiting free driver tags.
> 
> After commit 923218f6166a ("blk-mq: don't allocate driver tag upfront
> for flush rq"), flush requests will not get driver tag before queuing
> into flush queue.
> 
> * With elevator, flush request just get sched_tags before inserting
>   flush queue. It will not get driver tag until issue them to driver.
>   data request on list fq->flush_data_in_flight will complete in
>   the end.
> 
> * Without elevator, each flush request will get a driver tag when
>   allocate request. Then data request on fq->flush_data_in_flight
>   don't worry about lacking driver tag.
> 
> In both of these cases, cycled wait cannot be true. So we may allow
> to defer flush request.
> 
> Signed-off-by: Yufen Yu <yuyufen@huawei.com>
> ---
>  block/blk-flush.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/block/blk-flush.c b/block/blk-flush.c
> index 15ae0155ec07..24c208d21793 100644
> --- a/block/blk-flush.c
> +++ b/block/blk-flush.c
> @@ -286,13 +286,8 @@ static void blk_kick_flush(struct request_queue *q, struct blk_flush_queue *fq,
>  	if (fq->flush_pending_idx != fq->flush_running_idx || list_empty(pending))
>  		return;
>  
> -	/* C2 and C3
> -	 *
> -	 * For blk-mq + scheduling, we can risk having all driver tags
> -	 * assigned to empty flushes, and we deadlock if we are expecting
> -	 * other requests to make progress. Don't defer for that case.
> -	 */
> -	if (!list_empty(&fq->flush_data_in_flight) && q->elevator &&
> +	/* C2 and C3 */
> +	if (!list_empty(&fq->flush_data_in_flight) &&
>  	    time_before(jiffies,
>  			fq->flush_pending_since + FLUSH_PENDING_TIMEOUT))
>  		return;
> -- 
> 2.25.4
> 

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

Re: [RFC PATCH] block: defer flush request no matter whether we have elevator

[Jens Axboe]

On 7/16/20 12:52 AM, Yufen Yu wrote:
> Commit 7520872c0cf4 ("block: don't defer flushes on blk-mq + scheduling")
> tried to fix deadlock for cycled wait between flush requests and data
> request into flush_data_in_flight. The former holded all driver tags
> and wait for data request completion, but the latter can not complete
> for waiting free driver tags.
> 
> After commit 923218f6166a ("blk-mq: don't allocate driver tag upfront
> for flush rq"), flush requests will not get driver tag before queuing
> into flush queue.
> 
> * With elevator, flush request just get sched_tags before inserting
>   flush queue. It will not get driver tag until issue them to driver.
>   data request on list fq->flush_data_in_flight will complete in
>   the end.
> 
> * Without elevator, each flush request will get a driver tag when
>   allocate request. Then data request on fq->flush_data_in_flight
>   don't worry about lacking driver tag.
> 
> In both of these cases, cycled wait cannot be true. So we may allow
> to defer flush request.

Applied, thanks.

-- 
Jens Axboe