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

Mathieu Desnoyers
2022-12-21T17:21:37Z

On 2022-12-21 07:11, Frederic Weisbecker wrote:
> On Tue, Dec 20, 2022 at 10:43:25PM -0500, Mathieu Desnoyers wrote:
>> On 2022-12-20 19:58, Frederic Weisbecker wrote:
>>> On Wed, Dec 21, 2022 at 01:49:57AM +0100, Frederic Weisbecker wrote:
>>>> On Tue, Dec 20, 2022 at 07:15:00PM -0500, Joel Fernandes wrote:
>>>>> On Tue, Dec 20, 2022 at 5:45 PM Frederic Weisbecker <frederic@kernel.org> wrote:
>>>>> Agreed about (1).
>>>>>
>>>>>> _ In (2), E pairs with the address-dependency between idx and lock_count.
>>>>>
>>>>> But that is not the only reason. If that was the only reason for (2),
>>>>> then there is an smp_mb() just before the next-scan post-flip before
>>>>> the lock counts are read.
>>>>
>>>> The post-flip barrier makes sure the new idx is visible on the next READER's
>>>> turn, but it doesn't protect against the fact that "READ idx then WRITE lock[idx]"
>>>> may appear unordered from the update side POV if there is no barrier between the
>>>> scan and the flip.
>>>>
>>>> If you remove the smp_mb() from the litmus test I sent, things explode.
>>>
>>> Or rather, look at it the other way, if there is no barrier between the lock
>>> scan and the index flip (E), then the index flip can appear to be written before the
>>> lock is read. Which means you may start activating the index before you finish
>>> reading it (at least it appears that way from the readers pont of view).
>>
>> Considering that you can have pre-existing readers from arbitrary index
>> appearing anywhere in the grace period (because a reader can fetch the
>> index and be preempted for an arbitrary amount of time before incrementing
>> the lock count), the grace period algorithm needs to deal with the fact that
>> a newcoming reader can appear in a given index either before or after the
>> flip.
>
> True but the number of preempted tasks is bound and there is a forward progress guarantee.
>
>> I don't see how flipping the index before or after loading the unlock/lock
>> values would break anything (except for unlikely counter overflow situations
>> as previously discussed).
>
> Forward progress guarantee.

Considering a coherent cache, the store-buffer will ensure that the
index flip eventually reaches all readers. This bounds the time during
which readers can flood the current index, and therefore guarantees
forward progress. AFAIK the Linux kernel does not support architectures
with incoherent caches.

So I still don't see how having the barrier before or after the index
flip is useful for forward progress.

Thanks,

Mathieu

>
> Thanks.
>
>>
>> Thanks,
>>
>> Mathieu
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> https://www.efficios.com
>>

--
Mathieu Desnoyers
EfficiOS Inc.