Revision 6e2df0581f569038719cf2bc2b3baa3fcc83cab4 authored by Peter Zijlstra on 08 November 2019, 10:11:52 UTC, committed by Peter Zijlstra on 08 November 2019, 21:34:14 UTC
Commit 67692435c411 ("sched: Rework pick_next_task() slow-path") inadvertly introduced a race because it changed a previously unexplored dependency between dropping the rq->lock and sched_class::put_prev_task(). The comments about dropping rq->lock, in for example newidle_balance(), only mentions the task being current and ->on_cpu being set. But when we look at the 'change' pattern (in for example sched_setnuma()): queued = task_on_rq_queued(p); /* p->on_rq == TASK_ON_RQ_QUEUED */ running = task_current(rq, p); /* rq->curr == p */ if (queued) dequeue_task(...); if (running) put_prev_task(...); /* change task properties */ if (queued) enqueue_task(...); if (running) set_next_task(...); It becomes obvious that if we do this after put_prev_task() has already been called on @p, things go sideways. This is exactly what the commit in question allows to happen when it does: prev->sched_class->put_prev_task(rq, prev, rf); if (!rq->nr_running) newidle_balance(rq, rf); The newidle_balance() call will drop rq->lock after we've called put_prev_task() and that allows the above 'change' pattern to interleave and mess up the state. Furthermore, it turns out we lost the RT-pull when we put the last DL task. Fix both problems by extracting the balancing from put_prev_task() and doing a multi-class balance() pass before put_prev_task(). Fixes: 67692435c411 ("sched: Rework pick_next_task() slow-path") Reported-by: Quentin Perret <qperret@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Tested-by: Quentin Perret <qperret@google.com> Tested-by: Valentin Schneider <valentin.schneider@arm.com>
1 parent e3b8b6a
id.c
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Squashfs - a compressed read only filesystem for Linux
*
* Copyright (c) 2002, 2003, 2004, 2005, 2006, 2007, 2008
* Phillip Lougher <phillip@squashfs.org.uk>
*
* id.c
*/
/*
* This file implements code to handle uids and gids.
*
* For space efficiency regular files store uid and gid indexes, which are
* converted to 32-bit uids/gids using an id look up table. This table is
* stored compressed into metadata blocks. A second index table is used to
* locate these. This second index table for speed of access (and because it
* is small) is read at mount time and cached in memory.
*/
#include <linux/fs.h>
#include <linux/vfs.h>
#include <linux/slab.h>
#include "squashfs_fs.h"
#include "squashfs_fs_sb.h"
#include "squashfs.h"
/*
* Map uid/gid index into real 32-bit uid/gid using the id look up table
*/
int squashfs_get_id(struct super_block *sb, unsigned int index,
unsigned int *id)
{
struct squashfs_sb_info *msblk = sb->s_fs_info;
int block = SQUASHFS_ID_BLOCK(index);
int offset = SQUASHFS_ID_BLOCK_OFFSET(index);
u64 start_block = le64_to_cpu(msblk->id_table[block]);
__le32 disk_id;
int err;
err = squashfs_read_metadata(sb, &disk_id, &start_block, &offset,
sizeof(disk_id));
if (err < 0)
return err;
*id = le32_to_cpu(disk_id);
return 0;
}
/*
* Read uncompressed id lookup table indexes from disk into memory
*/
__le64 *squashfs_read_id_index_table(struct super_block *sb,
u64 id_table_start, u64 next_table, unsigned short no_ids)
{
unsigned int length = SQUASHFS_ID_BLOCK_BYTES(no_ids);
__le64 *table;
TRACE("In read_id_index_table, length %d\n", length);
/* Sanity check values */
/* there should always be at least one id */
if (no_ids == 0)
return ERR_PTR(-EINVAL);
/*
* length bytes should not extend into the next table - this check
* also traps instances where id_table_start is incorrectly larger
* than the next table start
*/
if (id_table_start + length > next_table)
return ERR_PTR(-EINVAL);
table = squashfs_read_table(sb, id_table_start, length);
/*
* table[0] points to the first id lookup table metadata block, this
* should be less than id_table_start
*/
if (!IS_ERR(table) && le64_to_cpu(table[0]) >= id_table_start) {
kfree(table);
return ERR_PTR(-EINVAL);
}
return table;
}
![swh spinner](/static/img/swh-spinner.gif)
Computing file changes ...