On Tue, Mar 04, 2025 at 11:56:18AM +0100, Uladzislau Rezki wrote:
> On Tue, Mar 04, 2025 at 11:52:26AM +0100, Uladzislau Rezki wrote:
> > > > Did I get that right?
> > > >
> > >
> > > Other than I'm unable to follow what do you mean "WH has not been
> > > injected, so nothing to wait on", maybe because I am missing some
> > > terminology from you ;-) I think it's a good analysis, thank you!
> > >
> > > > I think this is a real bug AFAICS, hoping all the memory barriers are in
> > > > place to make sure the code reordering also correctly orders the
> > > > accesses.
> > > > I'll double check that.
> > > >
> > > > I also feel its 'theoretical', because as long as rcu_gp_init() and
> > > > rcu_gp_cleanup() are properly ordered WRT pre-existing readers, then
> > > > synchronize_rcu_normal() still waits for pre-existing readers even
> > > > though its
> > > > a bit confused about the value of the cookies.
> > > >
> > > > For the fix,
> > > > Reviewed-by: Joel Fernandes (Google) <j...@joelfernandes.org>
> > > >
> > > > (If possible, include a Link: to my (this) post so that the sequence of
> > > > events is further clarified.)
> > > >
> > >
> > > Will add the tag (with the email you really want ;-)) and a link to this
> > > email to the patch. Thanks!
> > >
> >
> > CPU_1: | CPU_2:
> > # Increase a seq-number |
> > rcu_seq_start(&rcu_state.gp_seq); |
> > | add_client() {
> > | # Record a gp-sec state
> > |
> > get_state_synchronize_rcu_full(&rs.oldstate);
> > | }
> > |
> > | rcu_sr_normal_gp_init() {
> > | add a dummy-wait-head;
> > | }
> >
> >
> > A client has been added with already updated gp-sec number, i.e.
> > "oldstate" would refer to this GP, not to previous. A
> > poll_state_synchronize_rcu_full()
> > will complain because this GP is not passed, it will on a next iteration.
> >
> > This is how i see this.
> >
> Updated the plain-text, removed tabs:
>
> CPU_1: | CPU_2:
> # Increase a seq-number |
> rcu_seq_start(&rcu_state.gp_seq); |
> | add_client() {
> | # Record a gp-sec state
> |
> get_state_synchronize_rcu_full(&rs.oldstate);
> | }
> |
> | rcu_sr_normal_gp_init() {
> | add a dummy-wait-head;
> | }
>
Thank you. I added links from you and Joel as the detailed explanation
to the commit log, and the comment I proposed[1].
[1]: https://lore.kernel.org/rcu/Z8SnhS_LnzN_wvxr@tardis/
Regards,
Boqun
> --
> Uladzislau Rezki
