e972b08b91
We're seeing crashes from rq_qos_wake_function that look like this:
BUG: unable to handle page fault for address: ffffafe180a40084
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 100000067 P4D 100000067 PUD 10027c067 PMD 10115d067 PTE 0
Oops: Oops: 0002 [#1] PREEMPT SMP PTI
CPU: 17 UID: 0 PID: 0 Comm: swapper/17 Not tainted 6.12.0-rc3-00013-geca631b8fe80 #11
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
RIP: 0010:_raw_spin_lock_irqsave+0x1d/0x40
Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 41 54 9c 41 5c fa 65 ff 05 62 97 30 4c 31 c0 ba 01 00 00 00 <f0> 0f b1 17 75 0a 4c 89 e0 41 5c c3 cc cc cc cc 89 c6 e8 2c 0b 00
RSP: 0018:ffffafe180580ca0 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffffafe180a3f7a8 RCX: 0000000000000011
RDX: 0000000000000001 RSI: 0000000000000003 RDI: ffffafe180a40084
RBP: 0000000000000000 R08: 00000000001e7240 R09: 0000000000000011
R10: 0000000000000028 R11: 0000000000000888 R12: 0000000000000002
R13: ffffafe180a40084 R14: 0000000000000000 R15: 0000000000000003
FS: 0000000000000000(0000) GS:ffff9aaf1f280000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffafe180a40084 CR3: 000000010e428002 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<IRQ>
try_to_wake_up+0x5a/0x6a0
rq_qos_wake_function+0x71/0x80
__wake_up_common+0x75/0xa0
__wake_up+0x36/0x60
scale_up.part.0+0x50/0x110
wb_timer_fn+0x227/0x450
...
So rq_qos_wake_function() calls wake_up_process(data->task), which calls
try_to_wake_up(), which faults in raw_spin_lock_irqsave(&p->pi_lock).
p comes from data->task, and data comes from the waitqueue entry, which
is stored on the waiter's stack in rq_qos_wait(). Analyzing the core
dump with drgn, I found that the waiter had already woken up and moved
on to a completely unrelated code path, clobbering what was previously
data->task. Meanwhile, the waker was passing the clobbered garbage in
data->task to wake_up_process(), leading to the crash.
What's happening is that in between rq_qos_wake_function() deleting the
waitqueue entry and calling wake_up_process(), rq_qos_wait() is finding
that it already got a token and returning. The race looks like this:
rq_qos_wait() rq_qos_wake_function()
==============================================================
prepare_to_wait_exclusive()
data->got_token = true;
list_del_init(&curr->entry);
if (data.got_token)
break;
finish_wait(&rqw->wait, &data.wq);
^- returns immediately because
list_empty_careful(&wq_entry->entry)
is true
... return, go do something else ...
wake_up_process(data->task)
(NO LONGER VALID!)-^
Normally, finish_wait() is supposed to synchronize against the waker.
But, as noted above, it is returning immediately because the waitqueue
entry has already been removed from the waitqueue.
The bug is that rq_qos_wake_function() is accessing the waitqueue entry
AFTER deleting it. Note that autoremove_wake_function() wakes the waiter
and THEN deletes the waitqueue entry, which is the proper order.
Fix it by swapping the order. We also need to use
list_del_init_careful() to match the list_empty_careful() in
finish_wait().
Fixes: 38cfb5a45e
("blk-wbt: improve waking of tasks")
Cc: stable@vger.kernel.org
Signed-off-by: Omar Sandoval <osandov@fb.com>
Acked-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Link: https://lore.kernel.org/r/d3bee2463a67b1ee597211823bf7ad3721c26e41.1729014591.git.osandov@fb.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
356 lines
8.2 KiB
C
356 lines
8.2 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
|
|
#include "blk-rq-qos.h"
|
|
|
|
/*
|
|
* Increment 'v', if 'v' is below 'below'. Returns true if we succeeded,
|
|
* false if 'v' + 1 would be bigger than 'below'.
|
|
*/
|
|
static bool atomic_inc_below(atomic_t *v, unsigned int below)
|
|
{
|
|
unsigned int cur = atomic_read(v);
|
|
|
|
do {
|
|
if (cur >= below)
|
|
return false;
|
|
} while (!atomic_try_cmpxchg(v, &cur, cur + 1));
|
|
|
|
return true;
|
|
}
|
|
|
|
bool rq_wait_inc_below(struct rq_wait *rq_wait, unsigned int limit)
|
|
{
|
|
return atomic_inc_below(&rq_wait->inflight, limit);
|
|
}
|
|
|
|
void __rq_qos_cleanup(struct rq_qos *rqos, struct bio *bio)
|
|
{
|
|
do {
|
|
if (rqos->ops->cleanup)
|
|
rqos->ops->cleanup(rqos, bio);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_done(struct rq_qos *rqos, struct request *rq)
|
|
{
|
|
do {
|
|
if (rqos->ops->done)
|
|
rqos->ops->done(rqos, rq);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_issue(struct rq_qos *rqos, struct request *rq)
|
|
{
|
|
do {
|
|
if (rqos->ops->issue)
|
|
rqos->ops->issue(rqos, rq);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_requeue(struct rq_qos *rqos, struct request *rq)
|
|
{
|
|
do {
|
|
if (rqos->ops->requeue)
|
|
rqos->ops->requeue(rqos, rq);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_throttle(struct rq_qos *rqos, struct bio *bio)
|
|
{
|
|
do {
|
|
if (rqos->ops->throttle)
|
|
rqos->ops->throttle(rqos, bio);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_track(struct rq_qos *rqos, struct request *rq, struct bio *bio)
|
|
{
|
|
do {
|
|
if (rqos->ops->track)
|
|
rqos->ops->track(rqos, rq, bio);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_merge(struct rq_qos *rqos, struct request *rq, struct bio *bio)
|
|
{
|
|
do {
|
|
if (rqos->ops->merge)
|
|
rqos->ops->merge(rqos, rq, bio);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_done_bio(struct rq_qos *rqos, struct bio *bio)
|
|
{
|
|
do {
|
|
if (rqos->ops->done_bio)
|
|
rqos->ops->done_bio(rqos, bio);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
void __rq_qos_queue_depth_changed(struct rq_qos *rqos)
|
|
{
|
|
do {
|
|
if (rqos->ops->queue_depth_changed)
|
|
rqos->ops->queue_depth_changed(rqos);
|
|
rqos = rqos->next;
|
|
} while (rqos);
|
|
}
|
|
|
|
/*
|
|
* Return true, if we can't increase the depth further by scaling
|
|
*/
|
|
bool rq_depth_calc_max_depth(struct rq_depth *rqd)
|
|
{
|
|
unsigned int depth;
|
|
bool ret = false;
|
|
|
|
/*
|
|
* For QD=1 devices, this is a special case. It's important for those
|
|
* to have one request ready when one completes, so force a depth of
|
|
* 2 for those devices. On the backend, it'll be a depth of 1 anyway,
|
|
* since the device can't have more than that in flight. If we're
|
|
* scaling down, then keep a setting of 1/1/1.
|
|
*/
|
|
if (rqd->queue_depth == 1) {
|
|
if (rqd->scale_step > 0)
|
|
rqd->max_depth = 1;
|
|
else {
|
|
rqd->max_depth = 2;
|
|
ret = true;
|
|
}
|
|
} else {
|
|
/*
|
|
* scale_step == 0 is our default state. If we have suffered
|
|
* latency spikes, step will be > 0, and we shrink the
|
|
* allowed write depths. If step is < 0, we're only doing
|
|
* writes, and we allow a temporarily higher depth to
|
|
* increase performance.
|
|
*/
|
|
depth = min_t(unsigned int, rqd->default_depth,
|
|
rqd->queue_depth);
|
|
if (rqd->scale_step > 0)
|
|
depth = 1 + ((depth - 1) >> min(31, rqd->scale_step));
|
|
else if (rqd->scale_step < 0) {
|
|
unsigned int maxd = 3 * rqd->queue_depth / 4;
|
|
|
|
depth = 1 + ((depth - 1) << -rqd->scale_step);
|
|
if (depth > maxd) {
|
|
depth = maxd;
|
|
ret = true;
|
|
}
|
|
}
|
|
|
|
rqd->max_depth = depth;
|
|
}
|
|
|
|
return ret;
|
|
}
|
|
|
|
/* Returns true on success and false if scaling up wasn't possible */
|
|
bool rq_depth_scale_up(struct rq_depth *rqd)
|
|
{
|
|
/*
|
|
* Hit max in previous round, stop here
|
|
*/
|
|
if (rqd->scaled_max)
|
|
return false;
|
|
|
|
rqd->scale_step--;
|
|
|
|
rqd->scaled_max = rq_depth_calc_max_depth(rqd);
|
|
return true;
|
|
}
|
|
|
|
/*
|
|
* Scale rwb down. If 'hard_throttle' is set, do it quicker, since we
|
|
* had a latency violation. Returns true on success and returns false if
|
|
* scaling down wasn't possible.
|
|
*/
|
|
bool rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle)
|
|
{
|
|
/*
|
|
* Stop scaling down when we've hit the limit. This also prevents
|
|
* ->scale_step from going to crazy values, if the device can't
|
|
* keep up.
|
|
*/
|
|
if (rqd->max_depth == 1)
|
|
return false;
|
|
|
|
if (rqd->scale_step < 0 && hard_throttle)
|
|
rqd->scale_step = 0;
|
|
else
|
|
rqd->scale_step++;
|
|
|
|
rqd->scaled_max = false;
|
|
rq_depth_calc_max_depth(rqd);
|
|
return true;
|
|
}
|
|
|
|
struct rq_qos_wait_data {
|
|
struct wait_queue_entry wq;
|
|
struct task_struct *task;
|
|
struct rq_wait *rqw;
|
|
acquire_inflight_cb_t *cb;
|
|
void *private_data;
|
|
bool got_token;
|
|
};
|
|
|
|
static int rq_qos_wake_function(struct wait_queue_entry *curr,
|
|
unsigned int mode, int wake_flags, void *key)
|
|
{
|
|
struct rq_qos_wait_data *data = container_of(curr,
|
|
struct rq_qos_wait_data,
|
|
wq);
|
|
|
|
/*
|
|
* If we fail to get a budget, return -1 to interrupt the wake up loop
|
|
* in __wake_up_common.
|
|
*/
|
|
if (!data->cb(data->rqw, data->private_data))
|
|
return -1;
|
|
|
|
data->got_token = true;
|
|
smp_wmb();
|
|
wake_up_process(data->task);
|
|
list_del_init_careful(&curr->entry);
|
|
return 1;
|
|
}
|
|
|
|
/**
|
|
* rq_qos_wait - throttle on a rqw if we need to
|
|
* @rqw: rqw to throttle on
|
|
* @private_data: caller provided specific data
|
|
* @acquire_inflight_cb: inc the rqw->inflight counter if we can
|
|
* @cleanup_cb: the callback to cleanup in case we race with a waker
|
|
*
|
|
* This provides a uniform place for the rq_qos users to do their throttling.
|
|
* Since you can end up with a lot of things sleeping at once, this manages the
|
|
* waking up based on the resources available. The acquire_inflight_cb should
|
|
* inc the rqw->inflight if we have the ability to do so, or return false if not
|
|
* and then we will sleep until the room becomes available.
|
|
*
|
|
* cleanup_cb is in case that we race with a waker and need to cleanup the
|
|
* inflight count accordingly.
|
|
*/
|
|
void rq_qos_wait(struct rq_wait *rqw, void *private_data,
|
|
acquire_inflight_cb_t *acquire_inflight_cb,
|
|
cleanup_cb_t *cleanup_cb)
|
|
{
|
|
struct rq_qos_wait_data data = {
|
|
.wq = {
|
|
.func = rq_qos_wake_function,
|
|
.entry = LIST_HEAD_INIT(data.wq.entry),
|
|
},
|
|
.task = current,
|
|
.rqw = rqw,
|
|
.cb = acquire_inflight_cb,
|
|
.private_data = private_data,
|
|
};
|
|
bool has_sleeper;
|
|
|
|
has_sleeper = wq_has_sleeper(&rqw->wait);
|
|
if (!has_sleeper && acquire_inflight_cb(rqw, private_data))
|
|
return;
|
|
|
|
has_sleeper = !prepare_to_wait_exclusive(&rqw->wait, &data.wq,
|
|
TASK_UNINTERRUPTIBLE);
|
|
do {
|
|
/* The memory barrier in set_current_state saves us here. */
|
|
if (data.got_token)
|
|
break;
|
|
if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) {
|
|
finish_wait(&rqw->wait, &data.wq);
|
|
|
|
/*
|
|
* We raced with rq_qos_wake_function() getting a token,
|
|
* which means we now have two. Put our local token
|
|
* and wake anyone else potentially waiting for one.
|
|
*/
|
|
smp_rmb();
|
|
if (data.got_token)
|
|
cleanup_cb(rqw, private_data);
|
|
break;
|
|
}
|
|
io_schedule();
|
|
has_sleeper = true;
|
|
set_current_state(TASK_UNINTERRUPTIBLE);
|
|
} while (1);
|
|
finish_wait(&rqw->wait, &data.wq);
|
|
}
|
|
|
|
void rq_qos_exit(struct request_queue *q)
|
|
{
|
|
mutex_lock(&q->rq_qos_mutex);
|
|
while (q->rq_qos) {
|
|
struct rq_qos *rqos = q->rq_qos;
|
|
q->rq_qos = rqos->next;
|
|
rqos->ops->exit(rqos);
|
|
}
|
|
mutex_unlock(&q->rq_qos_mutex);
|
|
}
|
|
|
|
int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
|
|
const struct rq_qos_ops *ops)
|
|
{
|
|
struct request_queue *q = disk->queue;
|
|
|
|
lockdep_assert_held(&q->rq_qos_mutex);
|
|
|
|
rqos->disk = disk;
|
|
rqos->id = id;
|
|
rqos->ops = ops;
|
|
|
|
/*
|
|
* No IO can be in-flight when adding rqos, so freeze queue, which
|
|
* is fine since we only support rq_qos for blk-mq queue.
|
|
*/
|
|
blk_mq_freeze_queue(q);
|
|
|
|
if (rq_qos_id(q, rqos->id))
|
|
goto ebusy;
|
|
rqos->next = q->rq_qos;
|
|
q->rq_qos = rqos;
|
|
|
|
blk_mq_unfreeze_queue(q);
|
|
|
|
if (rqos->ops->debugfs_attrs) {
|
|
mutex_lock(&q->debugfs_mutex);
|
|
blk_mq_debugfs_register_rqos(rqos);
|
|
mutex_unlock(&q->debugfs_mutex);
|
|
}
|
|
|
|
return 0;
|
|
ebusy:
|
|
blk_mq_unfreeze_queue(q);
|
|
return -EBUSY;
|
|
}
|
|
|
|
void rq_qos_del(struct rq_qos *rqos)
|
|
{
|
|
struct request_queue *q = rqos->disk->queue;
|
|
struct rq_qos **cur;
|
|
|
|
lockdep_assert_held(&q->rq_qos_mutex);
|
|
|
|
blk_mq_freeze_queue(q);
|
|
for (cur = &q->rq_qos; *cur; cur = &(*cur)->next) {
|
|
if (*cur == rqos) {
|
|
*cur = rqos->next;
|
|
break;
|
|
}
|
|
}
|
|
blk_mq_unfreeze_queue(q);
|
|
|
|
mutex_lock(&q->debugfs_mutex);
|
|
blk_mq_debugfs_unregister_rqos(rqos);
|
|
mutex_unlock(&q->debugfs_mutex);
|
|
}
|