Re: [PATCH 02/17] locking: Add split_lock

From: Matthew Wilcox
Date: Mon Apr 12 2021 - 10:45:39 EST


On Mon, Apr 12, 2021 at 04:29:28PM +0200, Thomas Gleixner wrote:
> On Fri, Apr 09 2021 at 03:51, Matthew Wilcox wrote:
> > Bitlocks do not currently participate in lockdep. Conceptually, a
> > bit_spinlock is a split lock, eg across each bucket in a hash table.
> > The struct split_lock gives us somewhere to record the lockdep_map.
>
> I like the concept, but the name is strange. The only purpose of
>
> > +struct split_lock {
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > + struct lockdep_map dep_map;
> > +#endif
> > +};
>
> is to have a place to stick the lockdep map into. So it's not a lock
> construct as the name suggests, it's just auxiliary data when lockdep is
> enabled.

That's the implementation _today_, but conceptually, it's a single lock.
I was thinking that for non-RT, we could put a qspinlock in there for a
thread to spin on if the bit is contended. It'd need a bit of ingenuity
to make sure that a thread unlocking a bitlock made sure that a thread
spinning on the qspinlock saw the wakeup, but it should be doable.

Anyway, from the point of view of the user, they should be declaring
"this is the lock", not "this is the lock tracking structure", right?

> I know you hinted that RT could make use of that data structure and the
> fact that it's mandatory for the various lock functions, but that's not
> really feasible if this is related to a hash with a bit spinlock per
> bucket as the data structure is hash global.
>
> Sure, we can do pointer math to find out the bucket index and do
> something from there, but I'm not sure whether that really helps. Need
> to stare at the remaining few places where bit spinlocks are an issue on
> RT.

I obviously don't understand exactly what the RT patchset does. My
thinking was that you could handle the bit locks like rw sems, and
sacrifice the scalability of per-bucket-lock for the determinism of
a single PI lock.