linux-kernel - Re: [PATCH RFC v2 rcu 1/8] srcu: Convert ->srcu_lock_count and ->srcu_unlock

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: <87ill4vrb9.fsf@jogness.linutronix.de>
Date:   Fri, 30 Sep 2022 17:08:18 +0206
From:   John Ogness <john.ogness@...utronix.de>
To:     "Paul E. McKenney" <paulmck@...nel.org>, rcu@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, kernel-team@...com,
        rostedt@...dmis.org, "Paul E. McKenney" <paulmck@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Petr Mladek <pmladek@...e.com>
Subject: Re: [PATCH RFC v2 rcu 1/8] srcu: Convert ->srcu_lock_count and
 ->srcu_unlock_count to atomic

Hi Paul,

On 2022-09-29, "Paul E. McKenney" <paulmck@...nel.org> wrote:
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 1c304fec89c0..6fd0665f4d1f 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -636,7 +636,7 @@ int __srcu_read_lock(struct srcu_struct *ssp)
>  	int idx;
>  
>  	idx = READ_ONCE(ssp->srcu_idx) & 0x1;
> -	this_cpu_inc(ssp->sda->srcu_lock_count[idx]);
> +	this_cpu_inc(ssp->sda->srcu_lock_count[idx].counter);
>  	smp_mb(); /* B */  /* Avoid leaking the critical section. */
>  	return idx;
>  }

Is there any particular reason that you are directly modifying @counter
instead of raw_cpu_ptr()+atomic_long_inc() that do you in
__srcu_read_lock_nmisafe() of patch 2?

> @@ -650,7 +650,7 @@ EXPORT_SYMBOL_GPL(__srcu_read_lock);
>  void __srcu_read_unlock(struct srcu_struct *ssp, int idx)
>  {
>  	smp_mb(); /* C */  /* Avoid leaking the critical section. */
> -	this_cpu_inc(ssp->sda->srcu_unlock_count[idx]);
> +	this_cpu_inc(ssp->sda->srcu_unlock_count[idx].counter);
>  }
>  EXPORT_SYMBOL_GPL(__srcu_read_unlock);

Ditto.

> @@ -1687,8 +1687,8 @@ void srcu_torture_stats_print(struct srcu_struct *ssp, char *tt, char *tf)
>  			struct srcu_data *sdp;
>  
>  			sdp = per_cpu_ptr(ssp->sda, cpu);
> -			u0 = data_race(sdp->srcu_unlock_count[!idx]);
> -			u1 = data_race(sdp->srcu_unlock_count[idx]);
> +			u0 = data_race(sdp->srcu_unlock_count[!idx].counter);
> +			u1 = data_race(sdp->srcu_unlock_count[idx].counter);
>  
>  			/*
>  			 * Make sure that a lock is always counted if the corresponding

And instead of atomic_long_read().

> @@ -1696,8 +1696,8 @@ void srcu_torture_stats_print(struct srcu_struct *ssp, char *tt, char *tf)
>  			 */
>  			smp_rmb();
>  
> -			l0 = data_race(sdp->srcu_lock_count[!idx]);
> -			l1 = data_race(sdp->srcu_lock_count[idx]);
> +			l0 = data_race(sdp->srcu_lock_count[!idx].counter);
> +			l1 = data_race(sdp->srcu_lock_count[idx].counter);
>  
>  			c0 = l0 - u0;
>  			c1 = l1 - u1;

Ditto.

John Ogness