linux-kernel - Re: [RFC 0/2] srcu: Remove pre-flip memory barrier

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: <20221220140714.GB22763@lothringen>
Date:   Tue, 20 Dec 2022 15:07:14 +0100
From:   Frederic Weisbecker <frederic@...nel.org>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     linux-kernel@...r.kernel.org,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...nel.org>, rcu@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [RFC 0/2] srcu: Remove pre-flip memory barrier

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...

> 
> Further, the comment does not talk about implicit memory ordering, it’s talking about explicit ordering due to B+C on one side, and E on the other.

Not arguing I'm also still confused by the comment...

Thanks.