On Tue, Apr 27, 2021 at 03:06:51PM +0800, Ming Lei wrote:
> 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);
oops, this way can't work because other .rq_flags modification still may
clear RQF_ITERATING.
As I mentioned in another thread, blk-mq needn't to consider the double
completion[1] any more, which is covered by driver. blk-mq just needs to
make sure that valid request is passed to ->fn(), and it is driver's
responsibility to avoid double completion.
[1] https://lore.kernel.org/linux-block/YIdWz8C5eklPvEiV@T590/T/#u
Thanks,
Ming
