Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 13 Mar 2022 00:23:07 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Frederic Weisbecker <frederic@kernel.org>
Cc:     LKML <linux-kernel@vger.kernel.org>, Phil Auld <pauld@redhat.com>,
        Alex Belits <abelits@marvell.com>,
        Nicolas Saenz Julienne <nsaenz@kernel.org>,
        Xiongfeng Wang <wangxiongfeng2@huawei.com>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Yu Liao <liaoyu15@huawei.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Marcelo Tosatti <mtosatti@redhat.com>,
        Paul Gortmaker <paul.gortmaker@windriver.com>,
        Uladzislau Rezki <uladzislau.rezki@sony.com>,
        Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH 11/19] rcu/context_tracking: Move dynticks_nesting to
 context tracking
Message-ID: <20220312232307.GJ28057@worktop.programming.kicks-ass.net>
References: <20220302154810.42308-1-frederic@kernel.org>
 <20220302154810.42308-12-frederic@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220302154810.42308-12-frederic@kernel.org>
Precedence: bulk

On Wed, Mar 02, 2022 at 04:48:02PM +0100, Frederic Weisbecker wrote:

> @@ -441,7 +440,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
>  	lockdep_assert_irqs_disabled();
>  
>  	/* Check for counter underflows */
> -	RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nesting) < 0,
> +	RCU_LOCKDEP_WARN(__this_cpu_read(context_tracking.dynticks_nesting) < 0,
>  			 "RCU dynticks_nesting counter underflow!");
>  	RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 0,
>  			 "RCU dynticks_nmi_nesting counter underflow/zero!");
> @@ -457,7 +456,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
>  	WARN_ON_ONCE(!nesting && !is_idle_task(current));
>  
>  	/* Does CPU appear to be idle from an RCU standpoint? */
> -	return __this_cpu_read(rcu_data.dynticks_nesting) == 0;
> +	return __this_cpu_read(context_tracking.dynticks_nesting) == 0;
>  }
>  
>  #define DEFAULT_RCU_BLIMIT (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD) ? 1000 : 10)

> @@ -798,7 +797,7 @@ void rcu_irq_exit_check_preempt(void)
>  {
>  	lockdep_assert_irqs_disabled();
>  
> -	RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nesting) <= 0,
> +	RCU_LOCKDEP_WARN(__this_cpu_read(context_tracking.dynticks_nesting) <= 0,
>  			 "RCU dynticks_nesting counter underflow/zero!");
>  	RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) !=
>  			 DYNTICK_IRQ_NONIDLE,

Would it make sense to create __ct_wrappers() to access the
dynticks_{nmi_,}_nesting counters ?


> @@ -4122,12 +4121,13 @@ static void rcu_init_new_rnp(struct rcu_node *rnp_leaf)
>  static void __init
>  rcu_boot_init_percpu_data(int cpu)
>  {
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
>  
>  	/* Set up local state, ensuring consistent view of global state. */
>  	rdp->grpmask = leaf_node_cpu_bit(rdp->mynode, cpu);
>  	INIT_WORK(&rdp->strict_work, strict_work_handler);
> -	WARN_ON_ONCE(rdp->dynticks_nesting != 1);
> +	WARN_ON_ONCE(ct->dynticks_nesting != 1);
>  	WARN_ON_ONCE(rcu_dynticks_in_eqs(rcu_dynticks_snap(cpu)));
>  	rdp->barrier_seq_snap = rcu_state.barrier_sequence;
>  	rdp->rcu_ofl_gp_seq = rcu_state.gp_seq;
> @@ -4152,6 +4152,7 @@ rcu_boot_init_percpu_data(int cpu)
>  int rcutree_prepare_cpu(unsigned int cpu)
>  {
>  	unsigned long flags;
> +	struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);
>  	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
>  	struct rcu_node *rnp = rcu_get_root();
>  

> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index 202129b1c7e4..30a5e0a8ddb3 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -429,6 +429,7 @@ static void print_cpu_stall_info(int cpu)
>  {
>  	unsigned long delta;
>  	bool falsepositive;
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
>  	char *ticks_title;
>  	unsigned long ticks_value;
> @@ -459,7 +460,7 @@ static void print_cpu_stall_info(int cpu)
>  				"!."[!delta],
>  	       ticks_value, ticks_title,
>  	       rcu_dynticks_snap(cpu) & 0xfff,
> -	       rdp->dynticks_nesting, rdp->dynticks_nmi_nesting,
> +	       ct->dynticks_nesting, rdp->dynticks_nmi_nesting,
>  	       rdp->softirq_snap, kstat_softirqs_cpu(RCU_SOFTIRQ, cpu),
>  	       data_race(rcu_state.n_force_qs) - rcu_state.n_force_qs_gpstart,
>  	       falsepositive ? " (false positive?)" : "");

And perhaps helpers here too? RCU grubbing in the context_tracking
internals seems a bit yuck.