linux-stable/block/blk-rq-qos.c
Omar Sandoval e972b08b91 blk-rq-qos: fix crash on rq_qos_wait vs. rq_qos_wake_function race
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>
2024-10-16 07:20:14 -06:00

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);
}