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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F4A2C4332F for ; Thu, 20 Oct 2022 17:54:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229635AbiJTRyf (ORCPT ); Thu, 20 Oct 2022 13:54:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41414 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229597AbiJTRye (ORCPT ); Thu, 20 Oct 2022 13:54:34 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5BDE029351; Thu, 20 Oct 2022 10:54:32 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 1873DB828AD; Thu, 20 Oct 2022 17:54:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CEFFBC433C1; Thu, 20 Oct 2022 17:54:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1666288469; bh=GCQ+1VSy9DlUaoSYI9kxL0iKmoCvhaDUmTklwlw81eY=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=mqMjGqb1QNR0m2u8JuW27pOAK0BbieZk2NryYAmCitBYiXwEmWbxM66M4pqnb/ifu FRCazSNNGNBJeRFGUBd1LqXd2CN0+n/L057yhKzdp5ctrXzi6mvz1Tktp9iOI2UFk4 37QMqJNwCl8W5b8OV+iB3bu18ZV6cZ9mmJUTnXnhGMw/zM9YAZQMK99vawnCV70GU1 xyauY1rET4DBmHJGKFKcu7b9CBzyvuM3fF/7JarDuB5D4k3kMub0YGVcvCkxwy6sbD LhpL2YDMjLV7egR8OIQ4hgYZfPr5AQvmbkw+TxPQvnD/0S8wS1DGY55s3jZ5bw+7kJ HG6ToQUzNjyfw== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id 708AE5C0624; Thu, 20 Oct 2022 10:54:29 -0700 (PDT) Date: Thu, 20 Oct 2022 10:54:29 -0700 From: "Paul E. McKenney" To: Steven Rostedt Cc: LKML , Linux Trace Kernel , Masami Hiramatsu , Andrew Morton , Mathieu Desnoyers , Joel Fernandes , Tom Zanussi Subject: Re: [PATCH] tracing: Add trace_trigger kernel command line option Message-ID: <20221020175429.GS5600@paulmck-ThinkPad-P17-Gen-1> Reply-To: paulmck@kernel.org References: <20221019200137.70343645@gandalf.local.home> <20221019200745.0152fc51@gandalf.local.home> <20221020155900.GM5600@paulmck-ThinkPad-P17-Gen-1> <20221020123357.0f90e823@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221020123357.0f90e823@gandalf.local.home> Precedence: bulk List-ID: X-Mailing-List: linux-trace-kernel@vger.kernel.org On Thu, Oct 20, 2022 at 12:33:57PM -0400, Steven Rostedt wrote: > On Thu, 20 Oct 2022 08:59:00 -0700 > "Paul E. McKenney" wrote: > > > On Wed, Oct 19, 2022 at 08:07:45PM -0400, Steven Rostedt wrote: > > > On Wed, 19 Oct 2022 20:01:37 -0400 > > > Steven Rostedt wrote: > > > > > > > @@ -90,6 +90,10 @@ int unregister_tracepoint_module_notifier(struct notifier_block *nb) > > > > #ifdef CONFIG_TRACEPOINTS > > > > static inline void tracepoint_synchronize_unregister(void) > > > > { > > > > + /* Early updates do not need synchronization */ > > > > + if (early_boot_irqs_disabled) > > > > + return; > > > > + > > > > synchronize_srcu(&tracepoint_srcu); > > > > synchronize_rcu(); > > > > > > I wonder if this check should be just moved to the RCU synchronization > > > code? That is, if early_boot_irqs_disabled is set, do nothing, as there > > > should be nothing to synchronize against. > > > > There already is a similar check, but it follows the lockdep check. > > > > Does the following patch help? > > Not sure if it would. > > I added this: > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index c03fd7037add..79ac31a6a87b 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -2748,9 +2748,12 @@ void trace_buffered_event_disable(void) > disable_trace_buffered_event, NULL, 1); > preempt_enable(); > > + if (!irqs_disabled()) > + printk("IRQS DISABLED!! before %s:%d\n", __func__, __LINE__); > /* Wait for all current users to finish */ > - if (!early_boot_irqs_disabled) > - synchronize_rcu(); > + synchronize_rcu(); > + if (!irqs_disabled()) > + printk("IRQS DISABLED!! after %s:%d\n", __func__, __LINE__); > > for_each_tracing_cpu(cpu) { > free_page((unsigned long)per_cpu(trace_buffered_event, cpu)); > > Which produced this: > > > [ 0.972867] ftrace: allocating 47021 entries in 184 pages > [ 0.978285] ftrace section at ffffffffacef74c0 sorted properly > [ 0.991153] ftrace: allocated 184 pages with 4 groups > [ 0.996163] Dynamic Preempt: full > [ 0.999354] rcu: Preemptible hierarchical RCU implementation. > [ 1.005020] rcu: RCU restricting CPUs from NR_CPUS=128 to nr_cpu_ids=8. > [ 1.011680] Tasks-RCU CPU stall warnings timeout set to 120000 (rcu_task_stall_timeout). > [ 1.019813] Trampoline variant of Tasks RCU enabled. > [ 1.024831] Rude variant of Tasks RCU enabled. > [ 1.029330] Tracing variant of Tasks RCU enabled. > [ 1.034089] rcu: RCU calculated value of scheduler-enlistment delay is 100 jiffies. > [ 1.041699] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=8 > [ 1.052607] IRQS DISABLED!! after trace_buffered_event_disable:2756 > [ 1.058712] NR_IRQS: 8448, nr_irqs: 488, preallocated irqs: 16 > [ 1.064670] NO_HZ: Full dynticks CPUs: 3. > [ 1.068501] rcu: Offload RCU callbacks from CPUs: 3. > [ 1.073519] rcu: srcu_init: Setting srcu_struct sizes based on contention. > [ 1.080400] ------------[ cut here ]------------ > [ 1.084938] Interrupts were enabled early > [ 1.088919] WARNING: CPU: 0 PID: 0 at init/main.c:1061 start_kernel+0x36f/0x4e8 > [ 1.096184] Modules linked in: > [ 1.099213] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.1.0-rc1-test+ #468 > [ 1.106047] Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016 > [ 1.114960] RIP: 0010:start_kernel+0x36f/0x4e8 > [ 1.119373] Code: 48 27 53 e8 fd 7b 03 00 e8 55 72 e5 fe e8 6b 05 03 00 ff 15 2a f9 89 ff 0f ba e0 09 73 0e 48 c7 c7 98 25 31 ac e8 01 5b df fe <0f> 0b c6 05 c7 2c af ff 00 e8 ba 36 47 fe ff 15 14 f9 89 ff e8 56 > [ 1.138057] RSP: 0000:ffffffffac603f30 EFLAGS: 00010286 > [ 1.143248] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 > [ 1.150343] RDX: 0000000000000002 RSI: 00000000ffffdfff RDI: 0000000000000001 > [ 1.157438] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffffac603dd8 > [ 1.164537] R10: 0000000000000003 R11: ffffffffac734168 R12: 000000000000180a > [ 1.171633] R13: 00000000010fecf0 R14: 0000000000000000 R15: 00000000d44e3018 > [ 1.178730] FS: 0000000000000000(0000) GS:ffff93ee1aa00000(0000) knlGS:0000000000000000 > [ 1.186778] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 1.192489] CR2: ffff93ee1edff000 CR3: 000000002e60a001 CR4: 00000000000606f0 > [ 1.199584] Call Trace: > [ 1.202009] > > > So I'm not sure how just updating srcu will do anything to prevent this. Apologies, SRCU on the brain due to a different email thread... :-/ Thanx, Paul > -- Steve > > > > > Thanx, Paul > > > > ------------------------------------------------------------------------ > > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > > index ca4b5dcec675b..356ef70d5442c 100644 > > --- a/kernel/rcu/srcutree.c > > +++ b/kernel/rcu/srcutree.c > > @@ -1267,14 +1267,15 @@ static void __synchronize_srcu(struct srcu_struct *ssp, bool do_norm) > > { > > struct rcu_synchronize rcu; > > > > + if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE) > > + return; > > + > > RCU_LOCKDEP_WARN(lockdep_is_held(ssp) || > > lock_is_held(&rcu_bh_lock_map) || > > lock_is_held(&rcu_lock_map) || > > lock_is_held(&rcu_sched_lock_map), > > "Illegal synchronize_srcu() in same-type SRCU (or in RCU) read-side critical section"); > > > > - if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE) > > - return; > > might_sleep(); > > check_init_srcu_struct(ssp); > > init_completion(&rcu.completion); >