[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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.

Thanks, Roger.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.