Re: [Xen-devel] [PATCH] rwlock: allow recursive read locking when already locked in write mode

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 20, 2020 at 05:00:37PM +0100, Jürgen Groß wrote:
> On 20.02.20 16:57, Roger Pau Monné wrote:
> > On Thu, Feb 20, 2020 at 04:50:22PM +0100, Jan Beulich wrote:
> > > On 20.02.2020 16:17, Roger Pau Monné wrote:
> > > > On Thu, Feb 20, 2020 at 04:02:55PM +0100, Jan Beulich wrote:
> > > > > On 20.02.2020 15:11, Roger Pau Monné wrote:
> > > > > > On Thu, Feb 20, 2020 at 01:48:54PM +0100, Jan Beulich wrote:
> > > > > > > Another option is to use the recurse_cpu field of the
> > > > > > > associated spin lock: The field is used for recursive locks
> > > > > > > only, and hence the only conflict would be with
> > > > > > > _spin_is_locked(), which we don't (and in the future then
> > > > > > > also shouldn't) use on this lock.
> > > > > > 
> > > > > > I looked into that also, but things get more complicated AFAICT, as 
> > > > > > it's
> > > > > > not possible to atomically fetch the state of the lock and the owner
> > > > > > CPU at the same time. Neither you could set the LOCKED bit and the 
> > > > > > CPU
> > > > > > at the same time.
> > > > > 
> > > > > There's no need to atomically fetch both afaics: The field is
> > > > > valid only if the LOCKED bit is set. And when reading the
> > > > > field you only care about the value being equal to
> > > > > smp_processor_id(), i.e. it is fine to set LOCKED before
> > > > > updating the CPU field on lock, and to reset the CPU field to
> > > > > SPINLOCK_NO_CPU (or whatever it's called) before clearing
> > > > > LOCKED.
> > > > 
> > > > Yes that would require the usage of a sentinel value as 0 would be a
> > > > valid processor ID.
> > > > 
> > > > I would try to refrain from (abu)sing internal spinlock fields, as I
> > > > think we can solve this just by using the cnts field on the current
> > > > rwlock implementation.
> > > > 
> > > > What issue do you have in mind that would prevent storing the CPU
> > > > write locked in the cnts field?
> > > 
> > > The reduction of the number of bits used for other purposes.
> > 
> > I think it should be fine for now, as we would support systems with up
> > to 16384 CPU IDs, and 65536 concurrent read lockers, which mean each
> > CPU could take the lock up to 4 times recursively. Note that
> > supporting 16384 CPUs is still very, very far off the radar.
> 
> Current spinlock implementation limits NR_CPUS to 4096.

Right, but here we can use up to 14 bits, so I think it's best to
assign them now than leave them as padding?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel