diff mbox series

[2/2] drm/i915: enable W=1 warnings by default

Message ID f50c8ea0e63615aea28fe7f6049703e1d28ba7eb.1696595500.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: align with W=1 warnings | expand

Commit Message

Jani Nikula Oct. 6, 2023, 12:34 p.m. UTC
We enable a bunch more compiler warnings than the kernel
defaults. However, they've drifted to become a unique set of warnings,
and have increasingly fallen behind from the W=1 set.

Align with the W=1 warnings from scripts/Makefile.extrawarn for clarity,
by copy-pasting them with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it
easier to compare in the future.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

An alternative or future option would be to have Makefile.extrawarn
assign W=1 etc. flags to intermediate variables, say KBUILD_CFLAGS_W1,
like this:

KBUILD_CFLAGS_W1 += -Wextra -Wunused -Wno-unused-parameter
etc...

export KBUILD_CFLAGS_W1

ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)

KBUILD_CFLAGS += $(KBUILD_CFLAGS_W1)

else
etc...

and then drivers and subsystems could simply use:

subdir-ccflags-y += $(KBUILD_CFLAGS_W1)

to enable and remain up-to-date with W=1 warnings.
---
 drivers/gpu/drm/i915/Makefile | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

Comments

kernel test robot Oct. 6, 2023, 4:49 p.m. UTC | #1
Hi Jani,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-tip/drm-tip]

url:    https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-i915-drop-Wall-and-related-disables-from-cflags-as-redundant/20231006-203658
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:    https://lore.kernel.org/r/f50c8ea0e63615aea28fe7f6049703e1d28ba7eb.1696595500.git.jani.nikula%40intel.com
patch subject: [Intel-gfx] [PATCH 2/2] drm/i915: enable W=1 warnings by default
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231007/202310070011.Fji48IBk-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310070011.Fji48IBk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310070011.Fji48IBk-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/i915_driver.c:30:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/resource_ext.h:11:
   In file included from include/linux/slab.h:16:
   In file included from include/linux/gfp.h:7:
   In file included from include/linux/mmzone.h:8:
   In file included from include/linux/spinlock.h:56:
   In file included from include/linux/preempt.h:79:
   In file included from arch/x86/include/asm/preempt.h:9:
>> include/linux/thread_info.h:240:29: warning: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare]
           if (unlikely(sz >= 0 && sz < bytes)) {
                                   ~~ ^ ~~~~~
   include/linux/compiler.h:77:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   In file included from drivers/gpu/drm/i915/i915_driver.c:30:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/resource_ext.h:11:
   In file included from include/linux/slab.h:16:
   In file included from include/linux/gfp.h:7:
   In file included from include/linux/mmzone.h:22:
>> include/linux/mm_types.h:1036:13: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           return cid == MM_CID_UNSET;
                  ~~~ ^  ~~~~~~~~~~~~
   include/linux/mm_types.h:1074:2: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           for_each_possible_cpu(i) {
           ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:974:36: note: expanded from macro 'for_each_possible_cpu'
   #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:282:2: note: expanded from macro 'for_each_cpu'
           for_each_set_bit(cpu, cpumask_bits(mask), small_cpumask_bits)
           ^                ~~~                      ~~~~~~~~~~~~~~~~~~
   include/linux/find.h:559:70: note: expanded from macro 'for_each_set_bit'
           for ((bit) = 0; (bit) = find_next_bit((addr), (size), (bit)), (bit) < (size); (bit)++)
                                                                          ~~~  ^  ~~~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:30:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/resource_ext.h:11:
   In file included from include/linux/slab.h:16:
   In file included from include/linux/gfp.h:7:
>> include/linux/mmzone.h:1627:44: warning: comparison of integers of different signs: 'int' and 'enum zone_type' [-Wsign-compare]
           if (likely(!nodes && zonelist_zone_idx(z) <= highest_zoneidx))
                                ~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~
   include/linux/compiler.h:76:40: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                                               ^
   In file included from drivers/gpu/drm/i915/i915_driver.c:30:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/resource_ext.h:11:
   In file included from include/linux/slab.h:20:
>> include/linux/percpu-refcount.h:205:3: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
                   this_cpu_add(*percpu_count, nr);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:491:33: note: expanded from macro 'this_cpu_add'
   #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:368:11: note: expanded from macro '__pcpu_size_call'
                   case 8: stem##8(variable, __VA_ARGS__);break;           \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:49:1: note: expanded from here
   this_cpu_add_8
   ^
   arch/x86/include/asm/percpu.h:370:35: note: expanded from macro 'this_cpu_add_8'
   #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                                 ((val) == 1 || (val) == -1)) ?            \
                                                 ~~~  ^  ~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:30:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/resource_ext.h:11:
   In file included from include/linux/slab.h:20:
   include/linux/percpu-refcount.h:244:3: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
                   this_cpu_add(*percpu_count, nr);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:491:33: note: expanded from macro 'this_cpu_add'
   #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:368:11: note: expanded from macro '__pcpu_size_call'
                   case 8: stem##8(variable, __VA_ARGS__);break;           \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:101:1: note: expanded from here
   this_cpu_add_8
   ^
   arch/x86/include/asm/percpu.h:370:35: note: expanded from macro 'this_cpu_add_8'
   #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                                 ((val) == 1 || (val) == -1)) ?            \
                                                 ~~~  ^  ~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:30:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/resource_ext.h:11:
   In file included from include/linux/slab.h:20:
   include/linux/percpu-refcount.h:333:3: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
                   this_cpu_sub(*percpu_count, nr);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:500:33: note: expanded from macro 'this_cpu_sub'
   #define this_cpu_sub(pcp, val)          this_cpu_add(pcp, -(typeof(pcp))(val))
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:491:33: note: expanded from macro 'this_cpu_add'
   #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:368:11: note: expanded from macro '__pcpu_size_call'
                   case 8: stem##8(variable, __VA_ARGS__);break;           \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:209:1: note: expanded from here
   this_cpu_add_8
   ^
   arch/x86/include/asm/percpu.h:370:35: note: expanded from macro 'this_cpu_add_8'
   #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                                 ((val) == 1 || (val) == -1)) ?            \
                                                 ~~~  ^  ~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:33:
   In file included from include/linux/oom.h:11:
   In file included from include/linux/mm.h:2168:
>> include/linux/vmstat.h:231:2: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           for_each_online_cpu(cpu)
           ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:975:36: note: expanded from macro 'for_each_online_cpu'
   #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:282:2: note: expanded from macro 'for_each_cpu'
           for_each_set_bit(cpu, cpumask_bits(mask), small_cpumask_bits)
           ^                ~~~                      ~~~~~~~~~~~~~~~~~~
   include/linux/find.h:559:70: note: expanded from macro 'for_each_set_bit'
           for ((bit) = 0; (bit) = find_next_bit((addr), (size), (bit)), (bit) < (size); (bit)++)
                                                                          ~~~  ^  ~~~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:39:
   In file included from include/linux/vga_switcheroo.h:34:
   In file included from include/linux/fb.h:7:
   In file included from include/uapi/linux/fb.h:6:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:203:
>> include/linux/bio.h:401:14: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           if (sectors >= bio_sectors(bio))
               ~~~~~~~ ^  ~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:39:
   In file included from include/linux/vga_switcheroo.h:34:
   In file included from include/linux/fb.h:7:
   In file included from include/uapi/linux/fb.h:6:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:13:
>> include/linux/pagemap.h:813:36: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
           return index - folio_index(folio) < folio_nr_pages(folio);
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:52:
   In file included from drivers/gpu/drm/i915/display/intel_display_types.h:31:
>> include/linux/pwm.h:335:50: warning: comparison of integers of different signs: 'u64' (aka 'unsigned long long') and 'int' [-Wsign-compare]
           if (state.duty_cycle == duty_ns && state.period == period_ns)
                                              ~~~~~~~~~~~~ ^  ~~~~~~~~~
   include/linux/pwm.h:335:23: warning: comparison of integers of different signs: 'u64' (aka 'unsigned long long') and 'int' [-Wsign-compare]
           if (state.duty_cycle == duty_ns && state.period == period_ns)
               ~~~~~~~~~~~~~~~~ ^  ~~~~~~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:52:
   In file included from drivers/gpu/drm/i915/display/intel_display_types.h:49:
   In file included from drivers/gpu/drm/i915/i915_vma.h:34:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_object.h:15:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_object_types.h:13:
   In file included from include/drm/ttm/ttm_bo.h:39:
   In file included from include/drm/ttm/ttm_device.h:30:
>> include/drm/ttm/ttm_resource.h:312:16: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           for (i = 0; i < TTM_MAX_BO_PRIORITY; i++)
                       ~ ^ ~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/i915/i915_driver.c:52:
   In file included from drivers/gpu/drm/i915/display/intel_display_types.h:49:
   In file included from drivers/gpu/drm/i915/i915_vma.h:34:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_object.h:16:
   In file included from drivers/gpu/drm/i915/i915_gem_gtt.h:14:
>> drivers/gpu/drm/i915/gt/intel_gtt.h:435:16: warning: comparison of integers of different signs: 'int' and 'unsigned long' [-Wsign-compare]
           if ((int)type >= ARRAY_SIZE(vm->min_alignment))
               ~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   14 warnings generated.
--
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:7:
   In file included from include/linux/slab.h:16:
   In file included from include/linux/gfp.h:7:
   In file included from include/linux/mmzone.h:8:
   In file included from include/linux/spinlock.h:56:
   In file included from include/linux/preempt.h:79:
   In file included from arch/x86/include/asm/preempt.h:9:
>> include/linux/thread_info.h:240:29: warning: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare]
           if (unlikely(sz >= 0 && sz < bytes)) {
                                   ~~ ^ ~~~~~
   include/linux/compiler.h:77:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:7:
   In file included from include/linux/slab.h:16:
   In file included from include/linux/gfp.h:7:
   In file included from include/linux/mmzone.h:22:
>> include/linux/mm_types.h:1036:13: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           return cid == MM_CID_UNSET;
                  ~~~ ^  ~~~~~~~~~~~~
   include/linux/mm_types.h:1074:2: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           for_each_possible_cpu(i) {
           ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:974:36: note: expanded from macro 'for_each_possible_cpu'
   #define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:282:2: note: expanded from macro 'for_each_cpu'
           for_each_set_bit(cpu, cpumask_bits(mask), small_cpumask_bits)
           ^                ~~~                      ~~~~~~~~~~~~~~~~~~
   include/linux/find.h:559:70: note: expanded from macro 'for_each_set_bit'
           for ((bit) = 0; (bit) = find_next_bit((addr), (size), (bit)), (bit) < (size); (bit)++)
                                                                          ~~~  ^  ~~~~
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:7:
   In file included from include/linux/slab.h:16:
   In file included from include/linux/gfp.h:7:
>> include/linux/mmzone.h:1627:44: warning: comparison of integers of different signs: 'int' and 'enum zone_type' [-Wsign-compare]
           if (likely(!nodes && zonelist_zone_idx(z) <= highest_zoneidx))
                                ~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~
   include/linux/compiler.h:76:40: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                                               ^
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:7:
   In file included from include/linux/slab.h:20:
>> include/linux/percpu-refcount.h:205:3: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
                   this_cpu_add(*percpu_count, nr);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:491:33: note: expanded from macro 'this_cpu_add'
   #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:368:11: note: expanded from macro '__pcpu_size_call'
                   case 8: stem##8(variable, __VA_ARGS__);break;           \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:28:1: note: expanded from here
   this_cpu_add_8
   ^
   arch/x86/include/asm/percpu.h:370:35: note: expanded from macro 'this_cpu_add_8'
   #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                                 ((val) == 1 || (val) == -1)) ?            \
                                                 ~~~  ^  ~~
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:7:
   In file included from include/linux/slab.h:20:
   include/linux/percpu-refcount.h:244:3: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
                   this_cpu_add(*percpu_count, nr);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:491:33: note: expanded from macro 'this_cpu_add'
   #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:368:11: note: expanded from macro '__pcpu_size_call'
                   case 8: stem##8(variable, __VA_ARGS__);break;           \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:80:1: note: expanded from here
   this_cpu_add_8
   ^
   arch/x86/include/asm/percpu.h:370:35: note: expanded from macro 'this_cpu_add_8'
   #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                                 ((val) == 1 || (val) == -1)) ?            \
                                                 ~~~  ^  ~~
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:7:
   In file included from include/linux/slab.h:20:
   include/linux/percpu-refcount.h:333:3: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
                   this_cpu_sub(*percpu_count, nr);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:500:33: note: expanded from macro 'this_cpu_sub'
   #define this_cpu_sub(pcp, val)          this_cpu_add(pcp, -(typeof(pcp))(val))
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:491:33: note: expanded from macro 'this_cpu_add'
   #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:368:11: note: expanded from macro '__pcpu_size_call'
                   case 8: stem##8(variable, __VA_ARGS__);break;           \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:188:1: note: expanded from here
   this_cpu_add_8
   ^
   arch/x86/include/asm/percpu.h:370:35: note: expanded from macro 'this_cpu_add_8'
   #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                                 ((val) == 1 || (val) == -1)) ?            \
                                                 ~~~  ^  ~~
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:14:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_context.h:10:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_context_types.h:20:
   In file included from drivers/gpu/drm/i915/gt/intel_context_types.h:18:
   In file included from drivers/gpu/drm/i915/gt/intel_engine_types.h:21:
   In file included from drivers/gpu/drm/i915/i915_pmu.h:11:
   In file included from include/linux/perf_event.h:18:
   In file included from include/uapi/linux/bpf_perf_event.h:11:
   In file included from ./arch/x86/include/generated/uapi/asm/bpf_perf_event.h:1:
   In file included from include/uapi/asm-generic/bpf_perf_event.h:4:
   In file included from include/linux/ptrace.h:10:
   In file included from include/linux/pid_namespace.h:7:
   In file included from include/linux/mm.h:2168:
>> include/linux/vmstat.h:231:2: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           for_each_online_cpu(cpu)
           ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:975:36: note: expanded from macro 'for_each_online_cpu'
   #define for_each_online_cpu(cpu)   for_each_cpu((cpu), cpu_online_mask)
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:282:2: note: expanded from macro 'for_each_cpu'
           for_each_set_bit(cpu, cpumask_bits(mask), small_cpumask_bits)
           ^                ~~~                      ~~~~~~~~~~~~~~~~~~
   include/linux/find.h:559:70: note: expanded from macro 'for_each_set_bit'
           for ((bit) = 0; (bit) = find_next_bit((addr), (size), (bit)), (bit) < (size); (bit)++)
                                                                          ~~~  ^  ~~~~
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:14:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_context.h:12:
   In file included from drivers/gpu/drm/i915/gt/intel_context.h:14:
   In file included from drivers/gpu/drm/i915/i915_drv.h:37:
   In file included from include/drm/ttm/ttm_device.h:30:
>> include/drm/ttm/ttm_resource.h:312:16: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
           for (i = 0; i < TTM_MAX_BO_PRIORITY; i++)
                       ~ ^ ~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/i915/i915_drm_client.c:14:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_context.h:12:
   In file included from drivers/gpu/drm/i915/gt/intel_context.h:14:
   In file included from drivers/gpu/drm/i915/i915_drv.h:46:
   In file included from drivers/gpu/drm/i915/gt/intel_engine.h:18:
   In file included from drivers/gpu/drm/i915/gt/intel_gt_types.h:19:
   In file included from drivers/gpu/drm/i915/gt/uc/intel_uc.h:9:
   In file included from drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h:9:
   In file included from drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h:14:
   In file included from drivers/gpu/drm/i915/i915_vma.h:34:
   In file included from drivers/gpu/drm/i915/gem/i915_gem_object.h:16:
   In file included from drivers/gpu/drm/i915/i915_gem_gtt.h:14:
>> drivers/gpu/drm/i915/gt/intel_gtt.h:435:16: warning: comparison of integers of different signs: 'int' and 'unsigned long' [-Wsign-compare]
           if ((int)type >= ARRAY_SIZE(vm->min_alignment))
               ~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   10 warnings generated.
..


vim +240 include/linux/thread_info.h

b0377fedb65280 Al Viro   2017-06-29  235  
9dd819a15162f8 Kees Cook 2019-09-25  236  static __always_inline __must_check bool
b0377fedb65280 Al Viro   2017-06-29  237  check_copy_size(const void *addr, size_t bytes, bool is_source)
b0377fedb65280 Al Viro   2017-06-29  238  {
c80d92fbb67b2c Kees Cook 2021-06-17  239  	int sz = __builtin_object_size(addr, 0);
b0377fedb65280 Al Viro   2017-06-29 @240  	if (unlikely(sz >= 0 && sz < bytes)) {
b0377fedb65280 Al Viro   2017-06-29  241  		if (!__builtin_constant_p(bytes))
b0377fedb65280 Al Viro   2017-06-29  242  			copy_overflow(sz, bytes);
b0377fedb65280 Al Viro   2017-06-29  243  		else if (is_source)
b0377fedb65280 Al Viro   2017-06-29  244  			__bad_copy_from();
b0377fedb65280 Al Viro   2017-06-29  245  		else
b0377fedb65280 Al Viro   2017-06-29  246  			__bad_copy_to();
b0377fedb65280 Al Viro   2017-06-29  247  		return false;
b0377fedb65280 Al Viro   2017-06-29  248  	}
6d13de1489b6bf Kees Cook 2019-12-04  249  	if (WARN_ON_ONCE(bytes > INT_MAX))
6d13de1489b6bf Kees Cook 2019-12-04  250  		return false;
b0377fedb65280 Al Viro   2017-06-29  251  	check_object_size(addr, bytes, is_source);
b0377fedb65280 Al Viro   2017-06-29  252  	return true;
b0377fedb65280 Al Viro   2017-06-29  253  }
b0377fedb65280 Al Viro   2017-06-29  254
Nathan Chancellor Oct. 6, 2023, 5:45 p.m. UTC | #2
On Fri, Oct 06, 2023 at 03:34:47PM +0300, Jani Nikula wrote:
> We enable a bunch more compiler warnings than the kernel
> defaults. However, they've drifted to become a unique set of warnings,
> and have increasingly fallen behind from the W=1 set.
> 
> Align with the W=1 warnings from scripts/Makefile.extrawarn for clarity,
> by copy-pasting them with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it
> easier to compare in the future.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

One meta comment and review comment below. Feel free to carry forward

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

on future revisions.

> ---
> 
> An alternative or future option would be to have Makefile.extrawarn
> assign W=1 etc. flags to intermediate variables, say KBUILD_CFLAGS_W1,
> like this:
> 
> KBUILD_CFLAGS_W1 += -Wextra -Wunused -Wno-unused-parameter
> etc...
> 
> export KBUILD_CFLAGS_W1
> 
> ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
> 
> KBUILD_CFLAGS += $(KBUILD_CFLAGS_W1)
> 
> else
> etc...
> 
> and then drivers and subsystems could simply use:
> 
> subdir-ccflags-y += $(KBUILD_CFLAGS_W1)
> 
> to enable and remain up-to-date with W=1 warnings.

This has definitely come up a few times and while I am generally in
favor of something like this, it makes adding warnings to W=1 a little
bit harder because when we add warnings to W=1, we typically are not
concerned with breaking the build, as W=1 is not the default build. If a
subsystem has opted into "whatever the current W=1 is" by default,
changes to W=1 will have to be reviewed/tested within a normal build.

Doing something like this patch with a more regular cadence (maybe every
update after the merge window) seems like a reasonable compromise to me,
although I know that means more work for individual subsystem
maintainers.

> ---
>  drivers/gpu/drm/i915/Makefile | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 623f81217442..0485157054fc 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -3,22 +3,25 @@
>  # Makefile for the drm device driver.  This driver provides support for the
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
> -# Add a set of useful warning flags and enable -Werror for CI to prevent
> -# trivial mistakes from creeping in. We have to do this piecemeal as we reject
> -# any patch that isn't warning clean, so turning on -Wextra (or W=1) we
> -# need to filter out dubious warnings.  Still it is our interest
> -# to keep running locally with W=1 C=1 until we are completely clean.
> -#
> -# Note the danger in using -Wextra is that when CI updates gcc we
> -# will most likely get a sudden build breakage... Hopefully we will fix
> -# new warnings before CI updates!
> -subdir-ccflags-y := -Wextra
> -subdir-ccflags-y += -Wno-unused-parameter
> -subdir-ccflags-y += -Wno-type-limits
> -subdir-ccflags-y += -Wno-missing-field-initializers
> -subdir-ccflags-y += -Wno-sign-compare
> -subdir-ccflags-y += -Wno-shift-negative-value

As the test robot points out, you'll want to keep these four, as they
are only enabled for W=2 or W=3. With this diff on top of these two
patches:

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0485157054fc..9c4e09c8aa4e 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -21,6 +21,12 @@ subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
 subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
 # --- end copy-paste
 
+# The following turn off the warnings enabled by -Wextra
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-shift-negative-value
+
 # Enable -Werror in CI and development
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 
I can continue to build i915 warning free with ARCH=x86_64 allmodconfig
using all supported versions of LLVM for building the kernel.

> +# Unconditionally enable W=1 warnings locally
> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
> +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
> +subdir-ccflags-y += -Wmissing-declarations
> +subdir-ccflags-y += $(call cc-option, -Wrestrict)
> +subdir-ccflags-y += -Wmissing-format-attribute
> +subdir-ccflags-y += -Wmissing-prototypes
> +subdir-ccflags-y += -Wold-style-definition
> +subdir-ccflags-y += -Wmissing-include-dirs
>  subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
> +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
> +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
> +subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
> +subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
> +subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
> +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
> +# --- end copy-paste
> +
> +# Enable -Werror in CI and development
>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>  
>  # Fine grained warnings disable
> -- 
> 2.39.2
>
Jani Nikula Oct. 10, 2023, 8:33 a.m. UTC | #3
On Fri, 06 Oct 2023, Nathan Chancellor <nathan@kernel.org> wrote:
> On Fri, Oct 06, 2023 at 03:34:47PM +0300, Jani Nikula wrote:
>> We enable a bunch more compiler warnings than the kernel
>> defaults. However, they've drifted to become a unique set of warnings,
>> and have increasingly fallen behind from the W=1 set.
>> 
>> Align with the W=1 warnings from scripts/Makefile.extrawarn for clarity,
>> by copy-pasting them with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it
>> easier to compare in the future.
>> 
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Cc: Nathan Chancellor <nathan@kernel.org>
>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> One meta comment and review comment below. Feel free to carry forward
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> on future revisions.

Thanks!

>
>> ---
>> 
>> An alternative or future option would be to have Makefile.extrawarn
>> assign W=1 etc. flags to intermediate variables, say KBUILD_CFLAGS_W1,
>> like this:
>> 
>> KBUILD_CFLAGS_W1 += -Wextra -Wunused -Wno-unused-parameter
>> etc...
>> 
>> export KBUILD_CFLAGS_W1
>> 
>> ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
>> 
>> KBUILD_CFLAGS += $(KBUILD_CFLAGS_W1)
>> 
>> else
>> etc...
>> 
>> and then drivers and subsystems could simply use:
>> 
>> subdir-ccflags-y += $(KBUILD_CFLAGS_W1)
>> 
>> to enable and remain up-to-date with W=1 warnings.
>
> This has definitely come up a few times and while I am generally in
> favor of something like this, it makes adding warnings to W=1 a little
> bit harder because when we add warnings to W=1, we typically are not
> concerned with breaking the build, as W=1 is not the default build. If a
> subsystem has opted into "whatever the current W=1 is" by default,
> changes to W=1 will have to be reviewed/tested within a normal build.
>
> Doing something like this patch with a more regular cadence (maybe every
> update after the merge window) seems like a reasonable compromise to me,
> although I know that means more work for individual subsystem
> maintainers.

Makes sense.

>
>> ---
>>  drivers/gpu/drm/i915/Makefile | 33 ++++++++++++++++++---------------
>>  1 file changed, 18 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 623f81217442..0485157054fc 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -3,22 +3,25 @@
>>  # Makefile for the drm device driver.  This driver provides support for the
>>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>>  
>> -# Add a set of useful warning flags and enable -Werror for CI to prevent
>> -# trivial mistakes from creeping in. We have to do this piecemeal as we reject
>> -# any patch that isn't warning clean, so turning on -Wextra (or W=1) we
>> -# need to filter out dubious warnings.  Still it is our interest
>> -# to keep running locally with W=1 C=1 until we are completely clean.
>> -#
>> -# Note the danger in using -Wextra is that when CI updates gcc we
>> -# will most likely get a sudden build breakage... Hopefully we will fix
>> -# new warnings before CI updates!
>> -subdir-ccflags-y := -Wextra
>> -subdir-ccflags-y += -Wno-unused-parameter
>> -subdir-ccflags-y += -Wno-type-limits
>> -subdir-ccflags-y += -Wno-missing-field-initializers
>> -subdir-ccflags-y += -Wno-sign-compare
>> -subdir-ccflags-y += -Wno-shift-negative-value
>
> As the test robot points out, you'll want to keep these four, as they
> are only enabled for W=2 or W=3. With this diff on top of these two
> patches:

Right. I was confused by the refactoring done in Makefile.extrawarn, and
didn't notice that the disables were moved to else branches of the
checks for W=2 and W=3.

BR,
Jani.

>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0485157054fc..9c4e09c8aa4e 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -21,6 +21,12 @@ subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
>  subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
>  # --- end copy-paste
>  
> +# The following turn off the warnings enabled by -Wextra
> +subdir-ccflags-y += -Wno-type-limits
> +subdir-ccflags-y += -Wno-missing-field-initializers
> +subdir-ccflags-y += -Wno-sign-compare
> +subdir-ccflags-y += -Wno-shift-negative-value
> +
>  # Enable -Werror in CI and development
>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>  
> I can continue to build i915 warning free with ARCH=x86_64 allmodconfig
> using all supported versions of LLVM for building the kernel.
>
>> +# Unconditionally enable W=1 warnings locally
>> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
>> +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
>> +subdir-ccflags-y += -Wmissing-declarations
>> +subdir-ccflags-y += $(call cc-option, -Wrestrict)
>> +subdir-ccflags-y += -Wmissing-format-attribute
>> +subdir-ccflags-y += -Wmissing-prototypes
>> +subdir-ccflags-y += -Wold-style-definition
>> +subdir-ccflags-y += -Wmissing-include-dirs
>>  subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
>> +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
>> +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
>> +subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
>> +subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
>> +subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
>> +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
>> +# --- end copy-paste
>> +
>> +# Enable -Werror in CI and development
>>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>>  
>>  # Fine grained warnings disable
>> -- 
>> 2.39.2
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 623f81217442..0485157054fc 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -3,22 +3,25 @@ 
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-# Add a set of useful warning flags and enable -Werror for CI to prevent
-# trivial mistakes from creeping in. We have to do this piecemeal as we reject
-# any patch that isn't warning clean, so turning on -Wextra (or W=1) we
-# need to filter out dubious warnings.  Still it is our interest
-# to keep running locally with W=1 C=1 until we are completely clean.
-#
-# Note the danger in using -Wextra is that when CI updates gcc we
-# will most likely get a sudden build breakage... Hopefully we will fix
-# new warnings before CI updates!
-subdir-ccflags-y := -Wextra
-subdir-ccflags-y += -Wno-unused-parameter
-subdir-ccflags-y += -Wno-type-limits
-subdir-ccflags-y += -Wno-missing-field-initializers
-subdir-ccflags-y += -Wno-sign-compare
-subdir-ccflags-y += -Wno-shift-negative-value
+# Unconditionally enable W=1 warnings locally
+# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
+subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
+subdir-ccflags-y += -Wmissing-declarations
+subdir-ccflags-y += $(call cc-option, -Wrestrict)
+subdir-ccflags-y += -Wmissing-format-attribute
+subdir-ccflags-y += -Wmissing-prototypes
+subdir-ccflags-y += -Wold-style-definition
+subdir-ccflags-y += -Wmissing-include-dirs
 subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
+subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
+subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
+subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
+subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
+subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
+subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
+# --- end copy-paste
+
+# Enable -Werror in CI and development
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 
 # Fine grained warnings disable