Re: [PATCH 0/8]: blk-mq: use static_rqs to iterate busy tags
Date: Sun Mar 17 2019 - 22:45:49 EST
On 3/16/19 12:19 AM, Bart Van Assche wrote:
>> This stale request maybe something that has been freed due to io scheduler
>> is detached or a q using a shared tagset is gone.
>> And also the blk_mq_timeout_work could use it to pick up the expired request.
>> The driver would also use it to requeue the in-flight requests when the device is dead.
>> Compared with adding more synchronization, using static_rqs directly maybe simpler :)
> Hi Jianchao,
> Although I appreciate your work: I agree with Christoph that we should avoid races
> like this rather than modifying the block layer to make sure that such races are
> handled safely.
The root cause here is that there is window between set tag sbitmap and set tags->rqs
, and we don't clear the tags->rqs in free tags path. So when we iterate the busy
tags, we could see stale requests in tags->rqs and this stale requests maybe freed.
Looks like it is difficult to close the window above, so we used to try to clear the
tags->rqs in two ways,
1. clear the tags->rqs in the request free path
Jens doesn't like it
It's an extra store, and it's a store to an area that's then now shared
between issue and completion. Those are never a good idea. Besides, it's
the kind of issue you solve in the SLOW path, not in the fast path. Since
that's doable, it would be silly to do it for every IO.
2. clear the associated slots in tags->tags when blk_mq_free_rqs
sbitmap_for_each_set(&bt->sb, bt_iter, &iter_data);
The busy_iter_fn could sleep
-> mutex_lock dev->shutdown_lock
Since it is not so flexible to fix on tags->rqs, why not try to use tags->static_rqs ?
Then we wound never care about the stable requests any more.