+
On 04/01/2021 18:37, John Garry wrote:
Hi Bart,
Right, what I proposed is unrelated to the use-after-free triggered by
disabling I/O scheduling.
Regarding the races triggered by disabling I/O scheduling: can these be
fixed by quiescing all request queues associated with a tag set before
changing the I/O scheduler instead of only the request queue for which
the
I/O scheduler is changed? I think we already do this before updating the
number of hardware queues.
Maybe quiescing all request queues could solve the issue in
blk_mq_queue_tag_busy_iter(), as that issue involves interaction of 2x
request queues.
But the blk_mq_tagset_busy_iter() issue, above, it is related to only a
single request queue, so not sure how it could help.
But let me consider this more.
I have looked at this proposal again, that being to quiesce all (other)
request queues prior to freeing the IO scheduler tags+requests. I tried
this and it seems to work (changes not shown), in terms of solving the
issue of blk_mq_queue_tag_busy_iter() and freeing the requests+tags
racing. However, I am still not sure if it is acceptable to quiesce all
request queues like this just when changing IO scheduler, even if it is
done elsewhere.
In addition, this still leaves the blk_mq_tagset_busy_iter() issue; that
being that freeing requests+tags can race against that same function.
There is nothing to stop blk_mq_tagset_busy_iter() taking a reference to
a request and that request being freed in parallel when switching IO
scheduler, however unlikely.
Solving that becomes trickier. As Ming pointed out, that function can be
called from softirq and even hard irq context - like
ufshcd_tmv_handler() -> blk_mq_tagset_busy_iter() - so there is a
locking context issue.
Thanks,
John
