From cd77849a69cf5d81a86dd88dac7a594a67cb5c87 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 1 Jun 2018 07:23:51 -0500 Subject: objtool: Fix GCC 8 cold subfunction detection for aliased functions The kbuild test robot reported the following issue: kernel/time/posix-stubs.o: warning: objtool: sys_ni_posix_timers.cold.1()+0x0: unreachable instruction This file creates symbol aliases for the sys_ni_posix_timers() function. So there are multiple ELF function symbols for the same function: 23: 0000000000000150 26 FUNC GLOBAL DEFAULT 1 __x64_sys_timer_create 24: 0000000000000150 26 FUNC GLOBAL DEFAULT 1 sys_ni_posix_timers 25: 0000000000000150 26 FUNC GLOBAL DEFAULT 1 __ia32_sys_timer_create 26: 0000000000000150 26 FUNC GLOBAL DEFAULT 1 __x64_sys_timer_gettime Here's the corresponding cold subfunction: 11: 0000000000000000 45 FUNC LOCAL DEFAULT 6 sys_ni_posix_timers.cold.1 When analyzing overlapping functions, objtool only looks at the first one in the symbol list. The rest of the functions are basically ignored because they point to instructions which have already been analyzed. So in this case it analyzes the __x64_sys_timer_create() function, but then it fails to recognize that its cold subfunction is sys_ni_posix_timers.cold.1(), because the names are different. Make the subfunction detection a little smarter by associating each subfunction with the first function which jumps to it, since that's the one which will be analyzed. Unfortunately we still have to leave the original subfunction detection code in place, thanks to GCC switch tables. (See the comment for more details.) Fixes: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions") Reported-by: kbuild test robot Signed-off-by: Josh Poimboeuf Signed-off-by: Thomas Gleixner Cc: Peter Zijlstra Link: https://lkml.kernel.org/r/d3ba52662cbc8e3a64a3b64d44b4efc5674fd9ab.1527855808.git.jpoimboe@redhat.com --- tools/objtool/check.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 3a31b238f885..38047c6aa575 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -543,6 +543,28 @@ static int add_jump_destinations(struct objtool_file *file) dest_off); return -1; } + + /* + * For GCC 8+, create parent/child links for any cold + * subfunctions. This is _mostly_ redundant with a similar + * initialization in read_symbols(). + * + * If a function has aliases, we want the *first* such function + * in the symbol table to be the subfunction's parent. In that + * case we overwrite the initialization done in read_symbols(). + * + * However this code can't completely replace the + * read_symbols() code because this doesn't detect the case + * where the parent function's only reference to a subfunction + * is through a switch table. + */ + if (insn->func && insn->jump_dest->func && + insn->func != insn->jump_dest->func && + !strstr(insn->func->name, ".cold.") && + strstr(insn->jump_dest->func->name, ".cold.")) { + insn->func->cfunc = insn->jump_dest->func; + insn->jump_dest->func->pfunc = insn->func; + } } return 0; -- cgit v1.2.3 From ec84b27f9b3b569f9235413d1945a2006b97b0aa Mon Sep 17 00:00:00 2001 From: Anna-Maria Gleixner Date: Fri, 25 May 2018 11:05:06 +0200 Subject: rcu: Update documentation of rcu_read_unlock() Since commit b4abf91047cf ("rtmutex: Make wait_lock irq safe") the explanation in rcu_read_unlock() documentation about irq unsafe rtmutex wait_lock is no longer valid. Remove it to prevent kernel developers reading the documentation to rely on it. Suggested-by: Eric W. Biederman Signed-off-by: Anna-Maria Gleixner Signed-off-by: Thomas Gleixner Reviewed-by: Paul E. McKenney Acked-by: "Eric W. Biederman" Cc: bigeasy@linutronix.de Link: https://lkml.kernel.org/r/20180525090507.22248-2-anna-maria@linutronix.de --- include/linux/rcupdate.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index e679b175b411..65163aa0bb04 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -652,9 +652,7 @@ static inline void rcu_read_lock(void) * Unfortunately, this function acquires the scheduler's runqueue and * priority-inheritance spinlocks. This means that deadlock could result * if the caller of rcu_read_unlock() already holds one of these locks or - * any lock that is ever acquired while holding them; or any lock which - * can be taken from interrupt context because rcu_boost()->rt_mutex_lock() - * does not disable irqs while taking ->wait_lock. + * any lock that is ever acquired while holding them. * * That said, RCU readers are never priority boosted unless they were * preempted. Therefore, one way to avoid deadlock is to make sure -- cgit v1.2.3 From 59dc6f3c6d81c0c4379025c4eb56919391d62b67 Mon Sep 17 00:00:00 2001 From: Anna-Maria Gleixner Date: Fri, 25 May 2018 11:05:07 +0200 Subject: signal: Remove no longer required irqsave/restore Commit a841796f11c9 ("signal: align __lock_task_sighand() irq disabling and RCU") introduced a rcu read side critical section with interrupts disabled. The changelog suggested that a better long-term fix would be "to make rt_mutex_unlock() disable irqs when acquiring the rt_mutex structure's ->wait_lock". This long-term fix has been made in commit b4abf91047cf ("rtmutex: Make wait_lock irq safe") for a different reason. Therefore revert commit a841796f11c9 ("signal: align > __lock_task_sighand() irq disabling and RCU") as the interrupt disable dance is not longer required. The change was tested on the base of b4abf91047cf ("rtmutex: Make wait_lock irq safe") with a four hour run of rcutorture scenario TREE03 with lockdep enabled as suggested by Paul McKenney. Signed-off-by: Anna-Maria Gleixner Signed-off-by: Thomas Gleixner Acked-by: Paul E. McKenney Acked-by: "Eric W. Biederman" Cc: bigeasy@linutronix.de Link: https://lkml.kernel.org/r/20180525090507.22248-3-anna-maria@linutronix.de --- kernel/signal.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index 0f865d67415d..8d8a940422a8 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1244,19 +1244,12 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, { struct sighand_struct *sighand; + rcu_read_lock(); for (;;) { - /* - * Disable interrupts early to avoid deadlocks. - * See rcu_read_unlock() comment header for details. - */ - local_irq_save(*flags); - rcu_read_lock(); sighand = rcu_dereference(tsk->sighand); - if (unlikely(sighand == NULL)) { - rcu_read_unlock(); - local_irq_restore(*flags); + if (unlikely(sighand == NULL)) break; - } + /* * This sighand can be already freed and even reused, but * we rely on SLAB_TYPESAFE_BY_RCU and sighand_ctor() which @@ -1268,15 +1261,12 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, * __exit_signal(). In the latter case the next iteration * must see ->sighand == NULL. */ - spin_lock(&sighand->siglock); - if (likely(sighand == tsk->sighand)) { - rcu_read_unlock(); + spin_lock_irqsave(&sighand->siglock, *flags); + if (likely(sighand == tsk->sighand)) break; - } - spin_unlock(&sighand->siglock); - rcu_read_unlock(); - local_irq_restore(*flags); + spin_unlock_irqrestore(&sighand->siglock, *flags); } + rcu_read_unlock(); return sighand; } -- cgit v1.2.3