Re: [PATCH 1/7] blk-mq: factor out a blk_rq_init_flush helper

[Damien Le Moal <dlemoal@kernel.org>]

On 4/17/23 05:09, Christoph Hellwig wrote:
> Factor out a helper from blk_insert_flush that initializes the flush
> machine related fields in struct request.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-flush.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/block/blk-flush.c b/block/blk-flush.c
> index 04698ed9bcd4a9..422a6d5446d1c5 100644
> --- a/block/blk-flush.c
> +++ b/block/blk-flush.c
> @@ -376,6 +376,15 @@ static enum rq_end_io_ret mq_flush_data_end_io(struct request *rq,
>  	return RQ_END_IO_NONE;
>  }
>  
> +static void blk_rq_init_flush(struct request *rq)
> +{
> +	memset(&rq->flush, 0, sizeof(rq->flush));
> +	INIT_LIST_HEAD(&rq->flush.list);
> +	rq->rq_flags |= RQF_FLUSH_SEQ;
> +	rq->flush.saved_end_io = rq->end_io; /* Usually NULL */
> +	rq->end_io = mq_flush_data_end_io;
> +}

struct flush is:

struct {
	unsigned int		seq;
	struct list_head	list;
	rq_end_io_fn		*saved_end_io;
} flush;

So given that list and saved_end_io are initialized here, we could remove the
memset() by initializing seq "manually":

+static void blk_rq_init_flush(struct request *rq)
+{
+	rq->flush.seq = 0;
+	INIT_LIST_HEAD(&rq->flush.list);
+	rq->flush.saved_end_io = rq->end_io; /* Usually NULL */
+	rq->rq_flags |= RQF_FLUSH_SEQ;
+	rq->end_io = mq_flush_data_end_io;
+}

No ?

Otherwise, look good to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> +
>  /**
>   * blk_insert_flush - insert a new PREFLUSH/FUA request
>   * @rq: request to insert
> @@ -437,13 +446,7 @@ void blk_insert_flush(struct request *rq)
>  	 * @rq should go through flush machinery.  Mark it part of flush
>  	 * sequence and submit for further processing.
>  	 */
> -	memset(&rq->flush, 0, sizeof(rq->flush));
> -	INIT_LIST_HEAD(&rq->flush.list);
> -	rq->rq_flags |= RQF_FLUSH_SEQ;
> -	rq->flush.saved_end_io = rq->end_io; /* Usually NULL */
> -
> -	rq->end_io = mq_flush_data_end_io;
> -
> +	blk_rq_init_flush(rq);
>  	spin_lock_irq(&fq->mq_flush_lock);
>  	blk_flush_complete_seq(rq, fq, REQ_FSEQ_ACTIONS & ~policy, 0);
>  	spin_unlock_irq(&fq->mq_flush_lock);