Re: [PATCH v2 0/3] Toyrcu: replace plain accesses with READ_ONCE() and

[Junchang Wang <junchangwang@gmail.com>]

On Thu, Apr 25, 2019 at 11:38 PM Akira Yokosawa <akiyks@gmail.com> wrote:
>
> Hi Junchang,
>
> On Thu, 25 Apr 2019 16:29:51 +0800, Junchang Wang wrote:
> > Hi Paul and Akira,
> >
> > This patch set replaces plain accesses and ACCESS_ONCE() with READ_ONCE()
> > and WRITE_ONCE() in Toy RCU examples. I will send another patch updating
> > tex file accordingly on top of this patch set.
>
> As far as accesses to rcu_gp_ctr, this patch set looks good to me.
>
> In updating toyrcu.tex file, I suggested to extract the snippets from
> code under CodeSamples/defer, but some snippets in toyrcu.tex consist of
> code from both .h and .c. Current scheme does not support such merging
> of snippets from multiple source files.
>
> Your option is to manually convert verbbox environment to Verbatim
> environment. Sorry I suggested infeasible extractions the other day.
> rcu_qs.c and rcu_qs.h are in separate snippets, and they can be
> extracted by fcvextract.pl. In this case, commit 4a41491166cd
> ("defer/hazptr: Extract snippet from hazptr.c") should be a good example,
> especially the "gobbleblank=yes" option.
>
> If you have any question, feel free to ask me. I'm looking forward to
> seeing your update.

Hi Akira,

Thanks for the note. Would be happy to submit a new patch for
toyrcu.tex according to your suggestions soon.

>
> Paul, in the sources Junchang has touched, per-thread variable
> "rcu_reader_gp" is updated by its owner, but is read accessed from
> for_each_thread loops in updater side. So I think accesses to each of
> them need to be annotated by WRITE_ONCE (in owner) and READ_ONCE (in updater).
> I looked at only the diffs in the 1st patch set and didn't notice the raciness
> of them.
>
> Macros for accessing per-thread variables are expanded to array references,
> and there are "barrier()"s in these loops.
> So I think there is little chance those accesses would be optimized by
> currently available compilers.
>

Great catch! My understanding is that one of the nice benefits of
READ_ONCE/WRITE_ONCE is that they explicitly notify programmers the
spots where there might be racy accesses. The more we eliminate them,
the harder a programmer verifies the code. Of course, that's my
current point of view. I would be happy to see your choices.

Thanks,
--Junchang

> Do you see the need to annotate the accesses to per-thread variable?
>
>         Thanks, Akira
>
> >
> > --
> > Junchang Wang (3):
> >   rcu: Use READ_ONCE() and WRITE_ONCE() for shared variable rcu_gp_ctr
> >   rcu_nest: Use READ_ONCE() and WRITE_ONCE() for shared variable
> >     rcu_gp_ctr
> >   rcu_qs: Use READ_ONCE() AND WRITE_ONCE() for shared variable
> >     rcu_gp_ctr
> >
> >  CodeSamples/defer/rcu.c      | 2 +-
> >  CodeSamples/defer/rcu.h      | 4 ++--
> >  CodeSamples/defer/rcu_nest.c | 2 +-
> >  CodeSamples/defer/rcu_nest.h | 2 +-
> >  CodeSamples/defer/rcu_qs.c   | 2 +-
> >  CodeSamples/defer/rcu_qs.h   | 4 ++--
> >  6 files changed, 8 insertions(+), 8 deletions(-)
> >
>