* [PATCH] x86: fix get_wchan() not support the ORC unwinder @ 2021-06-11 12:46 Qi Zheng 2021-06-14 16:21 ` Andy Lutomirski 0 siblings, 1 reply; 3+ messages in thread From: Qi Zheng @ 2021-06-11 12:46 UTC (permalink / raw) To: tglx, mingo, bp, hpa, peterz, x86; +Cc: linux-kernel, songmuchun, Qi Zheng Currently, the kernel CONFIG_UNWINDER_ORC option is enabled by default on x86, but the implementation of get_wchan() is still based on the frame pointer unwinder, so the /proc/<pid>/wchan always return 0 regardless of whether the task <pid> is running. We reimplement the get_wchan() by calling stack_trace_save_tsk(), which is adapted to the ORC and frame pointer unwinders. Fixes: ee9f8fce9964(x86/unwind: Add the ORC unwinder) Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> --- arch/x86/kernel/process.c | 51 +++-------------------------------------------- 1 file changed, 3 insertions(+), 48 deletions(-) diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 5e1f38179f49..976a36918ed7 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -928,58 +928,13 @@ unsigned long arch_randomize_brk(struct mm_struct *mm) */ unsigned long get_wchan(struct task_struct *p) { - unsigned long start, bottom, top, sp, fp, ip, ret = 0; - int count = 0; + unsigned long entry = 0; if (p == current || p->state == TASK_RUNNING) return 0; - if (!try_get_task_stack(p)) - return 0; - - start = (unsigned long)task_stack_page(p); - if (!start) - goto out; - - /* - * Layout of the stack page: - * - * ----------- topmax = start + THREAD_SIZE - sizeof(unsigned long) - * PADDING - * ----------- top = topmax - TOP_OF_KERNEL_STACK_PADDING - * stack - * ----------- bottom = start - * - * The tasks stack pointer points at the location where the - * framepointer is stored. The data on the stack is: - * ... IP FP ... IP FP - * - * We need to read FP and IP, so we need to adjust the upper - * bound by another unsigned long. - */ - top = start + THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING; - top -= 2 * sizeof(unsigned long); - bottom = start; - - sp = READ_ONCE(p->thread.sp); - if (sp < bottom || sp > top) - goto out; - - fp = READ_ONCE_NOCHECK(((struct inactive_task_frame *)sp)->bp); - do { - if (fp < bottom || fp > top) - goto out; - ip = READ_ONCE_NOCHECK(*(unsigned long *)(fp + sizeof(unsigned long))); - if (!in_sched_functions(ip)) { - ret = ip; - goto out; - } - fp = READ_ONCE_NOCHECK(*(unsigned long *)fp); - } while (count++ < 16 && p->state != TASK_RUNNING); - -out: - put_task_stack(p); - return ret; + stack_trace_save_tsk(p, &entry, 1, 0); + return entry; } long do_arch_prctl_common(struct task_struct *task, int option, -- 2.11.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: fix get_wchan() not support the ORC unwinder 2021-06-11 12:46 [PATCH] x86: fix get_wchan() not support the ORC unwinder Qi Zheng @ 2021-06-14 16:21 ` Andy Lutomirski 2021-06-16 7:34 ` [External] " 郑琦 0 siblings, 1 reply; 3+ messages in thread From: Andy Lutomirski @ 2021-06-14 16:21 UTC (permalink / raw) To: Qi Zheng, tglx, mingo, bp, hpa, peterz, x86; +Cc: linux-kernel, songmuchun On 6/11/21 5:46 AM, Qi Zheng wrote: > Currently, the kernel CONFIG_UNWINDER_ORC option is enabled by > default on x86, but the implementation of get_wchan() is still > based on the frame pointer unwinder, so the /proc/<pid>/wchan > always return 0 regardless of whether the task <pid> is running. > > We reimplement the get_wchan() by calling stack_trace_save_tsk(), > which is adapted to the ORC and frame pointer unwinders. How much slower does this make ps? --Andy ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [External] Re: [PATCH] x86: fix get_wchan() not support the ORC unwinder 2021-06-14 16:21 ` Andy Lutomirski @ 2021-06-16 7:34 ` 郑琦 0 siblings, 0 replies; 3+ messages in thread From: 郑琦 @ 2021-06-16 7:34 UTC (permalink / raw) To: Andy Lutomirski Cc: tglx, mingo, bp, hpa, peterz, x86, linux-kernel, songmuchun On Tue, Jun 15, 2021 at 12:21 AM Andy Lutomirski <luto@kernel.org> wrote: > > On 6/11/21 5:46 AM, Qi Zheng wrote: > > Currently, the kernel CONFIG_UNWINDER_ORC option is enabled by > > default on x86, but the implementation of get_wchan() is still > > based on the frame pointer unwinder, so the /proc/<pid>/wchan > > always return 0 regardless of whether the task <pid> is running. > > > > We reimplement the get_wchan() by calling stack_trace_save_tsk(), > > which is adapted to the ORC and frame pointer unwinders. > > How much slower does this make ps? I used the bpftrace tool to test the running time of get_wchan() in the two cases of the ORC and frame pointer unwinders, the test script and the result are as follows: the test script: bpftrace -e 'kprobe:get_wchan { @start[tid] = nsecs; } kretprobe: get_wchan /@start[tid]/ { @ns[comm] = hist(nsecs - @start[tid]); delete(@start[tid]); }' the result: 1) ORC unwinder ( before applying this patch ) @ns[ps]: [512, 1K) 4609 |@@@@@@@@@@@@ | [1K, 2K) 18599 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@| [2K, 4K) 1848 |@@@@@ | [4K, 8K) 307 | | [8K, 16K) 74 | | [16K, 32K) 12 | | 73% of the cases are in the [1K, 2K) range. Notice: In this case, the get_wchan() always returns the wrong value of 0. 2) ORC unwinder ( after applying this patch ) @ns[ps]: [512, 1K) 536 |@ | [1K, 2K) 19945 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@| [2K, 4K) 5604 |@@@@@@@@@@@@@@ | [4K, 8K) 246 | | [8K, 16K) 154 | | [16K, 32K) 18 | | 75% of the cases are in the [1K, 2K) range. 3) frame point unwinder ( before applying this patch ) @ns[ps]: [512, 1K) 245 | | [1K, 2K) 16577 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@| [2K, 4K) 2788 |@@@@@@@@ | [4K, 8K) 190 | | [8K, 16K) 74 | | [16K, 32K) 9 | | 83% of the cases are in the [1K, 2K) range. 4) frame point unwinder ( after applying this patch ) @ns[ps]: [512, 1K) 85 | | [1K, 2K) 12023 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@| [2K, 4K) 7418 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ | [4K, 8K) 232 |@ | [8K, 16K) 104 | | [16K, 32K) 18 | | 60% of the cases are in the [1K, 2K) range. In summary, the running time of get_wchan() has increased after applying this patch. But the get_wchan() is not the hotspot function, and this is a bug in the default ORC option, so I think these increased runtimes are acceptable. In addition, this issue has existed for nearly 4 years and no one has fixed it, if nobody cares about the return value of the get_wchan(), maybe we can return 0 or remove this function directly. What do you think? Best regards, Qi Zheng > > --Andy ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-16 7:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-11 12:46 [PATCH] x86: fix get_wchan() not support the ORC unwinder Qi Zheng 2021-06-14 16:21 ` Andy Lutomirski 2021-06-16 7:34 ` [External] " 郑琦
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).