linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, tj@kernel.org, johannes.berg@intel.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 25/27] locking/lockdep: Add support for dynamic keys
Date: Thu, 29 Nov 2018 08:59:27 -0800	[thread overview]
Message-ID: <1543510767.185366.145.camel@acm.org> (raw)
In-Reply-To: <20181129120418.GI2131@hirez.programming.kicks-ass.net>

On Thu, 2018-11-29 at 13:04 +0100, Peter Zijlstra wrote:
> On Wed, Nov 28, 2018 at 03:43:23PM -0800, Bart Van Assche wrote:
> > A shortcoming of the current lockdep implementation is that it requires
> > lock keys to be allocated statically. That forces certain lock objects
> > to share lock keys. Since lock dependency analysis groups lock objects
> > per key sharing lock keys can cause false positive lockdep reports.
> > Make it possible to avoid such false positive reports by allowing lock
> > keys to be allocated dynamically. Require that dynamically allocated
> > lock keys are registered before use by calling lockdep_register_key().
> > Complain about attempts to register the same lock key pointer twice
> > without calling lockdep_unregister_key() between successive
> > registration calls.
> >  struct lock_class_key {
> > +	struct hlist_node		hash_entry;
> >  	struct lockdep_subclass_key	subkeys[MAX_LOCKDEP_SUBCLASSES];
> >  };
> 
> That hash_entry is purely for that double-register warning, right? I
> wonder if we can do that differently; by always doing
> register_lock_class(), and checking that state.

Hi Peter,

The hash_entry serves two purposes. One purpose is to verify whether the
lockdep_register_key() and  lockdep_unregister_key() functions are used
correctly. A second purpose is to avoid that lockdep_init_map() complains
when encountering a dynamically allocated key. I'm not sure how always
doing register_lock_class() would help?

Thanks,

Bart.

  reply	other threads:[~2018-11-29 16:59 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28 23:42 [PATCH 00/27] locking/lockdep: Add support for dynamic keys Bart Van Assche
2018-11-28 23:42 ` [PATCH 01/27] lockdep tests: Display compiler warning and error messages Bart Van Assche
2018-11-28 23:43 ` [PATCH 02/27] lockdep tests: Fix shellcheck warnings Bart Van Assche
2018-11-28 23:43 ` [PATCH 03/27] lockdep tests: Improve testing accuracy Bart Van Assche
2018-11-28 23:43 ` [PATCH 04/27] lockdep tests: Run lockdep tests a second time under Valgrind Bart Van Assche
2018-11-28 23:43 ` [PATCH 05/27] liblockdep: Rename "trywlock" into "trywrlock" Bart Van Assche
2018-11-28 23:43 ` [PATCH 06/27] liblockdep: Add dummy print_irqtrace_events() implementation Bart Van Assche
2018-11-28 23:43 ` [PATCH 07/27] lockdep tests: Test the lockdep_reset_lock() implementation Bart Van Assche
2018-11-28 23:43 ` [PATCH 08/27] locking/lockdep: Declare local symbols static Bart Van Assche
2018-11-28 23:43 ` [PATCH 09/27] locking/lockdep: Inline __lockdep_init_map() Bart Van Assche
2018-11-28 23:43 ` [PATCH 10/27] locking/lockdep: Introduce lock_class_cache_is_registered() Bart Van Assche
2018-11-28 23:43 ` [PATCH 11/27] timekeeping: Assign a name to tk_core.seq.dep_map Bart Van Assche
2018-12-05 10:03   ` [tip:timers/core] timekeeping: Use proper seqcount initializer tip-bot for Bart Van Assche
2018-11-28 23:43 ` [PATCH 12/27] net/core: Assign a name to devnet_rename_seq.dep_map Bart Van Assche
2018-11-29  0:45   ` David Miller
2018-11-28 23:43 ` [PATCH 13/27] locking/lockdep: Complain if a lock object has no name Bart Van Assche
2018-11-28 23:43 ` [PATCH 14/27] locking/lockdep: Remove a superfluous INIT_LIST_HEAD() statement Bart Van Assche
2018-11-28 23:43 ` [PATCH 15/27] locking/lockdep: Make concurrent lockdep_reset_lock() calls safe Bart Van Assche
2018-11-28 23:43 ` [PATCH 16/27] locking/lockdep: Stop using RCU primitives to access all_lock_classes Bart Van Assche
2018-11-28 23:43 ` [PATCH 17/27] locking/lockdep: Make zap_class() remove all matching lock order entries Bart Van Assche
2018-11-28 23:43 ` [PATCH 18/27] locking/lockdep: Reorder struct lock_class members Bart Van Assche
2018-11-28 23:43 ` [PATCH 19/27] locking/lockdep: Retain the class key and name while freeing a lock class Bart Van Assche
2018-11-28 23:43 ` [PATCH 20/27] locking/lockdep: Free lock classes that are no longer in use Bart Van Assche
2018-11-29 10:37   ` Peter Zijlstra
2018-11-28 23:43 ` [PATCH 21/27] locking/lockdep: Rename lock_list.entry into lock_list.lock_order_entry Bart Van Assche
2018-11-28 23:43 ` [PATCH 22/27] locking/lockdep: Reuse list entries that are no longer in use Bart Van Assche
2018-11-29 10:49   ` Peter Zijlstra
2018-11-29 12:01     ` Peter Zijlstra
2018-11-29 16:48       ` Bart Van Assche
2018-12-01 20:24         ` Peter Zijlstra
2018-12-03 16:40           ` Bart Van Assche
2018-12-03 17:32             ` Peter Zijlstra
2018-12-03 18:16               ` Bart Van Assche
2018-12-04  8:14                 ` Peter Zijlstra
2018-12-04 16:08                   ` Bart Van Assche
2018-11-28 23:43 ` [PATCH 23/27] locking/lockdep: Check data structure consistency Bart Van Assche
2018-11-29 12:30   ` Peter Zijlstra
2018-11-29 16:50     ` Bart Van Assche
2018-11-29 16:59       ` Peter Zijlstra
2018-11-28 23:43 ` [PATCH 24/27] locking/lockdep: Introduce __lockdep_free_key_range() Bart Van Assche
2018-11-29 10:00   ` Peter Zijlstra
2018-11-28 23:43 ` [PATCH 25/27] locking/lockdep: Add support for dynamic keys Bart Van Assche
2018-11-29 10:10   ` Peter Zijlstra
2018-12-03 17:07     ` Bart Van Assche
2018-12-03 17:31       ` Peter Zijlstra
2018-11-29 12:04   ` Peter Zijlstra
2018-11-29 16:59     ` Bart Van Assche [this message]
2018-11-28 23:43 ` [PATCH 26/27] kernel/workqueue: Use dynamic lockdep keys for workqueues Bart Van Assche
2018-11-28 23:43 ` [PATCH 27/27] lockdep tests: Test dynamic key registration Bart Van Assche
2018-11-29 12:31 ` [PATCH 00/27] locking/lockdep: Add support for dynamic keys Peter Zijlstra

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=1543510767.185366.145.camel@acm.org \
    --to=bvanassche@acm.org \
    --cc=johannes.berg@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tj@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).