From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D54B0C433E0 for ; Fri, 7 Aug 2020 02:59:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AF4A121744 for ; Fri, 7 Aug 2020 02:59:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1596769171; bh=rohTHTvBnOfXgLz546XmcHOe65GGSth3l9B42BKK52U=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=sAXNOOVWwR13fnK0sJFAS3DRwaGlyxSFqLD+bTXekN0kpkE867nZE3PVt+TTGkkro 8lIMIwjTPCWxwMye05mHthYjbI5XV9FxwXwUriS0ZJabgLpykqGkQ5tlDaBaTR6BDH p4/MOkTnKAJIVEUjZVRcEYBSoEmGy8/k3N9gdYPA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726058AbgHGC7b (ORCPT ); Thu, 6 Aug 2020 22:59:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:43950 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726038AbgHGC7a (ORCPT ); Thu, 6 Aug 2020 22:59:30 -0400 Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com [209.85.208.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D1D7122D01; Fri, 7 Aug 2020 02:59:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1596769170; bh=rohTHTvBnOfXgLz546XmcHOe65GGSth3l9B42BKK52U=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=LGUOG4xAAqNl5JK0ekaICDXc6dnE2jHVF1DS1L3bqOvRtbENdMSjKbKoas2QLjvnq 0V4Qhpzk4T0E/en+jHOc/4WRkHTexxWdMm7lw1EqJWDQhheo7OG9eMd435oFzsXgqq bCfbshAUDyck54ZmI6PyryxbkdaXV7S2JWyDuA1U= Received: by mail-lj1-f178.google.com with SMTP id z14so626443ljm.1; Thu, 06 Aug 2020 19:59:29 -0700 (PDT) X-Gm-Message-State: AOAM531oCDGQ7gGH1IIpYoFfcPr0dXBa+jPy5G5AxIGrIGVo/A9FuZHD Yt4vbUJC8zHVJNj+LOk3IE7WoyfKjRS810fAIHQ= X-Google-Smtp-Source: ABdhPJy3cyMLEpLQp+e9nmXl1yojXJ9/wlMyQ5xgEtncZI62/v4i1cFVkr3ygiVhv/Sk+csRIK1JhBPw/s7GD1hAbPs= X-Received: by 2002:a2e:2f02:: with SMTP id v2mr4659800ljv.79.1596769167982; Thu, 06 Aug 2020 19:59:27 -0700 (PDT) MIME-Version: 1.0 References: <1596725454-16245-1-git-send-email-guoren@kernel.org> <20200806114850.051f84d0@oasis.local.home> In-Reply-To: <20200806114850.051f84d0@oasis.local.home> From: Guo Ren Date: Fri, 7 Aug 2020 10:59:16 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] ftrace: Fixup lockdep assert held of text_mutex To: Steven Rostedt , Palmer Dabbelt , Paul Walmsley Cc: Ingo Molnar , Linux Kernel Mailing List , linux-csky@vger.kernel.org, Guo Ren , linux-riscv Content-Type: text/plain; charset="UTF-8" Sender: linux-csky-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-csky@vger.kernel.org On Thu, Aug 6, 2020 at 11:48 PM Steven Rostedt wrote: > > On Thu, 6 Aug 2020 14:50:54 +0000 > guoren@kernel.org wrote: > > > From: Guo Ren > > > > The function ftrace_process_locs() will modify text code, so we > > should give a text_mutex lock. Because some arch's patch code > > will assert held of text_mutex even during start_kernel-> > > ftrace_init(). > > NAK. > > This looks like a bug in the lockdep_assert_held() in whatever arch > (riscv) is running. Seems you think it's a bug of arch implementation with the wrong usage of text_mutex? Also @riscv maintainer, How about modifying it in riscv's code? we still need to solve it. ---------------- diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index ace8a6e..fb266c3 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -23,6 +23,12 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr) struct dyn_arch_ftrace { }; + +#ifdef CONFIG_DYNAMIC_FTRACE +struct dyn_ftrace; +int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); +#define ftrace_init_nop ftrace_init_nop +#endif #endif #ifdef CONFIG_DYNAMIC_FTRACE diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index 2ff63d0..9e9f7c0 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -97,6 +97,17 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, return __ftrace_modify_call(rec->ip, addr, false); } +int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) +{ + int ret; + + mutex_lock(&text_mutex); + ret = ftrace_make_nop(mod, rec, MCOUNT_ADDR); + mutex_unlock(&text_mutex); + + return ret; +} + int ftrace_update_ftrace_func(ftrace_func_t func) { int ret = __ftrace_modify_call((unsigned long)&ftrace_call, ------------------- > > > > > backtrace log: > > assert by lockdep_assert_held(&text_mutex) > > 0 patch_insn_write (addr=0xffffffe0000010fc , insn=0xffffffe001203eb8, len=8) at arch/riscv/kernel/patch.c:63 > > 1 0xffffffe0002042ec in patch_text_nosync (addr=, insns=, len=) at arch/riscv/kernel/patch.c:93 > > 2 0xffffffe00020628e in __ftrace_modify_call (hook_pos=, target=, enable=) at arch/riscv/kernel/ftrace.c:68 > > 3 0xffffffe0002063c0 in ftrace_make_nop (mod=, rec=0xffffffe001221c70 , addr=18446743936272720288) at arch/riscv/kernel/ftrace.c:97 > > 4 0xffffffe0002b13f0 in ftrace_init_nop (rec=, mod=) at ./include/linux/ftrace.h:647 > > 5 ftrace_nop_initialize (rec=, mod=) at kernel/trace/ftrace.c:2619 > > 6 ftrace_update_code (new_pgs=, mod=) at kernel/trace/ftrace.c:3063 > > 7 ftrace_process_locs (mod=, start=, end=) at kernel/trace/ftrace.c:6154 > > 8 0xffffffe00000b6e6 in ftrace_init () at kernel/trace/ftrace.c:6715 > > 9 0xffffffe000001b48 in start_kernel () at init/main.c:888 > > 10 0xffffffe0000010a8 in _start_kernel () at arch/riscv/kernel/head.S:247 > > > > Signed-off-by: Guo Ren > > Cc: Steven Rostedt > > Cc: Ingo Molnar > > --- > > kernel/trace/ftrace.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > > index 1903b80..4b48b88 100644 > > --- a/kernel/trace/ftrace.c > > +++ b/kernel/trace/ftrace.c > > @@ -26,6 +26,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -6712,9 +6713,11 @@ void __init ftrace_init(void) > > ftrace_init() is called before SMP is initialized. Nothing else should > be running here. That means grabbing a mutex is useless. I don't agree, ftrace_init are modifying kernel text, so we should give the lock of text_mutex to keep semantic consistency. > > -- Steve > > > > > > last_ftrace_enabled = ftrace_enabled = 1; > > > > + mutex_lock(&text_mutex); > > ret = ftrace_process_locs(NULL, > > __start_mcount_loc, > > __stop_mcount_loc); > > + mutex_unlock(&text_mutex); > > > > pr_info("ftrace: allocated %ld pages with %ld groups\n", > > ftrace_number_of_pages, ftrace_number_of_groups); > -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/