linux-kernel - Re: [RFC][PATCH v2 08/11] context_tracking,rcu: Replace RCU dynticks counter with context

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 29 Sep 2021 20:54:35 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     gor@...ux.ibm.com, jpoimboe@...hat.com, jikos@...nel.org,
        mbenes@...e.cz, pmladek@...e.com, mingo@...nel.org
Cc:     linux-kernel@...r.kernel.org, joe.lawrence@...hat.com,
        fweisbec@...il.com, tglx@...utronix.de, hca@...ux.ibm.com,
        svens@...ux.ibm.com, sumanthk@...ux.ibm.com,
        live-patching@...r.kernel.org, paulmck@...nel.org,
        rostedt@...dmis.org, x86@...nel.org
Subject: Re: [RFC][PATCH v2 08/11] context_tracking,rcu: Replace RCU dynticks
 counter with context_tracking

On Wed, Sep 29, 2021 at 05:17:31PM +0200, Peter Zijlstra wrote:
> XXX I'm pretty sure I broke task-trace-rcu.

> -static noinstr void rcu_dynticks_eqs_enter(void)
> -{
> -	int seq;
> -
> -	/*
> -	 * CPUs seeing atomic_add_return() must see prior RCU read-side
> -	 * critical sections, and we also must force ordering with the
> -	 * next idle sojourn.
> -	 */
> -	rcu_dynticks_task_trace_enter();  // Before ->dynticks update!
> -	seq = rcu_dynticks_inc(1);
> -	// RCU is no longer watching.  Better be in extended quiescent state!
> -	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && (seq & 0x1));
> -}

> -static noinstr void rcu_dynticks_eqs_exit(void)
> -{
> -	int seq;
> -
> -	/*
> -	 * CPUs seeing atomic_add_return() must see prior idle sojourns,
> -	 * and we also must force ordering with the next RCU read-side
> -	 * critical section.
> -	 */
> -	seq = rcu_dynticks_inc(1);
> -	// RCU is now watching.  Better not be in an extended quiescent state!
> -	rcu_dynticks_task_trace_exit();  // After ->dynticks update!
> -	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !(seq & 0x1));
> -}

So specifically rcu_dynticks_task_trace_{enter,exit}() are now orphaned.
After this patch, nothing calls them.

However, looking at this again, we've got:

  __context_tracking_enter()
    rcu_user_enter()
      rcu_eqs_enter()
        rcu_dynticks_eqs_enter()
	  rcu_dynticks_task_trace_enter()
	  rcu_dynticks_inc();
	rcu_dynticks_task_enter();

    ct_seq_user_enter()
      atomic_add_return()

and on the other end:

  __context_tracking_exit()
    ct_seq_user_exit()
      atomic_add_return()

    rcu_user_exit()
      rcu_esq_exit()
        rcu_dynticks_task_exit()
	rcu_dynticks_eqs_exit()
	  rcu_dynticks_inc()
	  rcu_dynticks_task_trace_exit()

And since we want to replace dynticks_inc() with ct_seq_*() the
rcu_dynticks_task_{enter,exit}() ought to be pulled before that..

Instead I orphaned rcu_dynticks_task_trace_{enter,exit}() which should
more or less stay where they are.

I seems to have confused the two :/