[PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Andy Shevchenko]

With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine.
This is due to some unused functions. Hence these quick fixes.

Andy Shevchenko (2):
  drm/i915/fence: Mark debug_fence_init_onstack() with __maybe_unused
  drm/i915/fence: Mark debug_fence_free() with __maybe_unused

 drivers/gpu/drm/i915/i915_sw_fence.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 1/2] drm/i915/fence: Mark debug_fence_init_onstack() with __maybe_unused

[Andy Shevchenko]

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

.../i915_sw_fence.c:97:20: error: unused function 'debug_fence_init_onstack' [-Werror,-Wunused-function]
   97 | static inline void debug_fence_init_onstack(struct i915_sw_fence *fence)
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~

Fix this by marking debug_fence_init_onstack() with __maybe_unused.

Fixes: 214707fc2ce0 ("drm/i915/selftests: Wrap a timer into a i915_sw_fence")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_sw_fence.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index 8a9aad523eec..d4020ff3549a 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -51,7 +51,7 @@ static inline void debug_fence_init(struct i915_sw_fence *fence)
 	debug_object_init(fence, &i915_sw_fence_debug_descr);
 }
 
-static inline void debug_fence_init_onstack(struct i915_sw_fence *fence)
+static inline __maybe_unused void debug_fence_init_onstack(struct i915_sw_fence *fence)
 {
 	debug_object_init_on_stack(fence, &i915_sw_fence_debug_descr);
 }
@@ -94,7 +94,7 @@ static inline void debug_fence_init(struct i915_sw_fence *fence)
 {
 }
 
-static inline void debug_fence_init_onstack(struct i915_sw_fence *fence)
+static inline __maybe_unused void debug_fence_init_onstack(struct i915_sw_fence *fence)
 {
 }
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 2/2] drm/i915/fence: Mark debug_fence_free() with __maybe_unused

[Andy Shevchenko]

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

.../i915_sw_fence.c:118:20: error: unused function 'debug_fence_free' [-Werror,-Wunused-function]
  118 | static inline void debug_fence_free(struct i915_sw_fence *fence)
      |                    ^~~~~~~~~~~~~~~~

Fix this by marking debug_fence_free() with __maybe_unused.

Fixes: fc1584059d6c ("drm/i915: Integrate i915_sw_fence with debugobjects")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_sw_fence.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index d4020ff3549a..1d4cc91c0e40 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -77,7 +77,7 @@ static inline void debug_fence_destroy(struct i915_sw_fence *fence)
 	debug_object_destroy(fence, &i915_sw_fence_debug_descr);
 }
 
-static inline void debug_fence_free(struct i915_sw_fence *fence)
+static inline __maybe_unused void debug_fence_free(struct i915_sw_fence *fence)
 {
 	debug_object_free(fence, &i915_sw_fence_debug_descr);
 	smp_wmb(); /* flush the change in state before reallocation */
@@ -115,7 +115,7 @@ static inline void debug_fence_destroy(struct i915_sw_fence *fence)
 {
 }
 
-static inline void debug_fence_free(struct i915_sw_fence *fence)
+static inline __maybe_unused void debug_fence_free(struct i915_sw_fence *fence)
 {
 }
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Jani Nikula]

On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine.
> This is due to some unused functions. Hence these quick fixes.

Since when have we been getting the warnings for static inlines?

BR,
Jani.


>
> Andy Shevchenko (2):
>   drm/i915/fence: Mark debug_fence_init_onstack() with __maybe_unused
>   drm/i915/fence: Mark debug_fence_free() with __maybe_unused
>
>  drivers/gpu/drm/i915/i915_sw_fence.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

-- 
Jani Nikula, Intel

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Andy Shevchenko]

On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote:
> On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine.
> > This is due to some unused functions. Hence these quick fixes.
> 
> Since when have we been getting the warnings for static inlines?

Do you want to see any additional information of my building environment?

Debian clang version 18.1.8 (9)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Andy Shevchenko]

On Thu, Aug 29, 2024 at 07:53:25PM +0300, Andy Shevchenko wrote:
> On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote:
> > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine.
> > > This is due to some unused functions. Hence these quick fixes.
> > 
> > Since when have we been getting the warnings for static inlines?
> 
> Do you want to see any additional information of my building environment?
> 
> Debian clang version 18.1.8 (9)

FWIW, clang-16 also behaves in the same way, Cc'ed to CLANG maintainers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Nathan Chancellor]

On Thu, Aug 29, 2024 at 09:10:41PM +0300, Andy Shevchenko wrote:
> On Thu, Aug 29, 2024 at 07:53:25PM +0300, Andy Shevchenko wrote:
> > On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote:
> > > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine.
> > > > This is due to some unused functions. Hence these quick fixes.
> > > 
> > > Since when have we been getting the warnings for static inlines?

Since commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
inline functions for W=1 build"). clang warns about unused static inline
functions in .c files, unlike GCC (they both do not warn for functions
coming from .h files). This difference is worked around for the normal
build by adding '__maybe_unused' to the definition of 'inline' but
Masahiro wanted to disable it for W=1 to allow this difference to find
unused/dead code. There have not been too many complaints as far as I am
aware but I can see how it is surprising.

Cheers,
Nathan

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Jani Nikula]

On Thu, 29 Aug 2024, Nathan Chancellor <nathan@kernel.org> wrote:
> On Thu, Aug 29, 2024 at 09:10:41PM +0300, Andy Shevchenko wrote:
>> On Thu, Aug 29, 2024 at 07:53:25PM +0300, Andy Shevchenko wrote:
>> > On Thu, Aug 29, 2024 at 07:38:08PM +0300, Jani Nikula wrote:
>> > > On Thu, 29 Aug 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>> > > > With CONFIG_WERROR=y and `make W=1` build fails on my x86_64 machine.
>> > > > This is due to some unused functions. Hence these quick fixes.
>> > > 
>> > > Since when have we been getting the warnings for static inlines?
>
> Since commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> inline functions for W=1 build"). clang warns about unused static inline
> functions in .c files, unlike GCC (they both do not warn for functions
> coming from .h files). This difference is worked around for the normal
> build by adding '__maybe_unused' to the definition of 'inline' but
> Masahiro wanted to disable it for W=1 to allow this difference to find
> unused/dead code. There have not been too many complaints as far as I am
> aware but I can see how it is surprising.

Heh, I was just going to reply citing the same commit.

I occasionally build with clang myself, and we do enable most W=1 by
default in the drm subsystem, so I was wondering why I hadn't hit
this. The crucial difference is that we lack -DKBUILD_EXTRA_WARN1 which
W=1 adds.

I see there's no subdir-cppflags-y, but I don't see any harm in us
adding -Wundef and -DKBUILD_EXTRA_WARN1 to subdir-ccflags-y. After we
fix the fallout, of course. Do you?

I don't much like the __maybe_unused stuff, but I guess it's fine as a
stopgap measure, and then we can grep for that when running out of
things to do. :p

The TL;DR is,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

on the series.

BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Nathan Chancellor]

On Thu, Aug 29, 2024 at 09:37:34PM +0300, Jani Nikula wrote:
> On Thu, 29 Aug 2024, Nathan Chancellor <nathan@kernel.org> wrote:
> > Since commit 6863f5643dd7 ("kbuild: allow Clang to find unused static
> > inline functions for W=1 build"). clang warns about unused static inline
> > functions in .c files, unlike GCC (they both do not warn for functions
> > coming from .h files). This difference is worked around for the normal
> > build by adding '__maybe_unused' to the definition of 'inline' but
> > Masahiro wanted to disable it for W=1 to allow this difference to find
> > unused/dead code. There have not been too many complaints as far as I am
> > aware but I can see how it is surprising.
> 
> Heh, I was just going to reply citing the same commit.
> 
> I occasionally build with clang myself, and we do enable most W=1 by
> default in the drm subsystem, so I was wondering why I hadn't hit
> this. The crucial difference is that we lack -DKBUILD_EXTRA_WARN1 which
> W=1 adds.
> 
> I see there's no subdir-cppflags-y, but I don't see any harm in us
> adding -Wundef and -DKBUILD_EXTRA_WARN1 to subdir-ccflags-y. After we
> fix the fallout, of course. Do you?

No, that seems entirely reasonable when your goal is to enable W=1 for
your subsystem.

> I don't much like the __maybe_unused stuff, but I guess it's fine as a
> stopgap measure, and then we can grep for that when running out of
> things to do. :p

Perhaps worth a TODO or something? Maybe a kernel newbie could work on
that at some point if it is not high enough priority.

Cheers,
Nathan

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Jani Nikula]

On Thu, 29 Aug 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> The TL;DR is,
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> on the series.

Both pushed to drm-intel-next, thanks for the patches and discussion.

I amended the commit message about clang, config options and commit
6863f5643dd7 ("kbuild: allow Clang to find unused static inline
functions for W=1 build") while pushing.

BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH v1 0/2] drm/i915/fence: A couple of build fixes

[Andy Shevchenko]

On Mon, Sep 02, 2024 at 11:27:57AM +0300, Jani Nikula wrote:
> On Thu, 29 Aug 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > The TL;DR is,
> >
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >
> > on the series.
> 
> Both pushed to drm-intel-next, thanks for the patches and discussion.
> 
> I amended the commit message about clang, config options and commit
> 6863f5643dd7 ("kbuild: allow Clang to find unused static inline
> functions for W=1 build") while pushing.

It all makes sense. Thank you!

-- 
With Best Regards,
Andy Shevchenko