diff mbox

[2/2] lockdep: Up MAX_LOCKDEP_CHAINS

Message ID 20171129154145.26755-3-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Nov. 29, 2017, 3:41 p.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

Peter Zijlstra Nov. 30, 2017, 7:47 a.m. UTC | #1
On Wed, Nov 29, 2017 at 04:41:45PM +0100, Daniel Vetter wrote:
> 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...

Yeah, this is a known issue, just reboot.

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

Its an absolute royal pain to remove all the resources consumed by a
module, and if you manage you then have to deal with fragmented storage
-- that is, we need to go keep track of which entries are used.

Its a giant heap of complexity that's just not worth it.


Given all that, I don't see why we should up this. Just don't reload
modules (or better, don't use modules at all).
Daniel Vetter Nov. 30, 2017, 8:08 a.m. UTC | #2
On Thu, Nov 30, 2017 at 08:47:18AM +0100, Peter Zijlstra wrote:
> On Wed, Nov 29, 2017 at 04:41:45PM +0100, Daniel Vetter wrote:
> > 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...
> 
> Yeah, this is a known issue, just reboot.
> 
> > "I did think about a module-hook to revoke the stale lockclasses, but
> > that still leaves all the hashed chains.
> 
> Its an absolute royal pain to remove all the resources consumed by a
> module, and if you manage you then have to deal with fragmented storage
> -- that is, we need to go keep track of which entries are used.
> 
> Its a giant heap of complexity that's just not worth it.
> 
> 
> Given all that, I don't see why we should up this. Just don't reload
> modules (or better, don't use modules at all).

We use excessive amounts of module reloading to validate the failure paths
of driver load. Rebooting takes too much time. I guess we could look into
just rebinding the driver without reloading, that should take the pain off
lockdep. Meanwhile we can carry this locally.

I just included this to check whether you have any plans, thanks for
clarifying that this is not worth it from a core perspective to get fixed.
The real issue we have in CI is the one the first patch papers over.
-Daniel
diff mbox

Patch

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