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
