[PATCH v1 1/1] lockdep: Move hlock_equal() to the respective ifdeffery

[PATCH v1 1/1] lockdep: Move hlock_equal() to the respective ifdeffery

[Andy Shevchenko]

When hlock_equal() is unused, it prevents kernel builds with clang,
`make W=1` and CONFIG_WERROR=y, CONFIG_LOCKDEP=y and
CONFIG_LOCKDEP_SMALL=n:

  lockdep.c:2005:20: error: unused function 'hlock_equal' [-Werror,-Wunused-function]

Fix this by moving the function to the respective existing ifdeffery
for its the only user.

See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
inline functions for W=1 build").

Fixes: 68e305678583 ("lockdep: Adjust check_redundant() for recursive read change")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/locking/lockdep.c | 70 ++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 58d78a33ac65..546e92827de4 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1976,41 +1976,6 @@ print_circular_bug_header(struct lock_list *entry, unsigned int depth,
 	print_circular_bug_entry(entry, depth);
 }
 
-/*
- * We are about to add A -> B into the dependency graph, and in __bfs() a
- * strong dependency path A -> .. -> B is found: hlock_class equals
- * entry->class.
- *
- * If A -> .. -> B can replace A -> B in any __bfs() search (means the former
- * is _stronger_ than or equal to the latter), we consider A -> B as redundant.
- * For example if A -> .. -> B is -(EN)-> (i.e. A -(E*)-> .. -(*N)-> B), and A
- * -> B is -(ER)-> or -(EN)->, then we don't need to add A -> B into the
- * dependency graph, as any strong path ..-> A -> B ->.. we can get with
- * having dependency A -> B, we could already get a equivalent path ..-> A ->
- * .. -> B -> .. with A -> .. -> B. Therefore A -> B is redundant.
- *
- * We need to make sure both the start and the end of A -> .. -> B is not
- * weaker than A -> B. For the start part, please see the comment in
- * check_redundant(). For the end part, we need:
- *
- * Either
- *
- *     a) A -> B is -(*R)-> (everything is not weaker than that)
- *
- * or
- *
- *     b) A -> .. -> B is -(*N)-> (nothing is stronger than this)
- *
- */
-static inline bool hlock_equal(struct lock_list *entry, void *data)
-{
-	struct held_lock *hlock = (struct held_lock *)data;
-
-	return hlock_class(hlock) == entry->class && /* Found A -> .. -> B */
-	       (hlock->read == 2 ||  /* A -> B is -(*R)-> */
-		!entry->only_xr); /* A -> .. -> B is -(*N)-> */
-}
-
 /*
  * We are about to add B -> A into the dependency graph, and in __bfs() a
  * strong dependency path A -> .. -> B is found: hlock_class equals
@@ -2915,6 +2880,41 @@ static inline bool usage_skip(struct lock_list *entry, void *mask)
 #endif /* CONFIG_TRACE_IRQFLAGS */
 
 #ifdef CONFIG_LOCKDEP_SMALL
+/*
+ * We are about to add A -> B into the dependency graph, and in __bfs() a
+ * strong dependency path A -> .. -> B is found: hlock_class equals
+ * entry->class.
+ *
+ * If A -> .. -> B can replace A -> B in any __bfs() search (means the former
+ * is _stronger_ than or equal to the latter), we consider A -> B as redundant.
+ * For example if A -> .. -> B is -(EN)-> (i.e. A -(E*)-> .. -(*N)-> B), and A
+ * -> B is -(ER)-> or -(EN)->, then we don't need to add A -> B into the
+ * dependency graph, as any strong path ..-> A -> B ->.. we can get with
+ * having dependency A -> B, we could already get a equivalent path ..-> A ->
+ * .. -> B -> .. with A -> .. -> B. Therefore A -> B is redundant.
+ *
+ * We need to make sure both the start and the end of A -> .. -> B is not
+ * weaker than A -> B. For the start part, please see the comment in
+ * check_redundant(). For the end part, we need:
+ *
+ * Either
+ *
+ *     a) A -> B is -(*R)-> (everything is not weaker than that)
+ *
+ * or
+ *
+ *     b) A -> .. -> B is -(*N)-> (nothing is stronger than this)
+ *
+ */
+static inline bool hlock_equal(struct lock_list *entry, void *data)
+{
+	struct held_lock *hlock = (struct held_lock *)data;
+
+	return hlock_class(hlock) == entry->class && /* Found A -> .. -> B */
+	       (hlock->read == 2 ||  /* A -> B is -(*R)-> */
+		!entry->only_xr); /* A -> .. -> B is -(*N)-> */
+}
+
 /*
  * Check that the dependency graph starting at <src> can lead to
  * <target> or not. If it can, <src> -> <target> dependency is already
-- 
2.47.2

Re: [PATCH v1 1/1] lockdep: Move hlock_equal() to the respective ifdeffery

[Andy Shevchenko]

On Tue, Apr 15, 2025 at 11:58:56AM +0300, Andy Shevchenko wrote:
> When hlock_equal() is unused, it prevents kernel builds with clang,
> `make W=1` and CONFIG_WERROR=y, CONFIG_LOCKDEP=y and
> CONFIG_LOCKDEP_SMALL=n:
> 
>   lockdep.c:2005:20: error: unused function 'hlock_equal' [-Werror,-Wunused-function]
> 
> Fix this by moving the function to the respective existing ifdeffery
> for its the only user.
> 
> See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> inline functions for W=1 build").

Any news here, please? The problem still exists in v6.15-rc4.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] lockdep: Move hlock_equal() to the respective ifdeffery

[Boqun Feng]

Hi Andy,

On Fri, May 02, 2025 at 05:00:20PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 15, 2025 at 11:58:56AM +0300, Andy Shevchenko wrote:
> > When hlock_equal() is unused, it prevents kernel builds with clang,
> > `make W=1` and CONFIG_WERROR=y, CONFIG_LOCKDEP=y and
> > CONFIG_LOCKDEP_SMALL=n:
> > 
> >   lockdep.c:2005:20: error: unused function 'hlock_equal' [-Werror,-Wunused-function]
> > 
> > Fix this by moving the function to the respective existing ifdeffery
> > for its the only user.
> > 
> > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > inline functions for W=1 build").
> 
> Any news here, please? The problem still exists in v6.15-rc4.
> 

This is in my radar, so it will be in a PR to tip soon. I didn't reply
earlier because I meant to find a whole cleanup for ifdefferies in
lockdep:

	https://lore.kernel.org/lkml/Z46BJ8FhWCIXbM7p@boqun-archlinux/

to avoid whack-a-mole fixes. I never found time so I have to postpone
that. Thanks!

Regards,
Boqun

> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v1 1/1] lockdep: Move hlock_equal() to the respective ifdeffery

[Andy Shevchenko]

On Fri, May 02, 2025 at 07:44:53AM -0700, Boqun Feng wrote:
> On Fri, May 02, 2025 at 05:00:20PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 15, 2025 at 11:58:56AM +0300, Andy Shevchenko wrote:
> > > When hlock_equal() is unused, it prevents kernel builds with clang,
> > > `make W=1` and CONFIG_WERROR=y, CONFIG_LOCKDEP=y and
> > > CONFIG_LOCKDEP_SMALL=n:
> > > 
> > >   lockdep.c:2005:20: error: unused function 'hlock_equal' [-Werror,-Wunused-function]
> > > 
> > > Fix this by moving the function to the respective existing ifdeffery
> > > for its the only user.
> > > 
> > > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > > inline functions for W=1 build").
> > 
> > Any news here, please? The problem still exists in v6.15-rc4.
> 
> This is in my radar, so it will be in a PR to tip soon. I didn't reply
> earlier because I meant to find a whole cleanup for ifdefferies in
> lockdep:
> 
> 	https://lore.kernel.org/lkml/Z46BJ8FhWCIXbM7p@boqun-archlinux/

This is good news! Nobody likes ifdeffery, it makes code harder to read.

> to avoid whack-a-mole fixes. I never found time so I have to postpone
> that. Thanks!

-- 
With Best Regards,
Andy Shevchenko