Date:   Thu, 10 Mar 2022 12:00:32 -0800
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Phil Auld <pauld@...hat.com>,
        Alex Belits <abelits@...vell.com>,
        Nicolas Saenz Julienne <nsaenz@...nel.org>,
        Xiongfeng Wang <wangxiongfeng2@...wei.com>,
        Neeraj Upadhyay <quic_neeraju@...cinc.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Yu Liao <liaoyu15@...wei.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        Uladzislau Rezki <uladzislau.rezki@...y.com>,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH 10/19] rcu/context_tracking: Move dynticks counter to
 context tracking

On Wed, Mar 02, 2022 at 04:48:01PM +0100, Frederic Weisbecker wrote:
> In order to prepare for merging RCU dynticks counter into the context
> tracking state, move the rcu_data's dynticks field to the context
> tracking structure. It will later be mixed within the context tracking
> state itself.
> 
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>

Assuming that the context_tracking_state.h definitions get everywhere
they need to go in all configurations:

Acked-by: Paul E. McKenney <paulmck@...nel.org>

> Cc: Paul E. McKenney <paulmck@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Neeraj Upadhyay <quic_neeraju@...cinc.com>
> Cc: Uladzislau Rezki <uladzislau.rezki@...y.com>
> Cc: Joel Fernandes <joel@...lfernandes.org>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Nicolas Saenz Julienne <nsaenz@...nel.org>
> Cc: Marcelo Tosatti <mtosatti@...hat.com>
> Cc: Xiongfeng Wang <wangxiongfeng2@...wei.com>
> Cc: Yu Liao<liaoyu15@...wei.com>
> Cc: Phil Auld <pauld@...hat.com>
> Cc: Paul Gortmaker<paul.gortmaker@...driver.com>
> Cc: Alex Belits <abelits@...vell.com>
> ---
>  include/linux/context_tracking_state.h | 10 ++++-
>  kernel/context_tracking.c              |  9 ++--
>  kernel/rcu/tree.c                      | 59 ++++++++++++++------------
>  kernel/rcu/tree.h                      |  1 -
>  kernel/rcu/tree_exp.h                  |  2 +-
>  kernel/rcu/tree_stall.h                |  4 +-
>  6 files changed, 48 insertions(+), 37 deletions(-)
> 
> diff --git a/include/linux/context_tracking_state.h b/include/linux/context_tracking_state.h
> index cdc692caa01d..5ad0e481c5a3 100644
> --- a/include/linux/context_tracking_state.h
> +++ b/include/linux/context_tracking_state.h
> @@ -7,6 +7,7 @@
>  #include <linux/context_tracking_irq.h>
>  
>  struct context_tracking {
> +#ifdef CONFIG_CONTEXT_TRACKING_USER
>  	/*
>  	 * When active is false, probes are unset in order
>  	 * to minimize overhead: TIF flags are cleared
> @@ -21,11 +22,16 @@ struct context_tracking {
>  		CONTEXT_USER,
>  		CONTEXT_GUEST,
>  	} state;
> +#endif
> +	atomic_t dynticks;		/* Even value for idle, else odd. */
>  };
>  
> -#ifdef CONFIG_CONTEXT_TRACKING_USER
> -extern struct static_key_false context_tracking_key;
> +#ifdef CONFIG_CONTEXT_TRACKING
>  DECLARE_PER_CPU(struct context_tracking, context_tracking);
> +#endif
> +
> +#ifdef CONFIG_CONTEXT_TRACKING_USER
> +extern struct static_key_false context_tracking_key;
>  
>  static __always_inline bool context_tracking_enabled(void)
>  {
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index ea22eb04750f..77b61a7c9890 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -30,9 +30,6 @@
>  DEFINE_STATIC_KEY_FALSE(context_tracking_key);
>  EXPORT_SYMBOL_GPL(context_tracking_key);
>  
> -DEFINE_PER_CPU(struct context_tracking, context_tracking);
> -EXPORT_SYMBOL_GPL(context_tracking);
> -
>  static noinstr bool context_tracking_recursion_enter(void)
>  {
>  	int recursion;
> @@ -236,6 +233,12 @@ void __init context_tracking_init(void)
>  
>  #endif /* #ifdef CONFIG_CONTEXT_TRACKING_USER */
>  
> +DEFINE_PER_CPU(struct context_tracking, context_tracking) = {
> +		.dynticks = ATOMIC_INIT(1),
> +};
> +EXPORT_SYMBOL_GPL(context_tracking);
> +
> +
>  void ct_idle_enter(void)
>  {
>  	rcu_idle_enter();
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index cadf5f5a4700..96eb8503f28e 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -77,7 +77,6 @@
>  static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
>  	.dynticks_nesting = 1,
>  	.dynticks_nmi_nesting = DYNTICK_IRQ_NONIDLE,
> -	.dynticks = ATOMIC_INIT(1),
>  #ifdef CONFIG_RCU_NOCB_CPU
>  	.cblist.flags = SEGCBLIST_RCU_CORE,
>  #endif
> @@ -268,7 +267,7 @@ void rcu_softirq_qs(void)
>   */
>  static noinline noinstr unsigned long rcu_dynticks_inc(int incby)
>  {
> -	return arch_atomic_add_return(incby, this_cpu_ptr(&rcu_data.dynticks));
> +	return arch_atomic_add_return(incby, this_cpu_ptr(&context_tracking.dynticks));
>  }
>  
>  /*
> @@ -324,9 +323,9 @@ static noinstr void rcu_dynticks_eqs_exit(void)
>   */
>  static void rcu_dynticks_eqs_online(void)
>  {
> -	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  
> -	if (atomic_read(&rdp->dynticks) & 0x1)
> +	if (atomic_read(&ct->dynticks) & 0x1)
>  		return;
>  	rcu_dynticks_inc(1);
>  }
> @@ -338,17 +337,19 @@ static void rcu_dynticks_eqs_online(void)
>   */
>  static __always_inline bool rcu_dynticks_curr_cpu_in_eqs(void)
>  {
> -	return !(arch_atomic_read(this_cpu_ptr(&rcu_data.dynticks)) & 0x1);
> +	return !(arch_atomic_read(this_cpu_ptr(&context_tracking.dynticks)) & 0x1);
>  }
>  
>  /*
>   * Snapshot the ->dynticks counter with full ordering so as to allow
>   * stable comparison of this counter with past and future snapshots.
>   */
> -static int rcu_dynticks_snap(struct rcu_data *rdp)
> +static int rcu_dynticks_snap(int cpu)
>  {
> +	struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);
> +
>  	smp_mb();  // Fundamental RCU ordering guarantee.
> -	return atomic_read_acquire(&rdp->dynticks);
> +	return atomic_read_acquire(&ct->dynticks);
>  }
>  
>  /*
> @@ -363,9 +364,7 @@ static bool rcu_dynticks_in_eqs(int snap)
>  /* Return true if the specified CPU is currently idle from an RCU viewpoint.  */
>  bool rcu_is_idle_cpu(int cpu)
>  {
> -	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> -
> -	return rcu_dynticks_in_eqs(rcu_dynticks_snap(rdp));
> +	return rcu_dynticks_in_eqs(rcu_dynticks_snap(cpu));
>  }
>  
>  /*
> @@ -375,7 +374,7 @@ bool rcu_is_idle_cpu(int cpu)
>   */
>  static bool rcu_dynticks_in_eqs_since(struct rcu_data *rdp, int snap)
>  {
> -	return snap != rcu_dynticks_snap(rdp);
> +	return snap != rcu_dynticks_snap(rdp->cpu);
>  }
>  
>  /*
> @@ -384,11 +383,11 @@ static bool rcu_dynticks_in_eqs_since(struct rcu_data *rdp, int snap)
>   */
>  bool rcu_dynticks_zero_in_eqs(int cpu, int *vp)
>  {
> -	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> +	struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);
>  	int snap;
>  
>  	// If not quiescent, force back to earlier extended quiescent state.
> -	snap = atomic_read(&rdp->dynticks) & ~0x1;
> +	snap = atomic_read(&ct->dynticks) & ~0x1;
>  
>  	smp_rmb(); // Order ->dynticks and *vp reads.
>  	if (READ_ONCE(*vp))
> @@ -396,7 +395,7 @@ bool rcu_dynticks_zero_in_eqs(int cpu, int *vp)
>  	smp_rmb(); // Order *vp read and ->dynticks re-read.
>  
>  	// If still in the same extended quiescent state, we are good!
> -	return snap == atomic_read(&rdp->dynticks);
> +	return snap == atomic_read(&ct->dynticks);
>  }
>  
>  /*
> @@ -620,6 +619,7 @@ EXPORT_SYMBOL_GPL(rcutorture_get_gp_data);
>  static noinstr void rcu_eqs_enter(bool user)
>  {
>  	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  
>  	WARN_ON_ONCE(rdp->dynticks_nmi_nesting != DYNTICK_IRQ_NONIDLE);
>  	WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
> @@ -633,12 +633,12 @@ static noinstr void rcu_eqs_enter(bool user)
>  
>  	lockdep_assert_irqs_disabled();
>  	instrumentation_begin();
> -	trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, atomic_read(&rdp->dynticks));
> +	trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, atomic_read(&ct->dynticks));
>  	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
>  	rcu_preempt_deferred_qs(current);
>  
>  	// instrumentation for the noinstr rcu_dynticks_eqs_enter()
> -	instrument_atomic_write(&rdp->dynticks, sizeof(rdp->dynticks));
> +	instrument_atomic_write(&ct->dynticks, sizeof(ct->dynticks));
>  
>  	instrumentation_end();
>  	WRITE_ONCE(rdp->dynticks_nesting, 0); /* Avoid irq-access tearing. */
> @@ -740,7 +740,7 @@ noinstr void rcu_user_enter(void)
>   * rcu_nmi_exit - inform RCU of exit from NMI context
>   *
>   * If we are returning from the outermost NMI handler that interrupted an
> - * RCU-idle period, update rdp->dynticks and rdp->dynticks_nmi_nesting
> + * RCU-idle period, update ct->dynticks and rdp->dynticks_nmi_nesting
>   * to let the RCU grace-period handling know that the CPU is back to
>   * being RCU-idle.
>   *
> @@ -749,6 +749,7 @@ noinstr void rcu_user_enter(void)
>   */
>  noinstr void rcu_nmi_exit(void)
>  {
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>  
>  	instrumentation_begin();
> @@ -766,7 +767,7 @@ noinstr void rcu_nmi_exit(void)
>  	 */
>  	if (rdp->dynticks_nmi_nesting != 1) {
>  		trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2,
> -				  atomic_read(&rdp->dynticks));
> +				  atomic_read(&ct->dynticks));
>  		WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
>  			   rdp->dynticks_nmi_nesting - 2);
>  		instrumentation_end();
> @@ -774,11 +775,11 @@ noinstr void rcu_nmi_exit(void)
>  	}
>  
>  	/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
> -	trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, atomic_read(&rdp->dynticks));
> +	trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, atomic_read(&ct->dynticks));
>  	WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
>  
>  	// instrumentation for the noinstr rcu_dynticks_eqs_enter()
> -	instrument_atomic_write(&rdp->dynticks, sizeof(rdp->dynticks));
> +	instrument_atomic_write(&ct->dynticks, sizeof(ct->dynticks));
>  	instrumentation_end();
>  
>  	// RCU is watching here ...
> @@ -817,6 +818,7 @@ void rcu_irq_exit_check_preempt(void)
>   */
>  static void noinstr rcu_eqs_exit(bool user)
>  {
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  	struct rcu_data *rdp;
>  	long oldval;
>  
> @@ -836,9 +838,9 @@ static void noinstr rcu_eqs_exit(bool user)
>  	instrumentation_begin();
>  
>  	// instrumentation for the noinstr rcu_dynticks_eqs_exit()
> -	instrument_atomic_write(&rdp->dynticks, sizeof(rdp->dynticks));
> +	instrument_atomic_write(&ct->dynticks, sizeof(ct->dynticks));
>  
> -	trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, atomic_read(&rdp->dynticks));
> +	trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, atomic_read(&ct->dynticks));
>  	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
>  	WRITE_ONCE(rdp->dynticks_nesting, 1);
>  	WARN_ON_ONCE(rdp->dynticks_nmi_nesting);
> @@ -944,7 +946,7 @@ void __rcu_irq_enter_check_tick(void)
>  /**
>   * rcu_nmi_enter - inform RCU of entry to NMI context
>   *
> - * If the CPU was idle from RCU's viewpoint, update rdp->dynticks and
> + * If the CPU was idle from RCU's viewpoint, update ct->dynticks and
>   * rdp->dynticks_nmi_nesting to let the RCU grace-period handling know
>   * that the CPU is active.  This implementation permits nested NMIs, as
>   * long as the nesting level does not overflow an int.  (You will probably
> @@ -957,6 +959,7 @@ noinstr void rcu_nmi_enter(void)
>  {
>  	long incby = 2;
>  	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
> +	struct context_tracking *ct = this_cpu_ptr(&context_tracking);
>  
>  	/* Complain about underflow. */
>  	WARN_ON_ONCE(rdp->dynticks_nmi_nesting < 0);
> @@ -980,9 +983,9 @@ noinstr void rcu_nmi_enter(void)
>  
>  		instrumentation_begin();
>  		// instrumentation for the noinstr rcu_dynticks_curr_cpu_in_eqs()
> -		instrument_atomic_read(&rdp->dynticks, sizeof(rdp->dynticks));
> +		instrument_atomic_read(&ct->dynticks, sizeof(ct->dynticks));
>  		// instrumentation for the noinstr rcu_dynticks_eqs_exit()
> -		instrument_atomic_write(&rdp->dynticks, sizeof(rdp->dynticks));
> +		instrument_atomic_write(&ct->dynticks, sizeof(ct->dynticks));
>  
>  		incby = 1;
>  	} else if (!in_nmi()) {
> @@ -994,7 +997,7 @@ noinstr void rcu_nmi_enter(void)
>  
>  	trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
>  			  rdp->dynticks_nmi_nesting,
> -			  rdp->dynticks_nmi_nesting + incby, atomic_read(&rdp->dynticks));
> +			  rdp->dynticks_nmi_nesting + incby, atomic_read(&ct->dynticks));
>  	instrumentation_end();
>  	WRITE_ONCE(rdp->dynticks_nmi_nesting, /* Prevent store tearing. */
>  		   rdp->dynticks_nmi_nesting + incby);
> @@ -1138,7 +1141,7 @@ static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp)
>   */
>  static int dyntick_save_progress_counter(struct rcu_data *rdp)
>  {
> -	rdp->dynticks_snap = rcu_dynticks_snap(rdp);
> +	rdp->dynticks_snap = rcu_dynticks_snap(rdp->cpu);
>  	if (rcu_dynticks_in_eqs(rdp->dynticks_snap)) {
>  		trace_rcu_fqs(rcu_state.name, rdp->gp_seq, rdp->cpu, TPS("dti"));
>  		rcu_gpnum_ovf(rdp->mynode, rdp);
> @@ -4125,7 +4128,7 @@ rcu_boot_init_percpu_data(int cpu)
>  	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(rcu_dynticks_in_eqs(rcu_dynticks_snap(rdp)));
> +	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;
>  	rdp->rcu_ofl_gp_flags = RCU_GP_CLEANED;
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index b8d07bf92d29..15246a3f0734 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -188,7 +188,6 @@ struct rcu_data {
>  	int dynticks_snap;		/* Per-GP tracking for dynticks. */
>  	long dynticks_nesting;		/* Track process nesting level. */
>  	long dynticks_nmi_nesting;	/* Track irq/NMI nesting level. */
> -	atomic_t dynticks;		/* Even value for idle, else odd. */
>  	bool rcu_need_heavy_qs;		/* GP old, so heavy quiescent state! */
>  	bool rcu_urgent_qs;		/* GP old need light quiescent state. */
>  	bool rcu_forced_tick;		/* Forced tick to provide QS. */
> diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
> index d5f30085b0cf..2210110990f4 100644
> --- a/kernel/rcu/tree_exp.h
> +++ b/kernel/rcu/tree_exp.h
> @@ -358,7 +358,7 @@ static void sync_rcu_exp_select_node_cpus(struct work_struct *wp)
>  		    !(rnp->qsmaskinitnext & mask)) {
>  			mask_ofl_test |= mask;
>  		} else {
> -			snap = rcu_dynticks_snap(rdp);
> +			snap = rcu_dynticks_snap(cpu);
>  			if (rcu_dynticks_in_eqs(snap))
>  				mask_ofl_test |= mask;
>  			else
> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index 84b812a3ab44..202129b1c7e4 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -448,7 +448,7 @@ static void print_cpu_stall_info(int cpu)
>  	}
>  	delta = rcu_seq_ctr(rdp->mynode->gp_seq - rdp->rcu_iw_gp_seq);
>  	falsepositive = rcu_is_gp_kthread_starving(NULL) &&
> -			rcu_dynticks_in_eqs(rcu_dynticks_snap(rdp));
> +			rcu_dynticks_in_eqs(rcu_dynticks_snap(cpu));
>  	pr_err("\t%d-%c%c%c%c: (%lu %s) idle=%03x/%ld/%#lx softirq=%u/%u fqs=%ld %s\n",
>  	       cpu,
>  	       "O."[!!cpu_online(cpu)],
> @@ -458,7 +458,7 @@ static void print_cpu_stall_info(int cpu)
>  			rdp->rcu_iw_pending ? (int)min(delta, 9UL) + '0' :
>  				"!."[!delta],
>  	       ticks_value, ticks_title,
> -	       rcu_dynticks_snap(rdp) & 0xfff,
> +	       rcu_dynticks_snap(cpu) & 0xfff,
>  	       rdp->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,
> -- 
> 2.25.1
> 

Powered by blists - more mailing lists