On 2023-06-23 17:38:45 [+0200], Petr Mladek wrote:
> > If the writer increments the counter,
> > after the reader unlocks, then the reader observers the odd seq number
> > and locks immediately (within read_seqbegin()) on the lock without
> > spending cycles within the read section (outside of read_seqbegin()) and
> > learning about the seq increment in read_seqretry().
>
> I am afraid that it will not get caugh:
>
> static inline int do___read_seqcount_retry(const seqcount_t *s, unsigned start)
> {
> kcsan_atomic_next(0);
> return unlikely(READ_ONCE(s->sequence) != start);
> }
>
> It checks if the sequence is the same. It does not check if it is odd.
You do realise that __read_seqcount_begin() is
| while ((__seq = seqprop_sequence(s)) & 1) \
| cpu_relax(); \
so the code you try to modify is within seqprop_sequence() so you _do_
loop until the resulting __seq is even. And only then you continue
towards do___read_seqcount_retry().
> But it might be odd if you read "start" outsize of the lock and do
> not check if it is odd.
no, see above.
> I do not feel to be expert but a code:
>
> spin_lock(s->lock);
> spin_unlock(s->lock);
> s = locked_variable;
>
> just triggers many bells in my head. It is like from a book
> "Classic locking mistakes".
It is a counter and not locked variable and it is designed to be read
outside of the lock. The usage of the lock is not for "protecting" as
locking reasons but for making progress.
> With the code:
>
> spin_lock(s->lock);
> seq s->seqcount.sequence;
> spin_unlock(s->lock);
>
> you will always get the right value without any extra cost. The code
> took the lock anyway. And the single assignment is negligible.
> And RT could schedule inside the spin_lock anyway.
>
> Or do I miss anything?
Yes. See above. If after the unlock the read sequence is odd again we do
notice it. Doing it from within the locked section, we don't.
> Best Regards,
> Petr
Sebastian
