Re: [PATCH] block: fix request.queuelist usage in flush

[Chengming Zhou]

On 2024/6/7 12:55, Christoph Hellwig wrote:
> On Fri, Jun 07, 2024 at 10:37:58AM +0800, Chengming Zhou wrote:
>> Yeah, right, it seems LVM may create this special request that only has
>> PREFLUSH | POSTFLUSH without any DATA, goes into the flush state machine.
>> Then, cause the request double list_add_tail() without list_del_init().
>> I don't know the reason behind it, but well, it's allowable in the current
>> flush code.
> 
> PREFLUSH | POSTFLUSH is a weird invalid format.  We'll need to fix this
> in dm, and probably also catch it in the block layer submission path.
> 

Right, how about add WARN here to catch it? Or just set it to PREFLUSH?
Not familiar with dm code, need help if we need to fix it in dm. :)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index c17cf8ed8113..3ce9ed78c375 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -185,7 +185,7 @@ static void blk_flush_complete_seq(struct request *rq,
                /* queue for flush */
                if (list_empty(pending))
                        fq->flush_pending_since = jiffies;
-               list_move_tail(&rq->queuelist, pending);
+               list_add_tail(&rq->queuelist, pending);
                break;

        case REQ_FSEQ_DATA:
@@ -263,6 +263,7 @@ static enum rq_end_io_ret flush_end_io(struct request *flush_rq,
                unsigned int seq = blk_flush_cur_seq(rq);

                BUG_ON(seq != REQ_FSEQ_PREFLUSH && seq != REQ_FSEQ_POSTFLUSH);
+               list_del_init(&rq->queuelist);
                blk_flush_complete_seq(rq, fq, seq, error);
        }

@@ -402,6 +403,12 @@ bool blk_insert_flush(struct request *rq)
        unsigned int policy = blk_flush_policy(fflags, rq);
        struct blk_flush_queue *fq = blk_get_flush_queue(q, rq->mq_ctx);

+       /*
+        * PREFLUSH | POSTFLUSH is a weird invalid format,
+        * need to fix in the upper layer, catch it here.
+        */
+       WARN_ON_ONCE(policy == (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH));
+
        /* FLUSH/FUA request must never be merged */
        WARN_ON_ONCE(rq->bio != rq->biotail);