Re: [PATCH V5 3/4] blk-mq: clear stale request in tags->rq[] before freeing one request pool

From: Bart Van Assche <bvanassche@acm.org>
To: Ming Lei <ming.lei@redhat.com>, Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org, Hannes Reinecke <hare@suse.de>,
	John Garry <john.garry@huawei.com>,
	David Jeffery <djeffery@redhat.com>
Subject: Re: [PATCH V5 3/4] blk-mq: clear stale request in tags->rq[] before freeing one request pool
Date: Thu, 6 May 2021 08:02:53 -0700	[thread overview]
Message-ID: <db8a5f29-46b6-42c3-72c0-7ac70509e5d6@acm.org> (raw)
In-Reply-To: <20210505145855.174127-4-ming.lei@redhat.com>

On 5/5/21 7:58 AM, Ming Lei wrote:
> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index 4a40d409f5dd..8b239dcce85f 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -203,9 +203,14 @@ static struct request *blk_mq_find_and_get_req(struct blk_mq_tags *tags,
>  		unsigned int bitnr)
>  {
>  	struct request *rq = tags->rqs[bitnr];
> +	unsigned long flags;
>  
> -	if (!rq || !refcount_inc_not_zero(&rq->ref))
> +	spin_lock_irqsave(&tags->lock, flags);
> +	if (!rq || !refcount_inc_not_zero(&rq->ref)) {
> +		spin_unlock_irqrestore(&tags->lock, flags);
>  		return NULL;
> +	}
> +	spin_unlock_irqrestore(&tags->lock, flags);
>  	return rq;
>  }
Has it been considered to change the body of the if-statement into the
following?

		rq = NULL;

That change would reduce the number of return statements from two to one.

Thanks,

Bart.