Skip to content

Commit e5dbda8

Browse files
Changbin Duintel-lab-lkp
authored andcommitted
riscv: patch: Fixup lockdep warning in stop_machine
The task of ftrace_arch_code_modify(_post)_prepare() caller is stop_machine, whose caller and work thread are of different tasks. The lockdep checker needs the same task context, or it's wrong. That means it's a bug here to use lockdep_assert_held because we don't guarantee the same task context. kernel/locking/lockdep.c: int __lock_is_held(const struct lockdep_map *lock, int read) { struct task_struct *curr = current; int i; for (i = 0; i < curr->lockdep_depth; i++) { ^^^^^^^^^^^^^^^^^^^ struct held_lock *hlock = curr->held_locks + i; ^^^^^^^^^^^^^^^^ if (match_held_lock(hlock, lock)) { if (read == -1 || !!hlock->read == read) return LOCK_STATE_HELD; The __lock_is_held depends on current held_locks records; if stop_machine makes the checker running on another task, that's wrong. Here is the log: [ 15.761523] ------------[ cut here ]------------ [ 15.762125] WARNING: CPU: 0 PID: 15 at arch/riscv/kernel/patch.c:63 patch_insn_write+0x72/0x364 [ 15.763258] Modules linked in: [ 15.764154] CPU: 0 PID: 15 Comm: migration/0 Not tainted 6.1.0-rc1-00014-g66924be85884-dirty torvalds#377 [ 15.765339] Hardware name: riscv-virtio,qemu (DT) [ 15.765985] Stopper: multi_cpu_stop+0x0/0x192 <- stop_cpus.constprop.0+0x90/0xe2 [ 15.766711] epc : patch_insn_write+0x72/0x364 [ 15.767011] ra : patch_insn_write+0x70/0x364 [ 15.767276] epc : ffffffff8000721e ra : ffffffff8000721c sp : ff2000000067bca0 [ 15.767622] gp : ffffffff81603f90 tp : ff60000002432a00 t0 : 7300000000000000 [ 15.767919] t1 : 0000000000000000 t2 : 73695f6b636f6c5f s0 : ff2000000067bcf0 [ 15.768238] s1 : 0000000000000008 a0 : 0000000000000000 a1 : 0000000000000000 [ 15.768537] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000 [ 15.768837] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 [ 15.769139] s2 : ffffffff80009faa s3 : ff2000000067bd10 s4 : ffffffffffffffff [ 15.769447] s5 : 0000000000000001 s6 : 0000000000000001 s7 : 0000000000000003 [ 15.769740] s8 : 0000000000000002 s9 : 0000000000000004 s10: 0000000000000003 [ 15.770027] s11: 0000000000000002 t3 : 0000000000000000 t4 : ffffffff819af097 [ 15.770323] t5 : ffffffff819af098 t6 : ff2000000067ba28 [ 15.770574] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 [ 15.771102] [<ffffffff80007520>] patch_text_nosync+0x10/0x3a [ 15.771421] [<ffffffff80009c66>] ftrace_update_ftrace_func+0x74/0x10a [ 15.771704] [<ffffffff800fa17e>] ftrace_modify_all_code+0xb0/0x16c [ 15.771958] [<ffffffff800fa24c>] __ftrace_modify_code+0x12/0x1c [ 15.772196] [<ffffffff800e110e>] multi_cpu_stop+0x14a/0x192 [ 15.772454] [<ffffffff800e0a34>] cpu_stopper_thread+0x96/0x14c [ 15.772699] [<ffffffff8003f4ea>] smpboot_thread_fn+0xf8/0x1cc [ 15.772945] [<ffffffff8003ac9c>] kthread+0xe2/0xf8 [ 15.773160] [<ffffffff80003e98>] ret_from_exception+0x0/0x14 [ 15.773471] ---[ end trace 0000000000000000 ]--- By the way, this also fixes the same issue for patch_text(). Fixes: 0ff7c3b ("riscv: Use text_mutex instead of patch_lock") Co-developed-by: Guo Ren <[email protected]> Signed-off-by: Guo Ren <[email protected]> Cc: Zong Li <[email protected]> Cc: Palmer Dabbelt <[email protected]> Signed-off-by: Changbin Du <[email protected]>
1 parent 9f266cc commit e5dbda8

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

arch/riscv/kernel/ftrace.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,8 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
126126
/*
127127
* This is called early on, and isn't wrapped by
128128
* ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold
129-
* text_mutex, which triggers a lockdep failure. SMP isn't running so we could
130-
* just directly poke the text, but it's simpler to just take the lock
131-
* ourselves.
129+
* text_mutex. SMP isn't running so we could just directly poke the text, but
130+
* it's simpler to just take the lock ourselves.
132131
*/
133132
int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec)
134133
{

arch/riscv/kernel/patch.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,20 @@ static void patch_unmap(int fixmap)
4949
}
5050
NOKPROBE_SYMBOL(patch_unmap);
5151

52+
/*
53+
* Before reaching here, it was expected to lock the text_mutex
54+
* already, so we don't need to give another lock here and could
55+
* ensure that it was safe between each cores. We do not add
56+
* lockdep assertion here since it would trigger a false positive
57+
* when called by stop_machine (The lockdep checker requires the
58+
* same task context).
59+
*/
5260
static int patch_insn_write(void *addr, const void *insn, size_t len)
5361
{
5462
void *waddr = addr;
5563
bool across_pages = (((uintptr_t) addr & ~PAGE_MASK) + len) > PAGE_SIZE;
5664
int ret;
5765

58-
/*
59-
* Before reaching here, it was expected to lock the text_mutex
60-
* already, so we don't need to give another lock here and could
61-
* ensure that it was safe between each cores.
62-
*/
63-
lockdep_assert_held(&text_mutex);
64-
6566
if (across_pages)
6667
patch_map(addr + len, FIX_TEXT_POKE1);
6768

0 commit comments

Comments
 (0)