[2/2] lockdep: Up MAX_LOCKDEP_CHAINS
diff mbox

Message ID 20171129094636.18638-2-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Nov. 29, 2017, 9:46 a.m. UTC
cross-release ftl

From Chris:

"Fwiw, this isn't cross-release but us reloading the module many times,
creating a whole host of new lockclasses. Even more fun is when the
module gets a slightly different address and the new lock address hashes
into an old lock...

"I did think about a module-hook to revoke the stale lockclasses, but
that still leaves all the hashed chains.

"This particular nuisance was temporarily pushed back by teaching igt not
to reload i915.ko on a whim."

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Marta Lofstedt <marta.lofstedt@intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=103707
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 kernel/locking/lockdep_internals.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Wilson Nov. 29, 2017, 9:57 a.m. UTC | #1
Quoting Daniel Vetter (2017-11-29 09:46:36)
> cross-release ftl
> 
> From Chris:
> 
> "Fwiw, this isn't cross-release but us reloading the module many times,
> creating a whole host of new lockclasses. Even more fun is when the
> module gets a slightly different address and the new lock address hashes
> into an old lock...
> 
> "I did think about a module-hook to revoke the stale lockclasses, but
> that still leaves all the hashed chains.
> 
> "This particular nuisance was temporarily pushed back by teaching igt not
> to reload i915.ko on a whim."
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=103707
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Yes, I think we need to carry this for CI. cross-release will add many
more chains, and since we already exhausted the previous array we will
need more (and just reducing the number of module reloads only gives a
temporary respite). Doubling the array doesn't seem like it'll buy us
much time though? Could we afford 20 bits? Might we not also need to
expand the lockclasses array? That we could just double?

In principle acked-by for core-for-CI, I think we need a bit surer
ground before saying that in general lockdep needs a larger array.
-Chris
Chris Wilson Nov. 29, 2017, 10:01 a.m. UTC | #2
Quoting Chris Wilson (2017-11-29 09:57:14)
> Quoting Daniel Vetter (2017-11-29 09:46:36)
> > cross-release ftl
> > 
> > From Chris:
> > 
> > "Fwiw, this isn't cross-release but us reloading the module many times,
> > creating a whole host of new lockclasses. Even more fun is when the
> > module gets a slightly different address and the new lock address hashes
> > into an old lock...
> > 
> > "I did think about a module-hook to revoke the stale lockclasses, but
> > that still leaves all the hashed chains.
> > 
> > "This particular nuisance was temporarily pushed back by teaching igt not
> > to reload i915.ko on a whim."
> > 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103707
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> In principle acked-by for core-for-CI, I think we need a bit surer
> ground before saying that in general lockdep needs a larger array.

For upstreaming, I wonder if we could sell them on a kconfig? That way
Tomi could adjust it more easily in his kconfig.git than having us
provide a patch.
-Chris
Daniel Vetter Nov. 29, 2017, 10:27 a.m. UTC | #3
On Wed, Nov 29, 2017 at 10:01:09AM +0000, Chris Wilson wrote:
> Quoting Chris Wilson (2017-11-29 09:57:14)
> > Quoting Daniel Vetter (2017-11-29 09:46:36)
> > > cross-release ftl
> > > 
> > > From Chris:
> > > 
> > > "Fwiw, this isn't cross-release but us reloading the module many times,
> > > creating a whole host of new lockclasses. Even more fun is when the
> > > module gets a slightly different address and the new lock address hashes
> > > into an old lock...
> > > 
> > > "I did think about a module-hook to revoke the stale lockclasses, but
> > > that still leaves all the hashed chains.
> > > 
> > > "This particular nuisance was temporarily pushed back by teaching igt not
> > > to reload i915.ko on a whim."
> > > 
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103707
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >
> > In principle acked-by for core-for-CI, I think we need a bit surer
> > ground before saying that in general lockdep needs a larger array.
> 
> For upstreaming, I wonder if we could sell them on a kconfig? That way
> Tomi could adjust it more easily in his kconfig.git than having us
> provide a patch.

I'll submit both (once CI is taken care of) to lockdep folks and hear what
they think about this issue. Maybe fixing module reload is already on
their plans, maybe not.

Same really for the kthread fix, lockdep code (and maintainers) are funky
enough that I don't really dwell too much in there ...
-Daniel

Patch
diff mbox

diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index d459d624ba2a..41630a5385c6 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -69,7 +69,7 @@  enum {
 #else
 #define MAX_LOCKDEP_ENTRIES	32768UL
 
-#define MAX_LOCKDEP_CHAINS_BITS	16
+#define MAX_LOCKDEP_CHAINS_BITS	17
 
 /*
  * Stack-trace: tightly packed array of stack backtrace