[PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Andy Shevchenko]

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

kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]

Fix this by marking it with __maybe_unused.

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/locking/lockdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index bb65abfcfa71..29acd238dad7 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -432,7 +432,7 @@ static inline u16 hlock_id(struct held_lock *hlock)
 	return (hlock->class_idx | (hlock->read << MAX_LOCKDEP_KEYS_BITS));
 }
 
-static inline unsigned int chain_hlock_class_idx(u16 hlock_id)
+static inline __maybe_unused unsigned int chain_hlock_class_idx(u16 hlock_id)
 {
 	return hlock_id & (MAX_LOCKDEP_KEYS - 1);
 }
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Boqun Feng]

Hi Andy,

On Mon, Dec 09, 2024 at 07:08:10PM +0200, Andy Shevchenko wrote:
> When chain_hlock_class_idx() is unused, it prevents kernel builds with clang,
> `make W=1` and CONFIG_WERROR=y:
> 
> kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]
> 
> Fix this by marking it with __maybe_unused.
> 
> See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> inline functions for W=1 build").
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This looks fine to me, so I'm going to queue it, but could you do me
favor if you could share the exact configs that would make
chain_hlock_class_idx() an unused fuction in kernel/locking/lockdep.c ?

I might add these information in commit log.

Thanks!

Regards,
Boqun

> ---
>  kernel/locking/lockdep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index bb65abfcfa71..29acd238dad7 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -432,7 +432,7 @@ static inline u16 hlock_id(struct held_lock *hlock)
>  	return (hlock->class_idx | (hlock->read << MAX_LOCKDEP_KEYS_BITS));
>  }
>  
> -static inline unsigned int chain_hlock_class_idx(u16 hlock_id)
> +static inline __maybe_unused unsigned int chain_hlock_class_idx(u16 hlock_id)
>  {
>  	return hlock_id & (MAX_LOCKDEP_KEYS - 1);
>  }
> -- 
> 2.43.0.rc1.1336.g36b5255a03ac
> 

Re: [PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Andy Shevchenko]

On Sun, Dec 15, 2024 at 11:55:08AM -0800, Boqun Feng wrote:
> On Mon, Dec 09, 2024 at 07:08:10PM +0200, Andy Shevchenko wrote:
> > When chain_hlock_class_idx() is unused, it prevents kernel builds with clang,
> > `make W=1` and CONFIG_WERROR=y:
> > 
> > kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]
> > 
> > Fix this by marking it with __maybe_unused.
> > 
> > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > inline functions for W=1 build").
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This looks fine to me, so I'm going to queue it, but could you do me
> favor if you could share the exact configs that would make
> chain_hlock_class_idx() an unused fuction in kernel/locking/lockdep.c ?

I might harvest this when I come from vacations (some around mid-January).

> I might add these information in commit log.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Boqun Feng]

On Sun, Dec 15, 2024 at 11:21:36PM +0200, Andy Shevchenko wrote:
> On Sun, Dec 15, 2024 at 11:55:08AM -0800, Boqun Feng wrote:
> > On Mon, Dec 09, 2024 at 07:08:10PM +0200, Andy Shevchenko wrote:
> > > When chain_hlock_class_idx() is unused, it prevents kernel builds with clang,
> > > `make W=1` and CONFIG_WERROR=y:
> > > 
> > > kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]
> > > 
> > > Fix this by marking it with __maybe_unused.
> > > 
> > > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > > inline functions for W=1 build").
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > This looks fine to me, so I'm going to queue it, but could you do me
> > favor if you could share the exact configs that would make
> > chain_hlock_class_idx() an unused fuction in kernel/locking/lockdep.c ?
> 
> I might harvest this when I come from vacations (some around mid-January).
> 

After some investigation myself, it turns out that
chain_hlock_class_idx() is defined outside "#ifdef CONFIG_PROVING_LOCK",
but only used under CONFIG_PROVING_LOCK=y, hence the unused warning.

TBH, I feel we should really clean-up/consolidate those "#ifdef" to make
internal definition/usage more clear. But nothing blocks this patch, it
fixes a real issue, I will add the CONFIG_PROVING_LOCK part in the
commit log. Thanks!

Regards,
Boqun

> > I might add these information in commit log.
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Andy Shevchenko]

On Thu, Dec 19, 2024 at 01:57:30PM -0800, Boqun Feng wrote:
> On Sun, Dec 15, 2024 at 11:21:36PM +0200, Andy Shevchenko wrote:
> > On Sun, Dec 15, 2024 at 11:55:08AM -0800, Boqun Feng wrote:
> > > On Mon, Dec 09, 2024 at 07:08:10PM +0200, Andy Shevchenko wrote:
> > > > When chain_hlock_class_idx() is unused, it prevents kernel builds with clang,
> > > > `make W=1` and CONFIG_WERROR=y:
> > > > 
> > > > kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]
> > > > 
> > > > Fix this by marking it with __maybe_unused.
> > > > 
> > > > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > > > inline functions for W=1 build").
> > > > 
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > This looks fine to me, so I'm going to queue it, but could you do me
> > > favor if you could share the exact configs that would make
> > > chain_hlock_class_idx() an unused fuction in kernel/locking/lockdep.c ?
> > 
> > I might harvest this when I come from vacations (some around mid-January).
> 
> After some investigation myself, it turns out that
> chain_hlock_class_idx() is defined outside "#ifdef CONFIG_PROVING_LOCK",
> but only used under CONFIG_PROVING_LOCK=y, hence the unused warning.
> 
> TBH, I feel we should really clean-up/consolidate those "#ifdef" to make
> internal definition/usage more clear. But nothing blocks this patch, it
> fixes a real issue, I will add the CONFIG_PROVING_LOCK part in the
> commit log. Thanks!

I take this as no AR from me needed anymore, tell me if it's not the case.
Thanks!

> > > I might add these information in commit log.

-- 
With Best Regards,
Andy Shevchenko

[tip: locking/core] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[tip-bot2 for Andy Shevchenko]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     8148fa2e022bae29f21bb9a2c4cc796334fd372b
Gitweb:        https://git.kernel.org/tip/8148fa2e022bae29f21bb9a2c4cc796334fd372b
Author:        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
AuthorDate:    Mon, 09 Dec 2024 19:08:10 +02:00
Committer:     Boqun Feng <boqun.feng@gmail.com>
CommitterDate: Thu, 19 Dec 2024 13:57:53 -08:00

lockdep: Mark chain_hlock_class_idx() with __maybe_unused

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

kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]

Fix this by marking it with __maybe_unused.

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

[Boqun: add more config information of the error]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20241209170810.1485183-1-andriy.shevchenko@linux.intel.com
---
 kernel/locking/lockdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 2d8ec03..fe04a21 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -430,7 +430,7 @@ static inline u16 hlock_id(struct held_lock *hlock)
 	return (hlock->class_idx | (hlock->read << MAX_LOCKDEP_KEYS_BITS));
 }
 
-static inline unsigned int chain_hlock_class_idx(u16 hlock_id)
+static inline __maybe_unused unsigned int chain_hlock_class_idx(u16 hlock_id)
 {
 	return hlock_id & (MAX_LOCKDEP_KEYS - 1);
 }

Re: [PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Ingo Molnar]

* Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sun, Dec 15, 2024 at 11:21:36PM +0200, Andy Shevchenko wrote:
> > On Sun, Dec 15, 2024 at 11:55:08AM -0800, Boqun Feng wrote:
> > > On Mon, Dec 09, 2024 at 07:08:10PM +0200, Andy Shevchenko wrote:
> > > > When chain_hlock_class_idx() is unused, it prevents kernel builds with clang,
> > > > `make W=1` and CONFIG_WERROR=y:
> > > > 
> > > > kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]
> > > > 
> > > > Fix this by marking it with __maybe_unused.
> > > > 
> > > > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > > > inline functions for W=1 build").
> > > > 
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > This looks fine to me, so I'm going to queue it, but could you do me
> > > favor if you could share the exact configs that would make
> > > chain_hlock_class_idx() an unused fuction in kernel/locking/lockdep.c ?
> > 
> > I might harvest this when I come from vacations (some around mid-January).
> > 
> 
> After some investigation myself, it turns out that
> chain_hlock_class_idx() is defined outside "#ifdef CONFIG_PROVING_LOCK",
> but only used under CONFIG_PROVING_LOCK=y, hence the unused warning.
> 
> TBH, I feel we should really clean-up/consolidate those "#ifdef" to make
> internal definition/usage more clear. But nothing blocks this patch, it
> fixes a real issue, I will add the CONFIG_PROVING_LOCK part in the
> commit log. Thanks!

So now that we have:

   343060092585 lockdep: Move lockdep_assert_locked() under #ifdef CONFIG_PROVE_LOCKING

Do we really need:

   8148fa2e022b lockdep: Mark chain_hlock_class_idx() with __maybe_unused

As it would hide the problem fixed by 343060092585?

__maybe_unused annotations are almost always canaries of something 
messy being hidden.

Thanks,

	Ingo

Re: [PATCH v1 1/1] lockdep: Mark chain_hlock_class_idx() with __maybe_unused

[Boqun Feng]

On Mon, Jan 20, 2025 at 09:11:09AM +0100, Ingo Molnar wrote:
> 
> * Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Sun, Dec 15, 2024 at 11:21:36PM +0200, Andy Shevchenko wrote:
> > > On Sun, Dec 15, 2024 at 11:55:08AM -0800, Boqun Feng wrote:
> > > > On Mon, Dec 09, 2024 at 07:08:10PM +0200, Andy Shevchenko wrote:
> > > > > When chain_hlock_class_idx() is unused, it prevents kernel builds with clang,
> > > > > `make W=1` and CONFIG_WERROR=y:
> > > > > 
> > > > > kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function]
> > > > > 
> > > > > Fix this by marking it with __maybe_unused.
> > > > > 
> > > > > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > > > > inline functions for W=1 build").
> > > > > 
> > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 
> > > > This looks fine to me, so I'm going to queue it, but could you do me
> > > > favor if you could share the exact configs that would make
> > > > chain_hlock_class_idx() an unused fuction in kernel/locking/lockdep.c ?
> > > 
> > > I might harvest this when I come from vacations (some around mid-January).
> > > 
> > 
> > After some investigation myself, it turns out that
> > chain_hlock_class_idx() is defined outside "#ifdef CONFIG_PROVING_LOCK",
> > but only used under CONFIG_PROVING_LOCK=y, hence the unused warning.
> > 
> > TBH, I feel we should really clean-up/consolidate those "#ifdef" to make
> > internal definition/usage more clear. But nothing blocks this patch, it
> > fixes a real issue, I will add the CONFIG_PROVING_LOCK part in the
> > commit log. Thanks!
> 
> So now that we have:
> 
>    343060092585 lockdep: Move lockdep_assert_locked() under #ifdef CONFIG_PROVE_LOCKING
> 
> Do we really need:
> 
>    8148fa2e022b lockdep: Mark chain_hlock_class_idx() with __maybe_unused
> 
> As it would hide the problem fixed by 343060092585?
> 

These two commits fix different functions, so they are both needed.
Though, we could probably regroup the functions into one big #ifdef
block so that we won't need the __maybe_unused annotations.

I will take a look.

Regards,
Boqun

> __maybe_unused annotations are almost always canaries of something 
> messy being hidden.
> 
> Thanks,
> 
> 	Ingo