2023-09-12 19:19:33

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 0/7] bcachefs compiler warning fixes for 32-bit

Hi all,

This is a series of fixes for warnings that I now see from bcachefs when
building my test matrix with LLVM in -next, mostly from 32-bit
architectures. Most of the patches should be uncontroversial; the
min_t/max_t changes are probably the worst ones.

I still see several instances of -Wframe-larger-than when building for
32-bit ARM (I am sure they will show up on other 32-bit architectures as
well), which I am not entirely sure how to tackle. It looks like the
majority of the instances are just due to large structures on the stack
in combination with some inlining resulting in some spills, so it seems
like moving to a heap allocation for some of those would the right fix
but I know some maintainers would rather fix them in their own way,
hence just the report. I have included the compiler output (clang and
GCC in some cases) along with the output of clang's
-Rpass-analysis=stack-frame-layout, which helps with seeing where the
extra stack usage comes from. Tallying up the larger variables shows
that most of these functions are pushing the 32-bit limit of 1024 bytes
without any inlining (+/- 50-100 bytes in majority of cases).

===

fs/bcachefs/fs-common.c:356:5: error: stack frame size (1128) exceeds limit (1024) in 'bch2_rename_trans' [-Werror,-Wframe-larger-than]
356 | int bch2_rename_trans(struct btree_trans *trans,
| ^

fs/bcachefs/fs-common.c:364:1: remark:
Function: bch2_rename_trans
Offset: [SP+52], Type: Variable, Align: 4, Size: 4
Offset: [SP+48], Type: Variable, Align: 8, Size: 4
Offset: [SP+44], Type: Variable, Align: 4, Size: 4
Offset: [SP+40], Type: Variable, Align: 8, Size: 4
Offset: [SP+36], Type: Variable, Align: 4, Size: 4
Offset: [SP+32], Type: Variable, Align: 8, Size: 40
Offset: [SP+28], Type: Variable, Align: 4, Size: 4
Offset: [SP+24], Type: Variable, Align: 8, Size: 4
Offset: [SP+20], Type: Variable, Align: 4, Size: 4
Offset: [SP+16], Type: Variable, Align: 8, Size: 4
Offset: [SP+8], Type: Variable, Align: 8, Size: 4
Offset: [SP+4], Type: Variable, Align: 4, Size: 4
Offset: [SP+0], Type: Variable, Align: 8, Size: 4
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-424], Type: Variable, Align: 8, Size: 376
Offset: [SP-456], Type: Variable, Align: 8, Size: 32
Offset: [SP-576], Type: Variable, Align: 8, Size: 120
Offset: [SP-696], Type: Variable, Align: 8, Size: 120
Offset: [SP-816], Type: Variable, Align: 8, Size: 120
Offset: [SP-936], Type: Variable, Align: 8, Size: 120
Offset: [SP-960], Type: Variable, Align: 8, Size: 24
Offset: [SP-984], Type: Variable, Align: 8, Size: 24
Offset: [SP-1000], Type: Variable, Align: 8, Size: 16
Offset: [SP-1016], Type: Variable, Align: 8, Size: 16
Offset: [SP-1024], Type: Variable, Align: 8, Size: 8
Offset: [SP-1032], Type: Variable, Align: 8, Size: 8
Offset: [SP-1036], Type: Spill, Align: 4, Size: 4
Offset: [SP-1040], Type: Spill, Align: 4, Size: 4
Offset: [SP-1044], Type: Spill, Align: 4, Size: 4
Offset: [SP-1048], Type: Spill, Align: 4, Size: 4
Offset: [SP-1052], Type: Spill, Align: 4, Size: 4
Offset: [SP-1056], Type: Spill, Align: 4, Size: 4
Offset: [SP-1060], Type: Spill, Align: 4, Size: 4
Offset: [SP-1064], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
364 | {
| ^

===

fs/bcachefs/reflink.c:244:5: error: stack frame size (1256) exceeds limit (1024) in 'bch2_remap_range' [-Werror,-Wframe-larger-than]
244 | s64 bch2_remap_range(struct bch_fs *c,
| ^

fs/bcachefs/reflink.c: In function 'bch2_remap_range':
fs/bcachefs/reflink.c:399:1: error: the frame size of 1192 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
399 | }
| ^

fs/bcachefs/reflink.c:249:1: remark:
Function: bch2_remap_range
Offset: [SP+56], Type: Variable, Align: 8, Size: 4
Offset: [SP+52], Type: Variable, Align: 4, Size: 4
Offset: [SP+48], Type: Variable, Align: 8, Size: 4
Offset: [SP+44], Type: Variable, Align: 4, Size: 4
Offset: [SP+40], Type: Variable, Align: 8, Size: 4
Offset: [SP+36], Type: Variable, Align: 4, Size: 4
Offset: [SP+32], Type: Variable, Align: 8, Size: 4
Offset: [SP+28], Type: Variable, Align: 4, Size: 4
Offset: [SP+24], Type: Variable, Align: 8, Size: 4
Offset: [SP+20], Type: Variable, Align: 4, Size: 4
Offset: [SP+16], Type: Variable, Align: 8, Size: 4
Offset: [SP+12], Type: Variable, Align: 4, Size: 4
Offset: [SP+8], Type: Variable, Align: 8, Size: 4
Offset: [SP+4], Type: Variable, Align: 4, Size: 4
Offset: [SP+0], Type: Variable, Align: 8, Size: 4
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-360], Type: Variable, Align: 8, Size: 312
Offset: [SP-464], Type: Variable, Align: 8, Size: 104
Offset: [SP-568], Type: Variable, Align: 8, Size: 104
Offset: [SP-688], Type: Variable, Align: 8, Size: 120
Offset: [SP-808], Type: Variable, Align: 8, Size: 120
Offset: [SP-968], Type: Variable, Align: 8, Size: 160
Offset: [SP-1088], Type: Variable, Align: 8, Size: 120
Offset: [SP-1096], Type: Variable, Align: 4, Size: 8
Offset: [SP-1100], Type: Variable, Align: 4, Size: 4
Offset: [SP-1104], Type: Variable, Align: 4, Size: 4
Offset: [SP-1108], Type: Spill, Align: 4, Size: 4
Offset: [SP-1112], Type: Spill, Align: 4, Size: 4
Offset: [SP-1116], Type: Spill, Align: 4, Size: 4
Offset: [SP-1120], Type: Spill, Align: 4, Size: 4
Offset: [SP-1124], Type: Spill, Align: 4, Size: 4
Offset: [SP-1128], Type: Spill, Align: 4, Size: 4
Offset: [SP-1132], Type: Spill, Align: 4, Size: 4
Offset: [SP-1136], Type: Spill, Align: 4, Size: 4
Offset: [SP-1140], Type: Spill, Align: 4, Size: 4
Offset: [SP-1144], Type: Spill, Align: 4, Size: 4
Offset: [SP-1148], Type: Spill, Align: 4, Size: 4
Offset: [SP-1152], Type: Spill, Align: 4, Size: 4
Offset: [SP-1156], Type: Spill, Align: 4, Size: 4
Offset: [SP-1160], Type: Spill, Align: 4, Size: 4
Offset: [SP-1164], Type: Spill, Align: 4, Size: 4
Offset: [SP-1168], Type: Spill, Align: 4, Size: 4
Offset: [SP-1172], Type: Spill, Align: 4, Size: 4
Offset: [SP-1176], Type: Spill, Align: 4, Size: 4
Offset: [SP-1180], Type: Spill, Align: 4, Size: 4
Offset: [SP-1184], Type: Spill, Align: 4, Size: 4
Offset: [SP-1188], Type: Spill, Align: 4, Size: 4
Offset: [SP-1192], Type: Spill, Align: 4, Size: 4
Offset: [SP-1196], Type: Spill, Align: 4, Size: 4
Offset: [SP-1200], Type: Spill, Align: 4, Size: 4
Offset: [SP-1204], Type: Spill, Align: 4, Size: 4
Offset: [SP-1208], Type: Spill, Align: 4, Size: 4
Offset: [SP-1212], Type: Spill, Align: 4, Size: 4
Offset: [SP-1216], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
249 | {
| ^

===

fs/bcachefs/recovery.c: In function 'bch2_fs_initialize':
fs/bcachefs/recovery.c:1057:1: error: the frame size of 1096 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
1057 | }
| ^

fs/bcachefs/recovery.c:924:5: error: stack frame size (1184) exceeds limit (1024) in 'bch2_fs_initialize' [-Werror,-Wframe-larger-than]
924 | int bch2_fs_initialize(struct bch_fs *c)
| ^

fs/bcachefs/recovery.c:925:1: remark:
Function: bch2_fs_initialize
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-440], Type: Variable, Align: 8, Size: 392
Offset: [SP-752], Type: Variable, Align: 8, Size: 312
Offset: [SP-912], Type: Variable, Align: 8, Size: 160
Offset: [SP-1072], Type: Variable, Align: 8, Size: 160
Offset: [SP-1088], Type: Variable, Align: 8, Size: 16
Offset: [SP-1092], Type: Variable, Align: 4, Size: 4
Offset: [SP-1096], Type: Spill, Align: 4, Size: 4
Offset: [SP-1100], Type: Spill, Align: 4, Size: 4
Offset: [SP-1104], Type: Spill, Align: 4, Size: 4
Offset: [SP-1108], Type: Spill, Align: 4, Size: 4
Offset: [SP-1112], Type: Spill, Align: 4, Size: 4
Offset: [SP-1116], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
925 | {
| ^

===

fs/bcachefs/fs.c: In function 'bch2_rename2':
fs/bcachefs/fs.c:639:1: error: the frame size of 1072 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
639 | }
| ^

fs/bcachefs/fs.c:534:12: error: stack frame size (1096) exceeds limit (1024) in 'bch2_rename2' [-Werror,-Wframe-larger-than]
534 | static int bch2_rename2(struct mnt_idmap *idmap,
| ^

fs/bcachefs/fs.c:538:1: remark:
Function: bch2_rename2
Offset: [SP+4], Type: Variable, Align: 4, Size: 4
Offset: [SP+0], Type: Variable, Align: 8, Size: 4
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-360], Type: Variable, Align: 8, Size: 312
Offset: [SP-520], Type: Variable, Align: 8, Size: 160
Offset: [SP-680], Type: Variable, Align: 8, Size: 160
Offset: [SP-840], Type: Variable, Align: 8, Size: 160
Offset: [SP-1000], Type: Variable, Align: 8, Size: 160
Offset: [SP-1004], Type: Spill, Align: 4, Size: 4
Offset: [SP-1008], Type: Spill, Align: 4, Size: 4
Offset: [SP-1012], Type: Spill, Align: 4, Size: 4
Offset: [SP-1016], Type: Spill, Align: 4, Size: 4
Offset: [SP-1020], Type: Spill, Align: 4, Size: 4
Offset: [SP-1024], Type: Spill, Align: 4, Size: 4
Offset: [SP-1028], Type: Spill, Align: 4, Size: 4
Offset: [SP-1032], Type: Spill, Align: 4, Size: 4
Offset: [SP-1036], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
538 | {
| ^

===

fs/bcachefs/fs-io.c: In function 'bchfs_fcollapse_finsert':
fs/bcachefs/fs-io.c:749:1: error: the frame size of 1032 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
749 | }
| ^

fs/bcachefs/fs-io.c:573:13: error: stack frame size (1088) exceeds limit (1024) in 'bchfs_fcollapse_finsert' [-Werror,-Wframe-larger-than]
573 | static long bchfs_fcollapse_finsert(struct bch_inode_info *inode,
| ^

fs/bcachefs/fs-io.c:576:1: remark:
Function: bchfs_fcollapse_finsert
Offset: [SP+8], Type: Variable, Align: 8, Size: 4
Offset: [SP+4], Type: Variable, Align: 4, Size: 4
Offset: [SP+0], Type: Variable, Align: 8, Size: 4
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-152], Type: Variable, Align: 8, Size: 104
Offset: [SP-464], Type: Variable, Align: 8, Size: 312
Offset: [SP-584], Type: Variable, Align: 8, Size: 120
Offset: [SP-704], Type: Variable, Align: 8, Size: 120
Offset: [SP-824], Type: Variable, Align: 8, Size: 120
Offset: [SP-864], Type: Variable, Align: 8, Size: 40
Offset: [SP-880], Type: Variable, Align: 8, Size: 16
Offset: [SP-904], Type: Variable, Align: 8, Size: 20
Offset: [SP-908], Type: Variable, Align: 4, Size: 4
Offset: [SP-916], Type: Variable, Align: 4, Size: 8
Offset: [SP-920], Type: Spill, Align: 4, Size: 4
Offset: [SP-924], Type: Spill, Align: 4, Size: 4
Offset: [SP-928], Type: Spill, Align: 4, Size: 4
Offset: [SP-932], Type: Spill, Align: 4, Size: 4
Offset: [SP-936], Type: Spill, Align: 4, Size: 4
Offset: [SP-940], Type: Spill, Align: 4, Size: 4
Offset: [SP-944], Type: Spill, Align: 4, Size: 4
Offset: [SP-948], Type: Spill, Align: 4, Size: 4
Offset: [SP-952], Type: Spill, Align: 4, Size: 4
Offset: [SP-956], Type: Spill, Align: 4, Size: 4
Offset: [SP-960], Type: Spill, Align: 4, Size: 4
Offset: [SP-964], Type: Spill, Align: 4, Size: 4
Offset: [SP-968], Type: Spill, Align: 4, Size: 4
Offset: [SP-972], Type: Spill, Align: 4, Size: 4
Offset: [SP-976], Type: Spill, Align: 4, Size: 4
Offset: [SP-980], Type: Spill, Align: 4, Size: 4
Offset: [SP-984], Type: Spill, Align: 4, Size: 4
Offset: [SP-988], Type: Spill, Align: 4, Size: 4
Offset: [SP-992], Type: Spill, Align: 4, Size: 4
Offset: [SP-996], Type: Spill, Align: 4, Size: 4
Offset: [SP-1000], Type: Spill, Align: 4, Size: 4
Offset: [SP-1004], Type: Spill, Align: 4, Size: 4
Offset: [SP-1008], Type: Spill, Align: 4, Size: 4
Offset: [SP-1012], Type: Spill, Align: 4, Size: 4
Offset: [SP-1016], Type: Spill, Align: 4, Size: 4
Offset: [SP-1020], Type: Spill, Align: 4, Size: 4
Offset: [SP-1024], Type: Spill, Align: 4, Size: 4
Offset: [SP-1028], Type: Spill, Align: 4, Size: 4
Offset: [SP-1032], Type: Spill, Align: 4, Size: 4
Offset: [SP-1036], Type: Spill, Align: 4, Size: 4
Offset: [SP-1040], Type: Spill, Align: 4, Size: 4
Offset: [SP-1044], Type: Spill, Align: 4, Size: 4
Offset: [SP-1048], Type: Spill, Align: 4, Size: 4
Offset: [SP-1052], Type: Spill, Align: 4, Size: 4
Offset: [SP-1056], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
576 | {
| ^

===

fs/bcachefs/fsck.c:1806:5: error: stack frame size (1232) exceeds limit (1024) in 'bch2_check_dirents' [-Werror,-Wframe-larger-than]
1806 | int bch2_check_dirents(struct bch_fs *c)
| ^

fs/bcachefs/fsck.c:1807:1: remark:
Function: bch2_check_dirents
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-424], Type: Variable, Align: 8, Size: 376
Offset: [SP-464], Type: Variable, Align: 8, Size: 40
Offset: [SP-776], Type: Variable, Align: 8, Size: 312
Offset: [SP-816], Type: Variable, Align: 8, Size: 36
Offset: [SP-856], Type: Variable, Align: 8, Size: 36
Offset: [SP-896], Type: Variable, Align: 8, Size: 36
Offset: [SP-1016], Type: Variable, Align: 8, Size: 120
Offset: [SP-1020], Type: Variable, Align: 4, Size: 4
Offset: [SP-1056], Type: Variable, Align: 8, Size: 32
Offset: [SP-1080], Type: Variable, Align: 8, Size: 24
Offset: [SP-1084], Type: Spill, Align: 4, Size: 4
Offset: [SP-1088], Type: Spill, Align: 4, Size: 4
Offset: [SP-1092], Type: Spill, Align: 4, Size: 4
Offset: [SP-1096], Type: Spill, Align: 4, Size: 4
Offset: [SP-1100], Type: Spill, Align: 4, Size: 4
Offset: [SP-1104], Type: Spill, Align: 4, Size: 4
Offset: [SP-1108], Type: Spill, Align: 4, Size: 4
Offset: [SP-1112], Type: Spill, Align: 4, Size: 4
Offset: [SP-1116], Type: Spill, Align: 4, Size: 4
Offset: [SP-1120], Type: Spill, Align: 4, Size: 4
Offset: [SP-1124], Type: Spill, Align: 4, Size: 4
Offset: [SP-1128], Type: Spill, Align: 4, Size: 4
Offset: [SP-1132], Type: Spill, Align: 4, Size: 4
Offset: [SP-1136], Type: Spill, Align: 4, Size: 4
Offset: [SP-1140], Type: Spill, Align: 4, Size: 4
Offset: [SP-1144], Type: Spill, Align: 4, Size: 4
Offset: [SP-1148], Type: Spill, Align: 4, Size: 4
Offset: [SP-1152], Type: Spill, Align: 4, Size: 4
Offset: [SP-1156], Type: Spill, Align: 4, Size: 4
Offset: [SP-1160], Type: Spill, Align: 4, Size: 4
Offset: [SP-1164], Type: Spill, Align: 4, Size: 4
Offset: [SP-1168], Type: Spill, Align: 4, Size: 4
Offset: [SP-1172], Type: Spill, Align: 4, Size: 4
Offset: [SP-1176], Type: Spill, Align: 4, Size: 4
Offset: [SP-1180], Type: Spill, Align: 4, Size: 4
Offset: [SP-1184], Type: Spill, Align: 4, Size: 4
Offset: [SP-1188], Type: Spill, Align: 4, Size: 4
Offset: [SP-1192], Type: Spill, Align: 4, Size: 4
Offset: [SP-1196], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
1807 | {
| ^

===

fs/bcachefs/fsck.c:1878:5: error: stack frame size (1040) exceeds limit (1024) in 'bch2_check_xattrs' [-Werror,-Wframe-larger-than]
1878 | int bch2_check_xattrs(struct bch_fs *c)
| ^

fs/bcachefs/fsck.c:1879:1: remark:
Function: bch2_check_xattrs
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-424], Type: Variable, Align: 8, Size: 376
Offset: [SP-456], Type: Variable, Align: 8, Size: 32
Offset: [SP-768], Type: Variable, Align: 8, Size: 312
Offset: [SP-808], Type: Variable, Align: 8, Size: 36
Offset: [SP-928], Type: Variable, Align: 8, Size: 120
Offset: [SP-952], Type: Variable, Align: 8, Size: 24
Offset: [SP-956], Type: Spill, Align: 4, Size: 4
Offset: [SP-960], Type: Spill, Align: 4, Size: 4
Offset: [SP-964], Type: Spill, Align: 4, Size: 4
Offset: [SP-968], Type: Spill, Align: 4, Size: 4
Offset: [SP-972], Type: Spill, Align: 4, Size: 4
Offset: [SP-976], Type: Spill, Align: 4, Size: 4
Offset: [SP-980], Type: Spill, Align: 4, Size: 4
Offset: [SP-984], Type: Spill, Align: 4, Size: 4
Offset: [SP-988], Type: Spill, Align: 4, Size: 4
Offset: [SP-992], Type: Spill, Align: 4, Size: 4
Offset: [SP-996], Type: Spill, Align: 4, Size: 4
Offset: [SP-1000], Type: Spill, Align: 4, Size: 4
Offset: [SP-1004], Type: Spill, Align: 4, Size: 4
Offset: [SP-1008], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
1879 | {
| ^

===

fs/bcachefs/fsck.c:359:12: error: stack frame size (1040) exceeds limit (1024) in 'reattach_inode' [-Werror,-Wframe-larger-than]
359 | static int reattach_inode(struct btree_trans *trans,
| ^

fs/bcachefs/fsck.c:362:1: remark:
Function: reattach_inode
Offset: [SP-4], Type: Spill, Align: 4, Size: 4
Offset: [SP-8], Type: Spill, Align: 4, Size: 4
Offset: [SP-12], Type: Spill, Align: 4, Size: 4
Offset: [SP-16], Type: Spill, Align: 4, Size: 4
Offset: [SP-20], Type: Spill, Align: 4, Size: 4
Offset: [SP-24], Type: Spill, Align: 4, Size: 4
Offset: [SP-28], Type: Spill, Align: 4, Size: 4
Offset: [SP-32], Type: Spill, Align: 4, Size: 4
Offset: [SP-36], Type: Spill, Align: 4, Size: 4
Offset: [SP-40], Type: Variable, Align: 4, Size: 4
Offset: [SP-44], Type: Protector, Align: 4, Size: 4
Offset: [SP-424], Type: Variable, Align: 8, Size: 376
Offset: [SP-456], Type: Variable, Align: 8, Size: 32
Offset: [SP-616], Type: Variable, Align: 8, Size: 160
Offset: [SP-640], Type: Variable, Align: 8, Size: 20
Offset: [SP-648], Type: Variable, Align: 4, Size: 8
Offset: [SP-664], Type: Variable, Align: 8, Size: 16
Offset: [SP-668], Type: Variable, Align: 4, Size: 4
Offset: [SP-696], Type: Variable, Align: 8, Size: 24
Offset: [SP-856], Type: Variable, Align: 8, Size: 160
Offset: [SP-872], Type: Variable, Align: 8, Size: 16
Offset: [SP-880], Type: Variable, Align: 8, Size: 8
Offset: [SP-884], Type: Spill, Align: 4, Size: 4
Offset: [SP-888], Type: Spill, Align: 4, Size: 4
Offset: [SP-892], Type: Spill, Align: 4, Size: 4
Offset: [SP-896], Type: Spill, Align: 4, Size: 4
Offset: [SP-900], Type: Spill, Align: 4, Size: 4
Offset: [SP-904], Type: Spill, Align: 4, Size: 4
Offset: [SP-908], Type: Spill, Align: 4, Size: 4
Offset: [SP-912], Type: Spill, Align: 4, Size: 4
Offset: [SP-916], Type: Spill, Align: 4, Size: 4
Offset: [SP-920], Type: Spill, Align: 4, Size: 4
Offset: [SP-924], Type: Spill, Align: 4, Size: 4
Offset: [SP-928], Type: Spill, Align: 4, Size: 4
Offset: [SP-932], Type: Spill, Align: 4, Size: 4
Offset: [SP-936], Type: Spill, Align: 4, Size: 4
Offset: [SP-940], Type: Spill, Align: 4, Size: 4
Offset: [SP-944], Type: Spill, Align: 4, Size: 4
Offset: [SP-948], Type: Spill, Align: 4, Size: 4
Offset: [SP-952], Type: Spill, Align: 4, Size: 4
Offset: [SP-956], Type: Spill, Align: 4, Size: 4
Offset: [SP-960], Type: Spill, Align: 4, Size: 4
Offset: [SP-964], Type: Spill, Align: 4, Size: 4
Offset: [SP-968], Type: Spill, Align: 4, Size: 4 [-Rpass-analysis=stack-frame-layout]
362 | {
| ^

---
Nathan Chancellor (7):
bcachefs: Fix -Wformat in bch2_set_bucket_needs_journal_commit()
bcachefs: Fix -Wformat in bch2_btree_key_cache_to_text()
bcachefs: Fix -Wformat in bch2_alloc_v4_invalid()
bcachefs: Fix -Wformat in bch2_bucket_gens_invalid()
bcachefs: Fix -Wincompatible-function-pointer-types-strict from key_invalid callbacks
bcachefs: Fix -Wcompare-distinct-pointer-types in do_encrypt()
bcachefs: Fix -Wcompare-distinct-pointer-types in bch2_copygc_get_buckets()

fs/bcachefs/alloc_background.c | 4 ++--
fs/bcachefs/bkey_methods.c | 10 +++++-----
fs/bcachefs/btree_key_cache.c | 2 +-
fs/bcachefs/buckets_waiting_for_journal.c | 2 +-
fs/bcachefs/checksum.c | 2 +-
fs/bcachefs/movinggc.c | 2 +-
fs/bcachefs/subvolume.c | 2 +-
fs/bcachefs/subvolume.h | 2 +-
8 files changed, 13 insertions(+), 13 deletions(-)
---
base-commit: e7e6c4189f70ab2d7c21eaec5b9e9c34527ef349
change-id: 20230912-bcachefs-warning-fixes-f136f6d33e71

Best regards,
--
Nathan Chancellor <[email protected]>


2023-09-12 19:19:40

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 2/7] bcachefs: Fix -Wformat in bch2_btree_key_cache_to_text()

When building bcachefs for 32-bit ARM, there is a compiler warning in
bch2_btree_key_cache_to_text() due to use of an incorrect format
specifier:

fs/bcachefs/btree_key_cache.c:1060:36: error: format specifies type 'size_t' (aka 'unsigned int') but the argument has type 'long' [-Werror,-Wformat]
1060 | prt_printf(out, "nr_freed:\t%zu", atomic_long_read(&c->nr_freed));
| ~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| %ld
fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf'
223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__)
| ^~~~~~~~~~~
1 error generated.

On 64-bit architectures, size_t is 'unsigned long', so there is no
warning when using %zu but on 32-bit architectures, size_t is
'unsigned int'. Use '%lu' to match the other format specifiers used in
this function for printing values returned from atomic_long_read().

Fixes: 6d799930ce0f ("bcachefs: btree key cache pcpu freedlist")
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/bcachefs/btree_key_cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
index 505e7c365ab7..a74ee6d8a7cf 100644
--- a/fs/bcachefs/btree_key_cache.c
+++ b/fs/bcachefs/btree_key_cache.c
@@ -1053,7 +1053,7 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)

void bch2_btree_key_cache_to_text(struct printbuf *out, struct btree_key_cache *c)
{
- prt_printf(out, "nr_freed:\t%zu", atomic_long_read(&c->nr_freed));
+ prt_printf(out, "nr_freed:\t%lu", atomic_long_read(&c->nr_freed));
prt_newline(out);
prt_printf(out, "nr_keys:\t%lu", atomic_long_read(&c->nr_keys));
prt_newline(out);

--
2.42.0

2023-09-12 19:20:27

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 1/7] bcachefs: Fix -Wformat in bch2_set_bucket_needs_journal_commit()

When building bcachefs for 32-bit ARM, there is a compiler warning in
bch2_set_bucket_needs_journal_commit() due to a debug print using the
wrong specifier:

fs/bcachefs/buckets_waiting_for_journal.c:137:30: error: format specifies type 'size_t' (aka 'unsigned int') but the argument has type 'unsigned long' [-Werror,-Wformat]
136 | pr_debug("took %zu rehashes, table at %zu/%zu elements",
| ~~~
| %lu
137 | nr_rehashes, nr_elements, 1UL << b->t->bits);
| ^~~~~~~~~~~~~~~~~
include/linux/printk.h:579:26: note: expanded from macro 'pr_debug'
579 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dynamic_debug.h:270:22: note: expanded from macro 'dynamic_pr_debug'
270 | pr_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dynamic_debug.h:250:59: note: expanded from macro '_dynamic_func_call'
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:248:65: note: expanded from macro '_dynamic_func_call_cls'
248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:224:15: note: expanded from macro '__dynamic_func_call_cls'
224 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
1 error generated.

On 64-bit architectures, size_t is 'unsigned long', so there is no
warning when using %zu but on 32-bit architectures, size_t is
'unsigned int'. Use the correct specifier to resolve the warning.

Fixes: 7a82e75ddaef ("bcachefs: New data structure for buckets waiting on journal commit")
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/bcachefs/buckets_waiting_for_journal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bcachefs/buckets_waiting_for_journal.c b/fs/bcachefs/buckets_waiting_for_journal.c
index 81ab685cdef9..ec1b636ef78d 100644
--- a/fs/bcachefs/buckets_waiting_for_journal.c
+++ b/fs/bcachefs/buckets_waiting_for_journal.c
@@ -133,7 +133,7 @@ int bch2_set_bucket_needs_journal_commit(struct buckets_waiting_for_journal *b,
b->t = n;
kvfree(t);

- pr_debug("took %zu rehashes, table at %zu/%zu elements",
+ pr_debug("took %zu rehashes, table at %zu/%lu elements",
nr_rehashes, nr_elements, 1UL << b->t->bits);
out:
mutex_unlock(&b->lock);

--
2.42.0

2023-09-12 19:20:28

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 4/7] bcachefs: Fix -Wformat in bch2_bucket_gens_invalid()

When building bcachefs for 32-bit ARM, there is a compiler warning in
bch2_bucket_gens_invalid() due to use of an incorrect format specifier:

fs/bcachefs/alloc_background.c:530:10: error: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
529 | prt_printf(err, "bad val size (%lu != %zu)",
| ~~~
| %zu
530 | bkey_val_bytes(k.k), sizeof(struct bch_bucket_gens));
| ^~~~~~~~~~~~~~~~~~~
fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf'
223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__)
| ^~~~~~~~~~~

On 64-bit architectures, size_t is 'unsigned long', so there is no
warning when using %lu but on 32-bit architectures, size_t is 'unsigned
int'. Use '%zu', the format specifier for 'size_t', to eliminate the
warning.

Fixes: 4be0d766a7e9 ("bcachefs: bucket_gens btree")
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/bcachefs/alloc_background.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bcachefs/alloc_background.c b/fs/bcachefs/alloc_background.c
index 67e73864823c..2b0d155d1ed5 100644
--- a/fs/bcachefs/alloc_background.c
+++ b/fs/bcachefs/alloc_background.c
@@ -526,7 +526,7 @@ int bch2_bucket_gens_invalid(const struct bch_fs *c, struct bkey_s_c k,
struct printbuf *err)
{
if (bkey_val_bytes(k.k) != sizeof(struct bch_bucket_gens)) {
- prt_printf(err, "bad val size (%lu != %zu)",
+ prt_printf(err, "bad val size (%zu != %zu)",
bkey_val_bytes(k.k), sizeof(struct bch_bucket_gens));
return -BCH_ERR_invalid_bkey;
}

--
2.42.0

2023-09-12 19:26:15

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 6/7] bcachefs: Fix -Wcompare-distinct-pointer-types in do_encrypt()

When building bcachefs for 32-bit ARM, there is a warning when using
min() to compare a variable of type 'size_t' with an expression of type
'unsigned long':

fs/bcachefs/checksum.c:142:22: error: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (((1UL) << 12) - offset) *' (aka 'unsigned long *')) [-Werror,-Wcompare-distinct-pointer-types]
142 | unsigned pg_len = min(len, PAGE_SIZE - offset);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:69:19: note: expanded from macro 'min'
69 | #define min(x, y) __careful_cmp(x, y, <)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:38:24: note: expanded from macro '__careful_cmp'
38 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~~~~~~~
include/linux/minmax.h:28:4: note: expanded from macro '__safe_cmp'
28 | (__typecheck(x, y) && __no_side_effects(x, y))
| ^~~~~~~~~~~~~~~~~
include/linux/minmax.h:22:28: note: expanded from macro '__typecheck'
22 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
1 error generated.

On 64-bit architectures, size_t is 'unsigned long', so there is no
warning when comparing these two expressions. Use min_t(size_t, ...) for
this situation, eliminating the warning.

Fixes: 1fb50457684f ("bcachefs: Fix memory corruption in encryption path")
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/bcachefs/checksum.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bcachefs/checksum.c b/fs/bcachefs/checksum.c
index 36939020f67d..ff0c3cd39ee2 100644
--- a/fs/bcachefs/checksum.c
+++ b/fs/bcachefs/checksum.c
@@ -139,7 +139,7 @@ static inline int do_encrypt(struct crypto_sync_skcipher *tfm,

for (i = 0; i < pages; i++) {
unsigned offset = offset_in_page(buf);
- unsigned pg_len = min(len, PAGE_SIZE - offset);
+ unsigned pg_len = min_t(size_t, len, PAGE_SIZE - offset);

sg_set_page(sg + i, vmalloc_to_page(buf), pg_len, offset);
buf += pg_len;

--
2.42.0

2023-09-13 01:45:00

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 5/7] bcachefs: Fix -Wincompatible-function-pointer-types-strict from key_invalid callbacks

When building bcachefs with -Wincompatible-function-pointer-types-strict,
a clang warning designed to catch issues with mismatched function
pointer types, which will be fatal at runtime due to kernel Control Flow
Integrity (kCFI), there are several instances along the lines of:

fs/bcachefs/bkey_methods.c:118:2: error: incompatible function pointer types initializing 'int (*)(const struct bch_fs *, struct bkey_s_c, enum bkey_invalid_flags, struct printbuf *)' with an expression of type 'int (const struct bch_fs *, struct bkey_s_c, unsigned int, struct printbuf *)' [-Werror,-Wincompatible-function-pointer-types-strict]
118 | BCH_BKEY_TYPES()
| ^~~~~~~~~~~~~~~~
fs/bcachefs/bcachefs_format.h:342:2: note: expanded from macro 'BCH_BKEY_TYPES'
342 | x(deleted, 0) \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/bcachefs/bkey_methods.c:117:41: note: expanded from macro 'x'
117 | #define x(name, nr) [KEY_TYPE_##name] = bch2_bkey_ops_##name,
| ^~~~~~~~~~~~~~~~~~~~
<scratch space>:206:1: note: expanded from here
206 | bch2_bkey_ops_deleted
| ^~~~~~~~~~~~~~~~~~~~~
fs/bcachefs/bkey_methods.c:34:17: note: expanded from macro 'bch2_bkey_ops_deleted'
34 | .key_invalid = deleted_key_invalid, \
| ^~~~~~~~~~~~~~~~~~~

The flags parameter should be of type 'enum bkey_invalid_flags', not
'unsigned int'. Adjust the type everywhere so that there is no more
warning.

Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/bcachefs/bkey_methods.c | 10 +++++-----
fs/bcachefs/subvolume.c | 2 +-
fs/bcachefs/subvolume.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/bcachefs/bkey_methods.c b/fs/bcachefs/bkey_methods.c
index 6547142db428..2a284c954c7a 100644
--- a/fs/bcachefs/bkey_methods.c
+++ b/fs/bcachefs/bkey_methods.c
@@ -25,7 +25,7 @@ const char * const bch2_bkey_types[] = {
};

static int deleted_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
- unsigned flags, struct printbuf *err)
+ enum bkey_invalid_flags flags, struct printbuf *err)
{
return 0;
}
@@ -39,7 +39,7 @@ static int deleted_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
})

static int empty_val_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
- unsigned flags, struct printbuf *err)
+ enum bkey_invalid_flags flags, struct printbuf *err)
{
if (bkey_val_bytes(k.k)) {
prt_printf(err, "incorrect value size (%zu != 0)",
@@ -55,7 +55,7 @@ static int empty_val_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
})

static int key_type_cookie_invalid(const struct bch_fs *c, struct bkey_s_c k,
- unsigned flags, struct printbuf *err)
+ enum bkey_invalid_flags flags, struct printbuf *err)
{
return 0;
}
@@ -70,7 +70,7 @@ static int key_type_cookie_invalid(const struct bch_fs *c, struct bkey_s_c k,
})

static int key_type_inline_data_invalid(const struct bch_fs *c, struct bkey_s_c k,
- unsigned flags, struct printbuf *err)
+ enum bkey_invalid_flags flags, struct printbuf *err)
{
return 0;
}
@@ -91,7 +91,7 @@ static void key_type_inline_data_to_text(struct printbuf *out, struct bch_fs *c,
})

static int key_type_set_invalid(const struct bch_fs *c, struct bkey_s_c k,
- unsigned flags, struct printbuf *err)
+ enum bkey_invalid_flags flags, struct printbuf *err)
{
if (bkey_val_bytes(k.k)) {
prt_printf(err, "incorrect value size (%zu != %zu)",
diff --git a/fs/bcachefs/subvolume.c b/fs/bcachefs/subvolume.c
index 0214a98deb4f..b371a5c4e06e 100644
--- a/fs/bcachefs/subvolume.c
+++ b/fs/bcachefs/subvolume.c
@@ -99,7 +99,7 @@ int bch2_check_subvols(struct bch_fs *c)
/* Subvolumes: */

int bch2_subvolume_invalid(const struct bch_fs *c, struct bkey_s_c k,
- unsigned flags, struct printbuf *err)
+ enum bkey_invalid_flags flags, struct printbuf *err)
{
if (bkey_lt(k.k->p, SUBVOL_POS_MIN) ||
bkey_gt(k.k->p, SUBVOL_POS_MAX)) {
diff --git a/fs/bcachefs/subvolume.h b/fs/bcachefs/subvolume.h
index 8d4c50f4cd05..bb14f92e8687 100644
--- a/fs/bcachefs/subvolume.h
+++ b/fs/bcachefs/subvolume.h
@@ -10,7 +10,7 @@ enum bkey_invalid_flags;
int bch2_check_subvols(struct bch_fs *);

int bch2_subvolume_invalid(const struct bch_fs *, struct bkey_s_c,
- unsigned, struct printbuf *);
+ enum bkey_invalid_flags, struct printbuf *);
void bch2_subvolume_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);

#define bch2_bkey_ops_subvolume ((struct bkey_ops) { \

--
2.42.0

2023-09-13 02:05:12

by Kent Overstreet

[permalink] [raw]
Subject: Re: [PATCH 0/7] bcachefs compiler warning fixes for 32-bit

On Tue, Sep 12, 2023 at 12:15:37PM -0700, Nathan Chancellor wrote:
> Hi all,
>
> This is a series of fixes for warnings that I now see from bcachefs when
> building my test matrix with LLVM in -next, mostly from 32-bit
> architectures. Most of the patches should be uncontroversial; the
> min_t/max_t changes are probably the worst ones.

Those patches all look fine, thanks

> I still see several instances of -Wframe-larger-than when building for
> 32-bit ARM (I am sure they will show up on other 32-bit architectures as
> well), which I am not entirely sure how to tackle. It looks like the
> majority of the instances are just due to large structures on the stack
> in combination with some inlining resulting in some spills, so it seems
> like moving to a heap allocation for some of those would the right fix
> but I know some maintainers would rather fix them in their own way,
> hence just the report.

I just started seeing these again as well. The biggest single object
that most of them have on their stack is btree_trans, but initializing
that has to do a heap allocation for the btree_paths array - so we might
as well switch to heap allocating the entire thing.

That will help.

2023-09-13 12:46:53

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 7/7] bcachefs: Fix -Wcompare-distinct-pointer-types in bch2_copygc_get_buckets()

When building bcachefs for 32-bit ARM, there is a warning when using
max() to compare an expression involving 'size_t' with an 'unsigned
long' literal:

fs/bcachefs/movinggc.c:159:21: error: comparison of distinct pointer types ('typeof (16UL) *' (aka 'unsigned long *') and 'typeof (buckets_in_flight->nr / 4) *' (aka 'unsigned int *')) [-Werror,-Wcompare-distinct-pointer-types]
159 | size_t nr_to_get = max(16UL, buckets_in_flight->nr / 4);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:76:19: note: expanded from macro 'max'
76 | #define max(x, y) __careful_cmp(x, y, >)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:38:24: note: expanded from macro '__careful_cmp'
38 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~~~~~~~
include/linux/minmax.h:28:4: note: expanded from macro '__safe_cmp'
28 | (__typecheck(x, y) && __no_side_effects(x, y))
| ^~~~~~~~~~~~~~~~~
include/linux/minmax.h:22:28: note: expanded from macro '__typecheck'
22 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
1 error generated.

On 64-bit architectures, size_t is 'unsigned long', so there is no
warning when comparing these two expressions. Use max_t(size_t, ...) for
this situation, eliminating the warning.

Fixes: dd49018737d4 ("bcachefs: Rhashtable based buckets_in_flight for copygc")
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/bcachefs/movinggc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bcachefs/movinggc.c b/fs/bcachefs/movinggc.c
index 256431a6dc0c..1cb441d90b34 100644
--- a/fs/bcachefs/movinggc.c
+++ b/fs/bcachefs/movinggc.c
@@ -156,7 +156,7 @@ static int bch2_copygc_get_buckets(struct btree_trans *trans,
struct bch_fs *c = trans->c;
struct btree_iter iter;
struct bkey_s_c k;
- size_t nr_to_get = max(16UL, buckets_in_flight->nr / 4);
+ size_t nr_to_get = max_t(size_t, 16U, buckets_in_flight->nr / 4);
size_t saw = 0, in_flight = 0, not_movable = 0, sectors = 0;
int ret;


--
2.42.0

2023-09-13 23:15:10

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH 2/7] bcachefs: Fix -Wformat in bch2_btree_key_cache_to_text()

On Tue, Sep 12, 2023 at 12:15:39PM -0700, Nathan Chancellor wrote:
> When building bcachefs for 32-bit ARM, there is a compiler warning in
> bch2_btree_key_cache_to_text() due to use of an incorrect format
> specifier:
>
> fs/bcachefs/btree_key_cache.c:1060:36: error: format specifies type 'size_t' (aka 'unsigned int') but the argument has type 'long' [-Werror,-Wformat]
> 1060 | prt_printf(out, "nr_freed:\t%zu", atomic_long_read(&c->nr_freed));
> | ~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | %ld
> fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf'
> 223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__)
> | ^~~~~~~~~~~
> 1 error generated.
>
> On 64-bit architectures, size_t is 'unsigned long', so there is no
> warning when using %zu but on 32-bit architectures, size_t is
> 'unsigned int'. Use '%lu' to match the other format specifiers used in
> this function for printing values returned from atomic_long_read().
>
> Fixes: 6d799930ce0f ("bcachefs: btree key cache pcpu freedlist")
> Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Justin Stitt <[email protected]>
> ---
> fs/bcachefs/btree_key_cache.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
> index 505e7c365ab7..a74ee6d8a7cf 100644
> --- a/fs/bcachefs/btree_key_cache.c
> +++ b/fs/bcachefs/btree_key_cache.c
> @@ -1053,7 +1053,7 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
>
> void bch2_btree_key_cache_to_text(struct printbuf *out, struct btree_key_cache *c)
> {
> - prt_printf(out, "nr_freed:\t%zu", atomic_long_read(&c->nr_freed));
> + prt_printf(out, "nr_freed:\t%lu", atomic_long_read(&c->nr_freed));
> prt_newline(out);
> prt_printf(out, "nr_keys:\t%lu", atomic_long_read(&c->nr_keys));
> prt_newline(out);
>
> --
> 2.42.0
>

2023-09-14 01:16:26

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH 1/7] bcachefs: Fix -Wformat in bch2_set_bucket_needs_journal_commit()

On Tue, Sep 12, 2023 at 12:15:38PM -0700, Nathan Chancellor wrote:
> When building bcachefs for 32-bit ARM, there is a compiler warning in
> bch2_set_bucket_needs_journal_commit() due to a debug print using the
> wrong specifier:
>
> fs/bcachefs/buckets_waiting_for_journal.c:137:30: error: format specifies type 'size_t' (aka 'unsigned int') but the argument has type 'unsigned long' [-Werror,-Wformat]
> 136 | pr_debug("took %zu rehashes, table at %zu/%zu elements",
> | ~~~
> | %lu
> 137 | nr_rehashes, nr_elements, 1UL << b->t->bits);
> | ^~~~~~~~~~~~~~~~~
> include/linux/printk.h:579:26: note: expanded from macro 'pr_debug'
> 579 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
> | ~~~ ^~~~~~~~~~~
> include/linux/dynamic_debug.h:270:22: note: expanded from macro 'dynamic_pr_debug'
> 270 | pr_fmt(fmt), ##__VA_ARGS__)
> | ~~~ ^~~~~~~~~~~
> include/linux/dynamic_debug.h:250:59: note: expanded from macro '_dynamic_func_call'
> 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~
> include/linux/dynamic_debug.h:248:65: note: expanded from macro '_dynamic_func_call_cls'
> 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~
> include/linux/dynamic_debug.h:224:15: note: expanded from macro '__dynamic_func_call_cls'
> 224 | func(&id, ##__VA_ARGS__); \
> | ^~~~~~~~~~~
> 1 error generated.
>
> On 64-bit architectures, size_t is 'unsigned long', so there is no
> warning when using %zu but on 32-bit architectures, size_t is
> 'unsigned int'. Use the correct specifier to resolve the warning.
>
> Fixes: 7a82e75ddaef ("bcachefs: New data structure for buckets waiting on journal commit")
> Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Justin Stitt <[email protected]>

> ---
> fs/bcachefs/buckets_waiting_for_journal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/bcachefs/buckets_waiting_for_journal.c b/fs/bcachefs/buckets_waiting_for_journal.c
> index 81ab685cdef9..ec1b636ef78d 100644
> --- a/fs/bcachefs/buckets_waiting_for_journal.c
> +++ b/fs/bcachefs/buckets_waiting_for_journal.c
> @@ -133,7 +133,7 @@ int bch2_set_bucket_needs_journal_commit(struct buckets_waiting_for_journal *b,
> b->t = n;
> kvfree(t);
>
> - pr_debug("took %zu rehashes, table at %zu/%zu elements",
> + pr_debug("took %zu rehashes, table at %zu/%lu elements",
> nr_rehashes, nr_elements, 1UL << b->t->bits);
> out:
> mutex_unlock(&b->lock);
>
> --
> 2.42.0
>

2023-09-14 02:58:56

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH 5/7] bcachefs: Fix -Wincompatible-function-pointer-types-strict from key_invalid callbacks

On Tue, Sep 12, 2023 at 12:15:42PM -0700, Nathan Chancellor wrote:
> When building bcachefs with -Wincompatible-function-pointer-types-strict,
> a clang warning designed to catch issues with mismatched function
> pointer types, which will be fatal at runtime due to kernel Control Flow
> Integrity (kCFI), there are several instances along the lines of:
>
> fs/bcachefs/bkey_methods.c:118:2: error: incompatible function pointer types initializing 'int (*)(const struct bch_fs *, struct bkey_s_c, enum bkey_invalid_flags, struct printbuf *)' with an expression of type 'int (const struct bch_fs *, struct bkey_s_c, unsigned int, struct printbuf *)' [-Werror,-Wincompatible-function-pointer-types-strict]
> 118 | BCH_BKEY_TYPES()
> | ^~~~~~~~~~~~~~~~
> fs/bcachefs/bcachefs_format.h:342:2: note: expanded from macro 'BCH_BKEY_TYPES'
> 342 | x(deleted, 0) \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/bcachefs/bkey_methods.c:117:41: note: expanded from macro 'x'
> 117 | #define x(name, nr) [KEY_TYPE_##name] = bch2_bkey_ops_##name,
> | ^~~~~~~~~~~~~~~~~~~~
> <scratch space>:206:1: note: expanded from here
> 206 | bch2_bkey_ops_deleted
> | ^~~~~~~~~~~~~~~~~~~~~
> fs/bcachefs/bkey_methods.c:34:17: note: expanded from macro 'bch2_bkey_ops_deleted'
> 34 | .key_invalid = deleted_key_invalid, \
> | ^~~~~~~~~~~~~~~~~~~
>
> The flags parameter should be of type 'enum bkey_invalid_flags', not
> 'unsigned int'. Adjust the type everywhere so that there is no more
> warning.
>
> Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Justin Stitt <[email protected]>
> ---
> fs/bcachefs/bkey_methods.c | 10 +++++-----
> fs/bcachefs/subvolume.c | 2 +-
> fs/bcachefs/subvolume.h | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/bcachefs/bkey_methods.c b/fs/bcachefs/bkey_methods.c
> index 6547142db428..2a284c954c7a 100644
> --- a/fs/bcachefs/bkey_methods.c
> +++ b/fs/bcachefs/bkey_methods.c
> @@ -25,7 +25,7 @@ const char * const bch2_bkey_types[] = {
> };
>
> static int deleted_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
> - unsigned flags, struct printbuf *err)
> + enum bkey_invalid_flags flags, struct printbuf *err)
> {
> return 0;
> }
> @@ -39,7 +39,7 @@ static int deleted_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
> })
>
> static int empty_val_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
> - unsigned flags, struct printbuf *err)
> + enum bkey_invalid_flags flags, struct printbuf *err)
> {
> if (bkey_val_bytes(k.k)) {
> prt_printf(err, "incorrect value size (%zu != 0)",
> @@ -55,7 +55,7 @@ static int empty_val_key_invalid(const struct bch_fs *c, struct bkey_s_c k,
> })
>
> static int key_type_cookie_invalid(const struct bch_fs *c, struct bkey_s_c k,
> - unsigned flags, struct printbuf *err)
> + enum bkey_invalid_flags flags, struct printbuf *err)
> {
> return 0;
> }
> @@ -70,7 +70,7 @@ static int key_type_cookie_invalid(const struct bch_fs *c, struct bkey_s_c k,
> })
>
> static int key_type_inline_data_invalid(const struct bch_fs *c, struct bkey_s_c k,
> - unsigned flags, struct printbuf *err)
> + enum bkey_invalid_flags flags, struct printbuf *err)
> {
> return 0;
> }
> @@ -91,7 +91,7 @@ static void key_type_inline_data_to_text(struct printbuf *out, struct bch_fs *c,
> })
>
> static int key_type_set_invalid(const struct bch_fs *c, struct bkey_s_c k,
> - unsigned flags, struct printbuf *err)
> + enum bkey_invalid_flags flags, struct printbuf *err)
> {
> if (bkey_val_bytes(k.k)) {
> prt_printf(err, "incorrect value size (%zu != %zu)",
> diff --git a/fs/bcachefs/subvolume.c b/fs/bcachefs/subvolume.c
> index 0214a98deb4f..b371a5c4e06e 100644
> --- a/fs/bcachefs/subvolume.c
> +++ b/fs/bcachefs/subvolume.c
> @@ -99,7 +99,7 @@ int bch2_check_subvols(struct bch_fs *c)
> /* Subvolumes: */
>
> int bch2_subvolume_invalid(const struct bch_fs *c, struct bkey_s_c k,
> - unsigned flags, struct printbuf *err)
> + enum bkey_invalid_flags flags, struct printbuf *err)
> {
> if (bkey_lt(k.k->p, SUBVOL_POS_MIN) ||
> bkey_gt(k.k->p, SUBVOL_POS_MAX)) {
> diff --git a/fs/bcachefs/subvolume.h b/fs/bcachefs/subvolume.h
> index 8d4c50f4cd05..bb14f92e8687 100644
> --- a/fs/bcachefs/subvolume.h
> +++ b/fs/bcachefs/subvolume.h
> @@ -10,7 +10,7 @@ enum bkey_invalid_flags;
> int bch2_check_subvols(struct bch_fs *);
>
> int bch2_subvolume_invalid(const struct bch_fs *, struct bkey_s_c,
> - unsigned, struct printbuf *);
> + enum bkey_invalid_flags, struct printbuf *);
> void bch2_subvolume_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
>
> #define bch2_bkey_ops_subvolume ((struct bkey_ops) { \
>
> --
> 2.42.0
>

2023-09-14 05:54:12

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH 6/7] bcachefs: Fix -Wcompare-distinct-pointer-types in do_encrypt()

On Tue, Sep 12, 2023 at 12:15:43PM -0700, Nathan Chancellor wrote:
> When building bcachefs for 32-bit ARM, there is a warning when using
> min() to compare a variable of type 'size_t' with an expression of type
> 'unsigned long':
>
> fs/bcachefs/checksum.c:142:22: error: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (((1UL) << 12) - offset) *' (aka 'unsigned long *')) [-Werror,-Wcompare-distinct-pointer-types]
> 142 | unsigned pg_len = min(len, PAGE_SIZE - offset);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/minmax.h:69:19: note: expanded from macro 'min'
> 69 | #define min(x, y) __careful_cmp(x, y, <)
> | ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/minmax.h:38:24: note: expanded from macro '__careful_cmp'
> 38 | __builtin_choose_expr(__safe_cmp(x, y), \
> | ^~~~~~~~~~~~~~~~
> include/linux/minmax.h:28:4: note: expanded from macro '__safe_cmp'
> 28 | (__typecheck(x, y) && __no_side_effects(x, y))
> | ^~~~~~~~~~~~~~~~~
> include/linux/minmax.h:22:28: note: expanded from macro '__typecheck'
> 22 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> | ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
> 1 error generated.
>
> On 64-bit architectures, size_t is 'unsigned long', so there is no
> warning when comparing these two expressions. Use min_t(size_t, ...) for
> this situation, eliminating the warning.
>
> Fixes: 1fb50457684f ("bcachefs: Fix memory corruption in encryption path")
> Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Justin Stitt <[email protected]>

> ---
> fs/bcachefs/checksum.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/bcachefs/checksum.c b/fs/bcachefs/checksum.c
> index 36939020f67d..ff0c3cd39ee2 100644
> --- a/fs/bcachefs/checksum.c
> +++ b/fs/bcachefs/checksum.c
> @@ -139,7 +139,7 @@ static inline int do_encrypt(struct crypto_sync_skcipher *tfm,
>
> for (i = 0; i < pages; i++) {
> unsigned offset = offset_in_page(buf);
> - unsigned pg_len = min(len, PAGE_SIZE - offset);
> + unsigned pg_len = min_t(size_t, len, PAGE_SIZE - offset);
>
> sg_set_page(sg + i, vmalloc_to_page(buf), pg_len, offset);
> buf += pg_len;
>
> --
> 2.42.0
>

2023-09-14 12:22:43

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH 7/7] bcachefs: Fix -Wcompare-distinct-pointer-types in bch2_copygc_get_buckets()

On Tue, Sep 12, 2023 at 12:15:44PM -0700, Nathan Chancellor wrote:
> When building bcachefs for 32-bit ARM, there is a warning when using
> max() to compare an expression involving 'size_t' with an 'unsigned
> long' literal:
>
> fs/bcachefs/movinggc.c:159:21: error: comparison of distinct pointer types ('typeof (16UL) *' (aka 'unsigned long *') and 'typeof (buckets_in_flight->nr / 4) *' (aka 'unsigned int *')) [-Werror,-Wcompare-distinct-pointer-types]
> 159 | size_t nr_to_get = max(16UL, buckets_in_flight->nr / 4);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/minmax.h:76:19: note: expanded from macro 'max'
> 76 | #define max(x, y) __careful_cmp(x, y, >)
> | ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/minmax.h:38:24: note: expanded from macro '__careful_cmp'
> 38 | __builtin_choose_expr(__safe_cmp(x, y), \
> | ^~~~~~~~~~~~~~~~
> include/linux/minmax.h:28:4: note: expanded from macro '__safe_cmp'
> 28 | (__typecheck(x, y) && __no_side_effects(x, y))
> | ^~~~~~~~~~~~~~~~~
> include/linux/minmax.h:22:28: note: expanded from macro '__typecheck'
> 22 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> | ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
> 1 error generated.
>
> On 64-bit architectures, size_t is 'unsigned long', so there is no
> warning when comparing these two expressions. Use max_t(size_t, ...) for
> this situation, eliminating the warning.
>
> Fixes: dd49018737d4 ("bcachefs: Rhashtable based buckets_in_flight for copygc")
> Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Justin Stitt <[email protected]>
> ---
> fs/bcachefs/movinggc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/bcachefs/movinggc.c b/fs/bcachefs/movinggc.c
> index 256431a6dc0c..1cb441d90b34 100644
> --- a/fs/bcachefs/movinggc.c
> +++ b/fs/bcachefs/movinggc.c
> @@ -156,7 +156,7 @@ static int bch2_copygc_get_buckets(struct btree_trans *trans,
> struct bch_fs *c = trans->c;
> struct btree_iter iter;
> struct bkey_s_c k;
> - size_t nr_to_get = max(16UL, buckets_in_flight->nr / 4);
> + size_t nr_to_get = max_t(size_t, 16U, buckets_in_flight->nr / 4);
> size_t saw = 0, in_flight = 0, not_movable = 0, sectors = 0;
> int ret;
>
>
> --
> 2.42.0
>

2023-09-15 08:08:09

by Justin Stitt

[permalink] [raw]
Subject: Re: [PATCH 4/7] bcachefs: Fix -Wformat in bch2_bucket_gens_invalid()

On Tue, Sep 12, 2023 at 12:15:41PM -0700, Nathan Chancellor wrote:
> When building bcachefs for 32-bit ARM, there is a compiler warning in
> bch2_bucket_gens_invalid() due to use of an incorrect format specifier:
>
> fs/bcachefs/alloc_background.c:530:10: error: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
> 529 | prt_printf(err, "bad val size (%lu != %zu)",
> | ~~~
> | %zu
> 530 | bkey_val_bytes(k.k), sizeof(struct bch_bucket_gens));
> | ^~~~~~~~~~~~~~~~~~~
> fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf'
> 223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__)
> | ^~~~~~~~~~~
>
> On 64-bit architectures, size_t is 'unsigned long', so there is no
> warning when using %lu but on 32-bit architectures, size_t is 'unsigned
> int'. Use '%zu', the format specifier for 'size_t', to eliminate the
> warning.
>
> Fixes: 4be0d766a7e9 ("bcachefs: bucket_gens btree")
> Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Justin Stitt <[email protected]>

> ---
> fs/bcachefs/alloc_background.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/bcachefs/alloc_background.c b/fs/bcachefs/alloc_background.c
> index 67e73864823c..2b0d155d1ed5 100644
> --- a/fs/bcachefs/alloc_background.c
> +++ b/fs/bcachefs/alloc_background.c
> @@ -526,7 +526,7 @@ int bch2_bucket_gens_invalid(const struct bch_fs *c, struct bkey_s_c k,
> struct printbuf *err)
> {
> if (bkey_val_bytes(k.k) != sizeof(struct bch_bucket_gens)) {
> - prt_printf(err, "bad val size (%lu != %zu)",
> + prt_printf(err, "bad val size (%zu != %zu)",
> bkey_val_bytes(k.k), sizeof(struct bch_bucket_gens));
> return -BCH_ERR_invalid_bkey;
> }
>
> --
> 2.42.0
>