linux-kernel - Re: [PATCH] rcu/doc: Add a quick quiz to explain further why we need smp_mb__after_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: <20210611103432.GA143096@lothringen>
Date:   Fri, 11 Jun 2021 12:34:32 +0200
From:   Frederic Weisbecker <frederic@...nel.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Neeraj Upadhyay <neeraju@...eaurora.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Uladzislau Rezki <urezki@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH] rcu/doc: Add a quick quiz to explain further why we need
 smp_mb__after_unlock_lock()

On Thu, Jun 10, 2021 at 09:57:10AM -0700, Paul E. McKenney wrote:
> diff --git a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst
> index 11cdab037bff..3cd5cb4d86e5 100644
> --- a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst
> +++ b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst
> @@ -112,6 +112,35 @@ on PowerPC.
>  The ``smp_mb__after_unlock_lock()`` invocations prevent this
>  ``WARN_ON()`` from triggering.
>  
> ++-----------------------------------------------------------------------+
> +| **Quick Quiz**:                                                       |
> ++-----------------------------------------------------------------------+
> +| But the whole chain of rcu_node-structure locking guarantees that     |
> +| readers see all pre-grace-period accesses from the updater and        |
> +| also guarantees that the updater to see all post-grace-period         |

Should it be either "that the updater see" or "the updater to see"?

> +| accesses from the readers.

Is it really post-grace-period that you meant here? The updater can't see
the future. It's rather all reader accesses before the end of the grace period?

>  So why do we need all of those calls      |
> +| to smp_mb__after_unlock_lock()?                                       |
> ++-----------------------------------------------------------------------+
> +| **Answer**:                                                           |
> ++-----------------------------------------------------------------------+
> +| Because we must provide ordering for RCU's polling grace-period       |
> +| primitives, for example, get_state_synchronize_rcu() and              |
> +| poll_state_synchronize_rcu().  For example:                           |

Two times "for example" (sorry I'm nitpicking...)

> +|                                                                       |
> +| CPU 0                                     CPU 1                       |
> +| ----                                      ----                        |
> +| WRITE_ONCE(X, 1)                          WRITE_ONCE(Y, 1)            |
> +| g = get_state_synchronize_rcu()           smp_mb()                    |
> +| while (!poll_state_synchronize_rcu(g))    r1 = READ_ONCE(X)           |
> +|         continue;                                                     |
> +| r0 = READ_ONCE(Y)                                                     |

Good point, it's a nice merge of the initial examples!

> +|                                                                       |
> +| RCU guarantees that that the outcome r0 == 0 && r1 == 0 will not      |

One "that" has to die here.

> +| happen, even if CPU 1 is in an RCU extended quiescent state (idle     |
> +| or offline) and thus won't interact directly with the RCU core        |
> +| processing at all.                                                    |

Thanks a lot!

> ++-----------------------------------------------------------------------+
> +
>  This approach must be extended to include idle CPUs, which need
>  RCU's grace-period memory ordering guarantee to extend to any
>  RCU read-side critical sections preceding and following the current