Re: [PATCH V2 2/3] blk-mq: complete request locally if the completion is from tagset iterator

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

On Mon, Apr 26, 2021 at 07:30:51PM -0700, Bart Van Assche wrote:
> On 4/26/21 6:45 PM, Ming Lei wrote:
> > diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> > index 100fa44d52a6..773aea4db90c 100644
> > --- a/block/blk-mq-tag.c
> > +++ b/block/blk-mq-tag.c
> > @@ -284,8 +284,11 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
> >  	if ((iter_data->flags & BT_TAG_ITER_STARTED) &&
> >  	    !blk_mq_request_started(rq))
> >  		ret = true;
> > -	else
> > +	else {
> > +		rq->rq_flags |= RQF_ITERATING;
> >  		ret = iter_data->fn(rq, iter_data->data, reserved);
> > +		rq->rq_flags &= ~RQF_ITERATING;
> > +	}
> >  	if (!iter_static_rqs)
> >  		blk_mq_put_rq_ref(rq);
> >  	return ret;
> 
> All existing rq->rq_flags modifications are serialized. The above change
> adds code that may change rq_flags concurrently with regular request
> processing. I think that counts as a race condition.

Good catch, but we still can change .rq_flags via atomic op, such as:

	do {
		old = rq->rq_flags;
		new = old | RQF_ITERATING;
	} while (cmpxchg(&rq->rq_flags, old, new) != old);

> Additionally, the
> RQF_ITERATING flag won't be set correctly in the (unlikely) case that
> two concurrent bt_tags_iter() calls examine the same request at the same
> time.

If the driver completes request from two concurrent bt_tags_iter(), there has
been big trouble of double completion, so I'd rather not to consider this case.


Thanks,
Ming