linux-kernel - Re: [PATCH 11/19] rcu/context_tracking: Move dynticks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220312232307.GJ28057@worktop.programming.kicks-ass.net>
Date:   Sun, 13 Mar 2022 00:23:07 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.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>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        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 11/19] rcu/context_tracking: Move dynticks_nesting to
 context tracking

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.