[PATCH v1 1/1] locking/rwsem: Mark inline helpers with __maybe_unused

[PATCH v1 1/1] locking/rwsem: Mark inline helpers with __maybe_unused

[Andy Shevchenko]

When one or more inline heplers are unused, it prevents kernel builds
with clang, `make W=1` and CONFIG_WERROR=y:

kernel/locking/rwsem.c:187:20: error: unused function 'is_rwsem_reader_owned' [-Werror,-Wunused-function]
  187 | static inline bool is_rwsem_reader_owned(struct rw_semaphore *sem)
      |                    ^~~~~~~~~~~~~~~~~~~~~
kernel/locking/rwsem.c:271:35: error: unused function 'rwsem_owner' [-Werror,-Wunused-function]
  271 | static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
      |                                   ^~~~~~~~~~~

Fix this by marking inline helpers 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/rwsem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index 33cac79e3994..c098567e2c68 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -184,7 +184,7 @@ static inline void rwsem_set_reader_owned(struct rw_semaphore *sem)
 /*
  * Return true if the rwsem is owned by a reader.
  */
-static inline bool is_rwsem_reader_owned(struct rw_semaphore *sem)
+static inline __maybe_unused bool is_rwsem_reader_owned(struct rw_semaphore *sem)
 {
 #ifdef CONFIG_DEBUG_RWSEMS
 	/*
@@ -268,7 +268,7 @@ static inline bool rwsem_write_trylock(struct rw_semaphore *sem)
 /*
  * Return just the real task structure pointer of the owner
  */
-static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
+static inline __maybe_unused struct task_struct *rwsem_owner(struct rw_semaphore *sem)
 {
 	return (struct task_struct *)
 		(atomic_long_read(&sem->owner) & ~RWSEM_OWNER_FLAGS_MASK);
@@ -1508,7 +1508,7 @@ static inline void __rwsem_set_reader_owned(struct rw_semaphore *sem,
 {
 }
 
-static inline bool is_rwsem_reader_owned(struct rw_semaphore *sem)
+static inline __maybe_unused bool is_rwsem_reader_owned(struct rw_semaphore *sem)
 {
 	int count = atomic_read(&sem->rwbase.readers);
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/1] locking/rwsem: Mark inline helpers with __maybe_unused

[Peter Zijlstra]

On Mon, Sep 09, 2024 at 02:58:39PM +0300, Andy Shevchenko wrote:
> When one or more inline heplers are unused, it prevents kernel builds
> with clang, `make W=1` and CONFIG_WERROR=y:
> 
> kernel/locking/rwsem.c:187:20: error: unused function 'is_rwsem_reader_owned' [-Werror,-Wunused-function]
>   187 | static inline bool is_rwsem_reader_owned(struct rw_semaphore *sem)
>       |                    ^~~~~~~~~~~~~~~~~~~~~
> kernel/locking/rwsem.c:271:35: error: unused function 'rwsem_owner' [-Werror,-Wunused-function]
>   271 | static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
>       |                                   ^~~~~~~~~~~
> 
> Fix this by marking inline helpers with __maybe_unused.
> 
> See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> inline functions for W=1 build").

:-(

And now you're back to the exact situation that people tried to avoid.
The moment one of these functions goes unused it will no longer scream
about it.

I'm for reverting the above commit, that gets all static inline on the
same footing, it should not matter if code is from a header file or not.

Re: [PATCH v1 1/1] locking/rwsem: Mark inline helpers with __maybe_unused

[Andy Shevchenko]

On Mon, Sep 09, 2024 at 03:29:41PM +0200, Peter Zijlstra wrote:
> On Mon, Sep 09, 2024 at 02:58:39PM +0300, Andy Shevchenko wrote:
> > When one or more inline heplers are unused, it prevents kernel builds
> > with clang, `make W=1` and CONFIG_WERROR=y:
> > 
> > kernel/locking/rwsem.c:187:20: error: unused function 'is_rwsem_reader_owned' [-Werror,-Wunused-function]
> >   187 | static inline bool is_rwsem_reader_owned(struct rw_semaphore *sem)
> >       |                    ^~~~~~~~~~~~~~~~~~~~~
> > kernel/locking/rwsem.c:271:35: error: unused function 'rwsem_owner' [-Werror,-Wunused-function]
> >   271 | static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
> >       |                                   ^~~~~~~~~~~
> > 
> > Fix this by marking inline helpers with __maybe_unused.
> > 
> > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > inline functions for W=1 build").
> 
> :-(
> 
> And now you're back to the exact situation that people tried to avoid.
> The moment one of these functions goes unused it will no longer scream
> about it.

Yeah... The problem is that I don't know well this code. This is, of course,
just a quickfix, the proper one should probably locate the function under
the proper guards (here all of them are used only for debugging AFAICS).
But I'm not sure. Hence consider this as
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> I'm for reverting the above commit, that gets all static inline on the
> same footing, it should not matter if code is from a header file or not.

Is it the case? Because to me it seems to complain only on C files.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] locking/rwsem: Mark inline helpers with __maybe_unused

[Peter Zijlstra]

On Mon, Sep 09, 2024 at 04:57:34PM +0300, Andy Shevchenko wrote:
> On Mon, Sep 09, 2024 at 03:29:41PM +0200, Peter Zijlstra wrote:
> > On Mon, Sep 09, 2024 at 02:58:39PM +0300, Andy Shevchenko wrote:
> > > When one or more inline heplers are unused, it prevents kernel builds
> > > with clang, `make W=1` and CONFIG_WERROR=y:
> > > 
> > > kernel/locking/rwsem.c:187:20: error: unused function 'is_rwsem_reader_owned' [-Werror,-Wunused-function]
> > >   187 | static inline bool is_rwsem_reader_owned(struct rw_semaphore *sem)
> > >       |                    ^~~~~~~~~~~~~~~~~~~~~
> > > kernel/locking/rwsem.c:271:35: error: unused function 'rwsem_owner' [-Werror,-Wunused-function]
> > >   271 | static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
> > >       |                                   ^~~~~~~~~~~
> > > 
> > > Fix this by marking inline helpers with __maybe_unused.
> > > 
> > > See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > > inline functions for W=1 build").
> > 
> > :-(
> > 
> > And now you're back to the exact situation that people tried to avoid.
> > The moment one of these functions goes unused it will no longer scream
> > about it.
> 
> Yeah... The problem is that I don't know well this code. This is, of course,
> just a quickfix, the proper one should probably locate the function under
> the proper guards (here all of them are used only for debugging AFAICS).
> But I'm not sure. Hence consider this as
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Yeah, so I'm *waaaay* too overloaded to care about this make-work
nonsense, the people that made this happen can either go and revert
their ill considered patch or spend their own time trying to come up
with a sane solution.

> > I'm for reverting the above commit, that gets all static inline on the
> > same footing, it should not matter if code is from a header file or not.
> 
> Is it the case? Because to me it seems to complain only on C files.

It's a clang special, and it used to be suppressed, but for some
mysterious reason people seem to want to re enable this behaviour. But
like said, if they want this, they can spend the time fixing their own
fallout.