On Tue, Dec 20, 2022 at 09:20:08AM -0500, Joel Fernandes wrote:
> > On Dec 20, 2022, at 9:07 AM, Frederic Weisbecker <frederic@xxxxxxxxxx> wrote:
> >
> > On Tue, Dec 20, 2022 at 08:44:40AM -0500, Joel Fernandes wrote:
> >>> C w-depend-r
> >>>
> >>> {
> >>> PLOCK=LOCK0;
> >>> }
> >>>
> >>> // updater
> >>> P0(int *LOCK1, int **PLOCK)
> >>> {
> >>> int lock1;
> >>>
> >>> lock1 = READ_ONCE(*LOCK1); // READ from inactive idx
> >>> smp_mb();
> >>> WRITE_ONCE(*PLOCK, LOCK1); // Flip idx
> >>> }
> >>>
> >>> // reader
> >>> P1(int **PLOCK)
> >>> {
> >>> int *plock;
> >>>
> >>> plock = READ_ONCE(*PLOCK); // Read active idx
> >>> WRITE_ONCE(*plock, 1); // Write to active idx
> >>
> >> I am a bit lost here, why would the reader want to write to the active idx?
> >> The reader does not update the idx, only the lock count.
> >
> > So &ssp->sda->srcu_lock_count is the base address and idx is the offset, right?
> > The write is then displayed that way:
> >
> > this_cpu_inc(ssp->sda->srcu_lock_count[idx].counter);
> >
> > But things could be also thought the other way around with idx being the base address and
> > ssp->sda->srcu_lock_count being the offset.
> >
> > this_cpu_inc(idx[ssp->sda->srcu_lock_count].counter);
> >
> > That would require to change some high level types but the result would be the same from
> > the memory point of view (and even from the ASM point of view). In the end we
> > are dealing with the same address and access.
> >
> > Now ssp->sda->srcu_lock_count is a constant address value. It doesn't change.
> > So it can be zero for example. Then the above increment becomes:
> >
> > this_cpu_inc(idx.counter);
> >
> > And then it can be modelized as in the above litmus test.
> >
> > I had to play that trick because litmus doesn't support arrays but I believe
> > it stands. Now of course I may well have got something wrong since I've always
> > been terrible at maths...
>
> Ah ok, I get where you were going with that. Yes there is address dependency
> between reading idx and writing lock count. But IMHO, the access on the update
> side is trying to order write to index, and reads from a lock count of a
> previous index (as far as E / B+C is concerned). So IMHO, on the read side you
> have to consider 2 consecutive readers and not the same reader in order to
> pair the same accesses correctly. But I could be missing something.
And you're right, for the first part of the comment (let's call that (1)):
* Ensure that if this updater saw a given reader's increment
* from __srcu_read_lock(), that reader was using an old value
* of ->srcu_idx.
My litmus test shows the ordering displayed in the second part of the comment
(call it (2)):
* Also ensure that if a given reader sees the
* new value of ->srcu_idx, this updater's earlier scans cannot
* have seen that reader's increments (which is OK, because this
* grace period need not wait on that reader).
_ In (1), E indeed pairs with B and C
_ In (2), E pairs with the address-dependency between idx and lock_count.
Thanks.
