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

[Ming Lei <ming.lei@redhat.com>]

On Thu, May 06, 2021 at 06:11:59PM -0700, Bart Van Assche wrote:
> 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;
> >   }
> 
> Shouldn't the 'rq = tags->rqs[bitnr]' assignment be protected by tags->lock
> too? Otherwise a request pointer could be read before the request pointer
> clearing happens and the refcount_inc_not_zero() call could happen after the
> request clearing.

Right, will fix it.

-- 
Ming