All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: neilb@suse.de, peterz@infradead.org, mingo@redhat.com,
	will@kernel.org, longman@redhat.com, boqun.feng@gmail.com,
	bigeasy@linutronix.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/17] locking: Add split_lock
Date: Mon, 12 Apr 2021 15:45:25 +0100	[thread overview]
Message-ID: <20210412144525.GM2531743@casper.infradead.org> (raw)
In-Reply-To: <87blaj1sqf.ffs@nanos.tec.linutronix.de>

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.

  reply	other threads:[~2021-04-12 14:45 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 01/17] x86: Rename split_lock_init to sld_init Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 02/17] locking: Add split_lock Matthew Wilcox (Oracle)
2021-04-12 14:29   ` Thomas Gleixner
2021-04-12 14:45     ` Matthew Wilcox [this message]
2021-04-12 15:01       ` Thomas Gleixner
2021-05-11  7:46       ` Peter Zijlstra
2021-04-09  2:51 ` [PATCH 03/17] bit_spinlock: Prepare for split_locks Matthew Wilcox (Oracle)
2021-04-09 14:32   ` Theodore Ts'o
2021-04-09 14:35     ` Matthew Wilcox
2021-04-09 14:55       ` Theodore Ts'o
2021-04-09  2:51 ` [PATCH 04/17] hlist_bl: " Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 05/17] dm-snap: Add dm_exceptional_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 06/17] dcache: Add d_hash_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 07/17] fscache: Add cookie_hash_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 08/17] gfs2: Add qd_hash_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 09/17] mbcache: Add mb_cache_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 10/17] hlist_bl: Make the split_lock parameter mandatory Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 11/17] s390: Add airq_iv_lock Matthew Wilcox (Oracle)
2021-04-09  6:18   ` kernel test robot
2021-04-09  6:18     ` kernel test robot
2021-04-09 13:20     ` Matthew Wilcox
2021-04-09 13:20       ` Matthew Wilcox
2021-04-09  2:51 ` [PATCH 12/17] zram: Add zram_table_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 13/17] jbd2: Add jbd2_jh_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 14/17] slub: Add slab_page_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 15/17] zsmalloc: Add zs_pin_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 16/17] rhashtable: Convert to split_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep Matthew Wilcox (Oracle)
2021-04-09  6:37   ` kernel test robot
2021-04-09  6:37     ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210412144525.GM2531743@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=bigeasy@linutronix.de \
    --cc=boqun.feng@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=neilb@suse.de \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.