On 7/18/21 3:08 PM, Oleksandr Natalenko wrote:
> + Paolo, Jens et al.
>
> On čtvrtek 15. července 2021 16:32:29 CEST jim.cromie@xxxxxxxxx wrote:
>> hi all,
>>
>> I noticed this report this morning, from 3 days ago,
>> about 10 minutes after boot.
>> Its easiest to ignore it, and I dont want to make a fuss,
>> but it looks useful to someone
>>
>>
>> [ 33.663464] Bluetooth: RFCOMM ver 1.11
>> [ 646.343628]
>> ================================================================== [
>> 646.343649] BUG: KASAN: use-after-free in bfq_get_queue+0x47d/0x900 [
>> 646.343680] Read of size 8 at addr ffff88810d864a00 by task
>> journal-offline/1639
There are only a few commits between 5.13 and master in this area, see
attached. I'd just start reverting from the top, one by one, and see
which one is causing the issue. Jim, would that be feasible?
--
Jens Axboe
commit fd2ef39cc9a6b9c4c41864ac506906c52f94b06a
Author: Jan Kara <jack@xxxxxxx>
Date: Wed Jun 23 11:36:34 2021 +0200
blk: Fix lock inversion between ioc lock and bfqd lock
Lockdep complains about lock inversion between ioc->lock and bfqd->lock:
bfqd -> ioc:
put_io_context+0x33/0x90 -> ioc->lock grabbed
blk_mq_free_request+0x51/0x140
blk_put_request+0xe/0x10
blk_attempt_req_merge+0x1d/0x30
elv_attempt_insert_merge+0x56/0xa0
blk_mq_sched_try_insert_merge+0x4b/0x60
bfq_insert_requests+0x9e/0x18c0 -> bfqd->lock grabbed
blk_mq_sched_insert_requests+0xd6/0x2b0
blk_mq_flush_plug_list+0x154/0x280
blk_finish_plug+0x40/0x60
ext4_writepages+0x696/0x1320
do_writepages+0x1c/0x80
__filemap_fdatawrite_range+0xd7/0x120
sync_file_range+0xac/0xf0
ioc->bfqd:
bfq_exit_icq+0xa3/0xe0 -> bfqd->lock grabbed
put_io_context_active+0x78/0xb0 -> ioc->lock grabbed
exit_io_context+0x48/0x50
do_exit+0x7e9/0xdd0
do_group_exit+0x54/0xc0
To avoid this inversion we change blk_mq_sched_try_insert_merge() to not
free the merged request but rather leave that upto the caller similarly
to blk_mq_sched_try_merge(). And in bfq_insert_requests() we make sure
to free all the merged requests after dropping bfqd->lock.
Fixes: aee69d78dec0 ("block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler")
Reviewed-by: Ming Lei <ming.lei@xxxxxxxxxx>
Acked-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Signed-off-by: Jan Kara <jack@xxxxxxx>
Link: https://lore.kernel.org/r/20210623093634.27879-3-jack@xxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit a921c655f2033dd1ce1379128efe881dda23ea37
Author: Jan Kara <jack@xxxxxxx>
Date: Wed Jun 23 11:36:33 2021 +0200
bfq: Remove merged request already in bfq_requests_merged()
Currently, bfq does very little in bfq_requests_merged() and handles all
the request cleanup in bfq_finish_requeue_request() called from
blk_mq_free_request(). That is currently safe only because
blk_mq_free_request() is called shortly after bfq_requests_merged()
while bfqd->lock is still held. However to fix a lock inversion between
bfqd->lock and ioc->lock, we need to call blk_mq_free_request() after
dropping bfqd->lock. That would mean that already merged request could
be seen by other processes inside bfq queues and possibly dispatched to
the device which is wrong. So move cleanup of the request from
bfq_finish_requeue_request() to bfq_requests_merged().
Acked-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Signed-off-by: Jan Kara <jack@xxxxxxx>
Link: https://lore.kernel.org/r/20210623093634.27879-2-jack@xxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit 9a2ac41b13c573703d6689f51f3e27dd658324be
Author: Paolo Valente <paolo.valente@xxxxxxxxxx>
Date: Sat Jun 19 16:09:48 2021 +0200
block, bfq: reset waker pointer with shared queues
Commit 85686d0dc194 ("block, bfq: keep shared queues out of the waker
mechanism") leaves shared bfq_queues out of the waker-detection
mechanism. It attains this goal by not updating the pointer
last_completed_rq_bfqq, if the last request completed belongs to a
shared bfq_queue (so that the pointer will not point to the shared
bfq_queue).
Yet this has a side effect: the pointer last_completed_rq_bfqq keeps
pointing, deceptively, to a bfq_queue that actually is not the last
one to have had a request completed. As a consequence, such a
bfq_queue may deceptively be considered as a waker of some bfq_queue,
even of some shared bfq_queue.
To address this issue, reset last_completed_rq_bfqq if the last
request completed belongs to a shared queue.
Fixes: 85686d0dc194 ("block, bfq: keep shared queues out of the waker mechanism")
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-8-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit efc72524b3a9e4e7bc7c07f756528736409ec1b7
Author: Paolo Valente <paolo.valente@xxxxxxxxxx>
Date: Sat Jun 19 16:09:47 2021 +0200
block, bfq: check waker only for queues with no in-flight I/O
Consider two bfq_queues, say Q1 and Q2, with Q2 empty. If a request of
Q1 gets completed shortly before a new request arrives for Q2, then
BFQ flags Q1 as a candidate waker for Q2. Yet, the arrival of this new
request may have a different cause, in the following case. If also Q2
has requests in flight while waiting for the arrival of a new request,
then the completion of its own requests may be the actual cause of the
awakening of the process that sends I/O to Q2. So Q1 may be flagged
wrongly as a candidate waker.
This commit avoids this deceptive flagging, by disabling
candidate-waker flagging for Q2, if Q2 has in-flight I/O.
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-7-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit bd3664b362381c4c1473753ebedf0ab242a60d1d
Author: Paolo Valente <paolo.valente@xxxxxxxxxx>
Date: Sat Jun 19 16:09:46 2021 +0200
block, bfq: avoid delayed merge of async queues
Since commit 430a67f9d616 ("block, bfq: merge bursts of newly-created
queues"), BFQ may schedule a merge between a newly created sync
bfq_queue, say Q2, and the last sync bfq_queue created, say Q1. To this
goal, BFQ stores the address of Q1 in the field bic->stable_merge_bfqq
of the bic associated with Q2. So, when the time for the possible merge
arrives, BFQ knows which bfq_queue to merge Q2 with. In particular,
BFQ checks for possible merges on request arrivals.
Yet the same bic may also be associated with an async bfq_queue, say
Q3. So, if a request for Q3 arrives, then the above check may happen
to be executed while the bfq_queue at hand is Q3, instead of Q2. In
this case, Q1 happens to be merged with an async bfq_queue. This is
not only a conceptual mistake, because async queues are to be kept out
of queue merging, but also a bug that leads to inconsistent states.
This commits simply filters async queues out of delayed merges.
Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Tested-by: Holger Hoffstätte <holger@xxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-6-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit 7812472f973047a886e4ed9a91d98d6627dd746f
Author: Pietro Pedroni <pedroni.pietro.96@xxxxxxxxx>
Date: Sat Jun 19 16:09:45 2021 +0200
block, bfq: boost throughput by extending queue-merging times
One of the methods with which bfq boosts throughput is by merging queues.
One of the merging variants in bfq is the stable merge.
This mechanism is activated between two queues only if they are created
within a certain maximum time T1 from each other.
Merging can happen soon or be delayed. In the second case, before
merging, bfq needs to evaluate a throughput-boost parameter that
indicates whether the queue generates a high throughput is served alone.
Merging occurs when this throughput-boost is not high enough.
In particular, this parameter is evaluated and late merging may occur
only after at least a time T2 from the creation of the queue.
Currently T1 and T2 are set to 180ms and 200ms, respectively.
In this way the merging mechanism rarely occurs because time is not
enough. This results in a noticeable lowering of the overall throughput
with some workloads (see the example below).
This commit introduces two constants bfq_activation_stable_merging and
bfq_late_stable_merging in order to increase the duration of T1 and T2.
Both the stable merging activation time and the late merging
time are set to 600ms. This value has been experimentally evaluated
using sqlite benchmark in the Phoronix Test Suite on a HDD.
The duration of the benchmark before this fix was 111.02s, while now
it has reached 97.02s, a better result than that of all the other
schedulers.
Signed-off-by: Pietro Pedroni <pedroni.pietro.96@xxxxxxxxx>
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-5-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit d4f49983fa3944416c28379c35fbe10c68455ea4
Author: Paolo Valente <paolo.valente@xxxxxxxxxx>
Date: Sat Jun 19 16:09:44 2021 +0200
block, bfq: consider also creation time in delayed stable merge
Since commit 430a67f9d616 ("block, bfq: merge bursts of newly-created
queues"), BFQ may schedule a merge between a newly created sync
bfq_queue and the last sync bfq_queue created. Such a merging is not
performed immediately, because BFQ needs first to find out whether the
newly created queue actually reaches a higher throughput if not merged
at all (and in that case BFQ will not perform any stable merging). To
check that, a little time must be waited after the creation of the new
queue, so that some I/O can flow in the queue, and statistics on such
I/O can be computed.
Yet, to evaluate the above waiting time, the last split time is
considered as start time, instead of the creation time of the
queue. This is a mistake, because considering the split time is
correct only in the following scenario.
The queue undergoes a non-stable merges on the arrival of its very
first I/O request, due to close I/O with some other queue. While the
queue is merged for close I/O, stable merging is not considered. Yet
the queue may then happen to be split, if the close I/O finishes (or
happens to be a false positive). From this time on, the queue can
again be considered for stable merging. But, again, a little time must
elapse, to let some new I/O flow in the queue and to get updated
statistics. To wait for this time, the split time is to be taken into
account.
Yet, if the queue does not undergo a non-stable merge on the arrival
of its very first request, then BFQ immediately checks whether the
stable merge is to be performed. It happens because the split time for
a queue is initialized to minus infinity when the queue is created.
This commit fixes this mistake by adding the missing condition. Now
the check for delayed stable-merge is performed after a little time is
elapsed not only from the last queue split time, but also from the
creation time of the queue.
Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-4-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit e03f2ab78a4a673e4af23c3b855591c48b9de4d7
Author: Luca Mariotti <mariottiluca1@xxxxxxxxxx>
Date: Sat Jun 19 16:09:43 2021 +0200
block, bfq: fix delayed stable merge check
When attempting to schedule a merge of a given bfq_queue with the currently
in-service bfq_queue or with a cooperating bfq_queue among the scheduled
bfq_queues, delayed stable merge is checked for rotational or non-queueing
devs. For this stable merge to be performed, some conditions must be met.
If the current bfq_queue underwent some split from some merged bfq_queue,
one of these conditions is that two hundred milliseconds must elapse from
split, otherwise this condition is always met.
Unfortunately, by mistake, time_is_after_jiffies() was written instead of
time_is_before_jiffies() for this check, verifying that less than two
hundred milliseconds have elapsed instead of verifying that at least two
hundred milliseconds have elapsed.
Fix this issue by replacing time_is_after_jiffies() with
time_is_before_jiffies().
Signed-off-by: Luca Mariotti <mariottiluca1@xxxxxxxxxx>
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Signed-off-by: Pietro Pedroni <pedroni.pietro.96@xxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-3-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
commit 511a2699237611b062df7798476bf3a1392910b9
Author: Paolo Valente <paolo.valente@xxxxxxxxxx>
Date: Sat Jun 19 16:09:42 2021 +0200
block, bfq: let also stably merged queues enjoy weight raising
Merged bfq_queues are kept out of weight-raising (low-latency)
mechanisms. The reason is that these queues are usually created for
non-interactive and non-soft-real-time tasks. Yet this is not the case
for stably-merged queues. These queues are merged just because they
are created shortly after each other. So they may easily serve the I/O
of an interactive or soft-real time application, if the application
happens to spawn multiple processes.
To address this issue, this commits lets also stably-merged queued
enjoy weight raising.
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20210619140948.98712-2-paolo.valente@xxxxxxxxxx
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
