Re: [PATCH v2 01/20] locking/rt: Use raw_spin_lock_irqsave() in __rwbase_read_unlock()

[Paolo Bonzini]

Il lun 1 giu 2026, 13:11 Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> ha scritto:
> On 2026-06-01 10:40:45 [+0200], Peter Zijlstra wrote:
> > > When raw_seqcount_try_begin() is used, the typical action when
> > > the seqcount is busy is to take the same lock that the writer uses.
> > > Another possibility, found in kernel/events/uprobes.c, is to delay
> > > the work to a safe point using RCU.  Either way there is no need
> > > to guarantee progress of the writer under CONFIG_PREEMPT_RT,
> > > because the caller is not going to call raw_seqcount_try_begin() in a loop.
>
> This does not happen. It only happens if the associated lock is
> preemptible. This is not the case for uprobes.c so there is no lock/
> unlock.

The uprobes reference is just an example of a different try_begin
pattern, that does not involve taking a lock but also doesn't need to
guarantee progress of the writer. uprobes uses it without a lock but
that doesn't have to be the case. I thought the commit message was
reasonably clear but perhaps I was wrong?

> > > However, raw_seqcount_try_begin() currently uses seqprop_sequence()
> > > via raw_read_seqcount(), and therefore does a lock/unlock of the
> > > write-side lock on PREEMPT_RT kernel.  This prevents it from being
> > > used in atomic context.  Use __seqprop_sequence() instead of
> > > raw_read_seqcount() to allow it.
>
> So it would work if there is no lock associated or it is something like
> raw_spinlock_t.

Yes, in which case the patch is a no-op. But in the KVM case that we
are thinking about in this thread, raw spinlocks are not needed
because the atomic-context read side would just punt if try_begin()
fails; and unlike the uprobes case, there would be a lock involved.
Since it'd be possible to use a regular spinlock, this patch would
help.

> > Yeah, I think this makes sense. Thanks!
>
> Do I make sense or do I miss something obvious? On the other it would
> make sense to have this even for spinlock_t assuming the try_begin
> version does not try spin and expect to make progress (which it does not
> for the current users).

That was my point. The lock/unlock is not causing any current bug in
the kernel, other than maybe some minor inefficiencies, but it makes
try_begin() subtly different for PREEMPT_RT kernels and the "obviously
correct" way to do it isn't the right one. Having to use &s->seqcount
is yucky.

The docs for try_begin() don't say it super explicitly, but still they
heavily imply you're not meant to spin on it. At that point it's not a
"try" anymore.

Thanks,

Paolo

>
> Sebastian
>