| Message ID | 20200716065201.3213045-1-yuyufen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] block: defer flush request no matter whether we have elevator | expand |
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>
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.
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;
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(-)