On Thu, Oct 19, 2023 at 04:17:26PM -0400, Hamza Mahfooz wrote: > On 10/19/23 11:56, Nathan Chancellor wrote: > > On Thu, Oct 19, 2023 at 02:53:01PM +0200, Arnd Bergmann wrote: > > > On Thu, Oct 19, 2023, at 12:04, Alexander Potapenko wrote: > > > > So the remaining option would be to just increase the frame size every > > > > time a new function surpasses the limit. > > > > > > That is clearly not an option, though we could try to > > > add Kconfig dependencies that avoid the known bad combinations, > > > such as annotating the AMD GPU driver as > > > > > > depends on (CC_IS_GCC || CLANG_VERSION >=180000) || !(KASAN || KCSAN) > > > > This would effectively disable the AMDGPU driver for allmodconfig, which > > is somewhat unfortunate as it is an easy testing target. > > > > Taking a step back, this is all being done because of a couple of > > warnings in the AMDGPU code. If fixing those in the source is too much > > effort (I did note [1] that GCC is at the current limit for that file > > even with Rodrigo's series applied [2]), couldn't we just take the > > existing workaround that this Makefile has for this file and its high > > stack usage and just extend it slightly for clang? > > I personally don't mind fixing these issues in the driver, but the fact > that they the creep back every time a new major version of Clang rolls > out (that has been true for the past couple of years at the very > least), makes it rather annoying to deal with. I am not sure I agree with that characterization of the situation. clang has been pretty consistent for the most part (which is certainly on us), as all versions that the kernel supports warns about this code. I believe it is more so the fact that there is a new copy of the dcn code added every year that has none of the fixes applied from earlier generations... It is not just me that has fixed issues like this, just run 'git log --grep=stack drivers/gpu/drm/amd/display'. It is not just clang that complains about the code when sanitizers are turned on, GCC does as well since Stephen reported them. Cheers, Nathan > > diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > index 66431525f2a0..fd49e3526c0d 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > @@ -58,7 +58,7 @@ endif > > endif > > ifneq ($(CONFIG_FRAME_WARN),0) > > -frame_warn_flag := -Wframe-larger-than=2048 > > +frame_warn_flag := -Wframe-larger-than=$(if $(CONFIG_CC_IS_CLANG),3072,2048) > > endif > > CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag) > > > > That would address the immediate concern of the warning breaking builds > > with CONFIG_WERROR=y while not raising the limit for other files in the > > kernel (just this one file in AMDGPU) and avoiding disabling the whole > > driver. The number could be lower, I think ~2500 bytes is the most usage > > I see with Rodrigo's series applied, so maybe 2800 would be a decent > > limit? Once there is a fix in the compiler, this expression could be > > changed to use clang-min-version or something of that sort. > > > > [1]: https://lore.kernel.org/20231017172231.GA2348194@dev-arch.thelio-3990X/ > > [2]: https://lore.kernel.org/20231016142031.241912-1-Rodrigo.Siqueira@xxxxxxx/ > > > > Cheers, > > Nathan > -- > Hamza >
