Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Wed, Nov 01, 2023 at 10:25:25AM +0100, Arnd Bergmann wrote:
> On Tue, Oct 31, 2023, at 18:14, Bjorn Helgaas wrote:
> > On Tue, Oct 31, 2023 at 09:59:29AM -0700, Nick Desaulniers wrote:
> >> On Tue, Oct 31, 2023 at 7:56 AM Bjorn Helgaas wrote:
>
> >> > arch/powerpc/xmon/xmon.c: release_output_lock();
> >> >
> >> > That said, the unused functions do look legit:
> >> >
> >> > grackle_set_stg() is a static function and the only call is under
> >> > "#if 0".
> >>
> >> Time to remove it then? Or is it a bug that it's not called?
> >> Otherwise the definition should be behind the same preprocessor guards
> >> as the caller. Same for the below.
>
> It would be nice to get rid of all warnings about unused
> "static inline" functions and "static const" variables in .c
> files. I think both these warnings got added at the W=1 level
> for compilers that support them at some point, but are ignored
> for normal builds without W=1 because they are too noisy.
>
> Obviously, all compilers ignore unused inline functions and
> const variables in header files regardless of the warning level.
Right, this was an intentional change done by Masahiro to try and take
advantage of the fact that clang warns about unused static inline
functions in .c files (whereas GCC has no warning in .c or .h files) to
clean up dead code. See commit 6863f5643dd7 ("kbuild: allow Clang to
find unused static inline functions for W=1 build") for more
information.
Cheers,
Nathan
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Tue, Oct 31, 2023, at 18:14, Bjorn Helgaas wrote: > On Tue, Oct 31, 2023 at 09:59:29AM -0700, Nick Desaulniers wrote: >> On Tue, Oct 31, 2023 at 7:56 AM Bjorn Helgaas wrote: >> > arch/powerpc/xmon/xmon.c: release_output_lock(); >> > >> > That said, the unused functions do look legit: >> > >> > grackle_set_stg() is a static function and the only call is under >> > "#if 0". >> >> Time to remove it then? Or is it a bug that it's not called? >> Otherwise the definition should be behind the same preprocessor guards >> as the caller. Same for the below. It would be nice to get rid of all warnings about unused "static inline" functions and "static const" variables in .c files. I think both these warnings got added at the W=1 level for compilers that support them at some point, but are ignored for normal builds without W=1 because they are too noisy. Obviously, all compilers ignore unused inline functions and const variables in header files regardless of the warning level. > I don't really care whether we keep the warning or not. > > My real complaint is that the 0-day report fingered > pci/controller/xilinx-xdma, which is completely unrelated, which is a > waste of time. I tried to figure this out but couldn't find the real reason either, clearly there is something wrong with the reporting here, my best guess would be that there is a spurious build failure elsewhere that leads to this file sometimes getting flagged as a false-positive. Arnd
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Tue, Oct 31, 2023 at 09:59:29AM -0700, Nick Desaulniers wrote:
> On Tue, Oct 31, 2023 at 7:56 AM Bjorn Helgaas wrote:
> > On Sat, Oct 28, 2023 at 08:22:54PM +0800, kernel test robot wrote:
> > > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
> > > controller/xilinx-xdma
> > > branch HEAD: 8d786149d78c7784144c7179e25134b6530b714b PCI: xilinx-xdma:
> > > Add Xilinx XDMA Root Port driver
> > >
> > > Error/Warning ids grouped by kconfigs:
> > >
> > > clang_recent_errors
> > > `-- powerpc-pmac32_defconfig
> > > |--
> > > arch-powerpc-sysdev-grackle.c:error:unused-function-grackle_set_stg-Werror-Wunused-function
> > > |--
> > > arch-powerpc-xmon-xmon.c:error:unused-function-get_output_lock-Werror-Wunused-function
> > > `--
> > > arch-powerpc-xmon-xmon.c:error:unused-function-release_output_lock-Werror-Wunused-function
> >
> > This report is close to useless. It doesn't show the complete error
> > message, it doesn't show how to reproduce the issue, and the pci -next
> > branch (including controller/xilinx-xdma) doesn't reference any of
> > these functions:
> >
> > $ git grep -E "grackle_set_stg|get_output_lock|release_output_lock" | cat
> > arch/powerpc/sysdev/grackle.c:static inline void grackle_set_stg(struct
> > pci_controller* bp, int enable)
> > arch/powerpc/sysdev/grackle.c:grackle_set_stg(hose, 1);
> > arch/powerpc/xmon/xmon.c:static void get_output_lock(void)
> > arch/powerpc/xmon/xmon.c:static void release_output_lock(void)
> > arch/powerpc/xmon/xmon.c:static inline void get_output_lock(void) {}
> > arch/powerpc/xmon/xmon.c:static inline void release_output_lock(void) {}
> > arch/powerpc/xmon/xmon.c: get_output_lock();
> > arch/powerpc/xmon/xmon.c: release_output_lock();
> > arch/powerpc/xmon/xmon.c: get_output_lock();
> > arch/powerpc/xmon/xmon.c: release_output_lock();
> > arch/powerpc/xmon/xmon.c: get_output_lock();
> > arch/powerpc/xmon/xmon.c: release_output_lock();
> > arch/powerpc/xmon/xmon.c: get_output_lock();
> > arch/powerpc/xmon/xmon.c: release_output_lock();
> >
> > That said, the unused functions do look legit:
> >
> > grackle_set_stg() is a static function and the only call is under
> > "#if 0".
>
> Time to remove it then? Or is it a bug that it's not called?
> Otherwise the definition should be behind the same preprocessor guards
> as the caller. Same for the below.
I don't really care whether we keep the warning or not.
My real complaint is that the 0-day report fingered
pci/controller/xilinx-xdma, which is completely unrelated, which is a
waste of time.
> > Same with get_output_lock() and release_output_lock(): they're static
> > and always defined in xmon.c, but only called if either CONFIG_SMP or
> > CONFIG_DEBUG_FS.
> >
> > But they're certainly not related to controller/xilinx-xdma, so I'm
> > going to ignore them.
> >
> > Bjorn
> >
> > P.S. Nathan & Nick, I cc'd you because of this earlier report that
> > also mentioned grackle_set_stg():
> > https://lore.kernel.org/lkml/202308121120.u2d3ypvt-...@intel.com/
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Tue, Oct 31, 2023 at 7:56 AM Bjorn Helgaas wrote:
>
> [+cc powerpc, clang folks]
>
> On Sat, Oct 28, 2023 at 08:22:54PM +0800, kernel test robot wrote:
> > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
> > controller/xilinx-xdma
> > branch HEAD: 8d786149d78c7784144c7179e25134b6530b714b PCI: xilinx-xdma:
> > Add Xilinx XDMA Root Port driver
> >
> > Error/Warning ids grouped by kconfigs:
> >
> > clang_recent_errors
> > `-- powerpc-pmac32_defconfig
> > |--
> > arch-powerpc-sysdev-grackle.c:error:unused-function-grackle_set_stg-Werror-Wunused-function
> > |--
> > arch-powerpc-xmon-xmon.c:error:unused-function-get_output_lock-Werror-Wunused-function
> > `--
> > arch-powerpc-xmon-xmon.c:error:unused-function-release_output_lock-Werror-Wunused-function
>
> This report is close to useless. It doesn't show the complete error
> message, it doesn't show how to reproduce the issue, and the pci -next
> branch (including controller/xilinx-xdma) doesn't reference any of
> these functions:
>
> $ git grep -E "grackle_set_stg|get_output_lock|release_output_lock" | cat
> arch/powerpc/sysdev/grackle.c:static inline void grackle_set_stg(struct
> pci_controller* bp, int enable)
> arch/powerpc/sysdev/grackle.c:grackle_set_stg(hose, 1);
> arch/powerpc/xmon/xmon.c:static void get_output_lock(void)
> arch/powerpc/xmon/xmon.c:static void release_output_lock(void)
> arch/powerpc/xmon/xmon.c:static inline void get_output_lock(void) {}
> arch/powerpc/xmon/xmon.c:static inline void release_output_lock(void) {}
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
>
> That said, the unused functions do look legit:
>
> grackle_set_stg() is a static function and the only call is under
> "#if 0".
Time to remove it then? Or is it a bug that it's not called?
Otherwise the definition should be behind the same preprocessor guards
as the caller. Same for the below.
>
> Same with get_output_lock() and release_output_lock(): they're static
> and always defined in xmon.c, but only called if either CONFIG_SMP or
> CONFIG_DEBUG_FS.
>
> But they're certainly not related to controller/xilinx-xdma, so I'm
> going to ignore them.
>
> Bjorn
>
> P.S. Nathan & Nick, I cc'd you because of this earlier report that
> also mentioned grackle_set_stg():
> https://lore.kernel.org/lkml/202308121120.u2d3ypvt-...@intel.com/
--
Thanks,
~Nick Desaulniers
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Tue, Oct 31, 2023 at 09:56:00AM -0500, Bjorn Helgaas wrote: > That said, the unused functions do look legit: > > grackle_set_stg() is a static function and the only call is under > "#if 0". It is a static inline. It is *normal* that that is uncalled. It is very similar to a macro that has no invocation. The warning is overeager. Segher
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
[+cc powerpc, clang folks]
On Sat, Oct 28, 2023 at 08:22:54PM +0800, kernel test robot wrote:
> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
> controller/xilinx-xdma
> branch HEAD: 8d786149d78c7784144c7179e25134b6530b714b PCI: xilinx-xdma: Add
> Xilinx XDMA Root Port driver
>
> Error/Warning ids grouped by kconfigs:
>
> clang_recent_errors
> `-- powerpc-pmac32_defconfig
> |--
> arch-powerpc-sysdev-grackle.c:error:unused-function-grackle_set_stg-Werror-Wunused-function
> |--
> arch-powerpc-xmon-xmon.c:error:unused-function-get_output_lock-Werror-Wunused-function
> `--
> arch-powerpc-xmon-xmon.c:error:unused-function-release_output_lock-Werror-Wunused-function
This report is close to useless. It doesn't show the complete error
message, it doesn't show how to reproduce the issue, and the pci -next
branch (including controller/xilinx-xdma) doesn't reference any of
these functions:
$ git grep -E "grackle_set_stg|get_output_lock|release_output_lock" | cat
arch/powerpc/sysdev/grackle.c:static inline void grackle_set_stg(struct
pci_controller* bp, int enable)
arch/powerpc/sysdev/grackle.c:grackle_set_stg(hose, 1);
arch/powerpc/xmon/xmon.c:static void get_output_lock(void)
arch/powerpc/xmon/xmon.c:static void release_output_lock(void)
arch/powerpc/xmon/xmon.c:static inline void get_output_lock(void) {}
arch/powerpc/xmon/xmon.c:static inline void release_output_lock(void) {}
arch/powerpc/xmon/xmon.c: get_output_lock();
arch/powerpc/xmon/xmon.c: release_output_lock();
arch/powerpc/xmon/xmon.c: get_output_lock();
arch/powerpc/xmon/xmon.c: release_output_lock();
arch/powerpc/xmon/xmon.c: get_output_lock();
arch/powerpc/xmon/xmon.c: release_output_lock();
arch/powerpc/xmon/xmon.c: get_output_lock();
arch/powerpc/xmon/xmon.c: release_output_lock();
That said, the unused functions do look legit:
grackle_set_stg() is a static function and the only call is under
"#if 0".
Same with get_output_lock() and release_output_lock(): they're static
and always defined in xmon.c, but only called if either CONFIG_SMP or
CONFIG_DEBUG_FS.
But they're certainly not related to controller/xilinx-xdma, so I'm
going to ignore them.
Bjorn
P.S. Nathan & Nick, I cc'd you because of this earlier report that
also mentioned grackle_set_stg():
https://lore.kernel.org/lkml/202308121120.u2d3ypvt-...@intel.com/
