gcc-8: mm/swapfile.c:863:40: error: array subscript 1 is above array bounds of 'struct list_head[1]' [-Werror=array-bounds]
The x86 builds failed with gcc-8 due to following build warnings / errors on Linux next-20240802 to next-20240819.
x86_64 defconfig gcc-8 build failed x86_64 defconfig gcc-13 build pass
First seen on the next-20240802 tag.
GOOD: next-20240730 BAD: next-20240802
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Build errors: -------- mm/swapfile.c: In function 'scan_swap_map_slots.constprop': mm/swapfile.c:863:40: error: array subscript 1 is above array bounds of 'struct list_head[1]' [-Werror=array-bounds] while (!list_empty(&si->frag_clusters[o])) { ~~~~~~~~~~~~~~~~~^~~ mm/swapfile.c:872:43: error: array subscript 1 is above array bounds of 'struct list_head[1]' [-Werror=array-bounds] while (!list_empty(&si->nonfull_clusters[o])) { ~~~~~~~~~~~~~~~~~~~~^~~ In file included from include/linux/list.h:5, from include/linux/wait.h:7, from include/linux/wait_bit.h:8, from include/linux/fs.h:6, from include/linux/highmem.h:5, from include/linux/bvec.h:10, from include/linux/blk_types.h:10, from include/linux/blkdev.h:9, from mm/swapfile.c:9: include/linux/list.h:612:18: error: array subscript 1 is above array bounds of 'struct list_head[1]' [-Werror=array-bounds] list_entry((ptr)->next, type, member) ^~ include/linux/container_of.h:19:26: note: in definition of macro 'container_of' void *__mptr = (void *)(ptr); \ ^~~ include/linux/list.h:612:2: note: in expansion of macro 'list_entry' list_entry((ptr)->next, type, member) ^~~~~~~~~~ mm/swapfile.c:873:9: note: in expansion of macro 'list_first_entry' ci = list_first_entry(&si->nonfull_clusters[o], ^~~~~~~~~~~~~~~~ include/linux/list.h:612:18: error: array subscript 1 is above array bounds of 'struct list_head[1]' [-Werror=array-bounds] list_entry((ptr)->next, type, member) ^~ include/linux/container_of.h:19:26: note: in definition of macro 'container_of' void *__mptr = (void *)(ptr); \ ^~~ include/linux/list.h:612:2: note: in expansion of macro 'list_entry' list_entry((ptr)->next, type, member) ^~~~~~~~~~ mm/swapfile.c:864:9: note: in expansion of macro 'list_first_entry' ci = list_first_entry(&si->frag_clusters[o], ^~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors
Steps to reproduce: ------- # tuxmake --runtime podman --target-arch x86_64 --toolchain gcc-8 --kconfig x86_64_defconfig
Build log link, ------ - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240819/tes...
metadata: ----- git describe: next-20240802 git repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next git short_log: f524a5e4dfb7 ("Add linux-next specific files for 20240802") config: https://storage.tuxsuite.com/public/linaro/lkft/builds/2k6wLr4UFODrd3snUDTPP... download_url: https://storage.tuxsuite.com/public/linaro/lkft/builds/2k6wLr4UFODrd3snUDTPP... toolchain: gcc-8 arch: x86_64
-- Linaro LKFT https://lkft.linaro.org
On Mon, 19 Aug 2024 at 13:58, Naresh Kamboju naresh.kamboju@linaro.org wrote:
The x86 builds failed with gcc-8 due to following build warnings / errors on Linux next-20240802 to next-20240819.
x86_64 defconfig gcc-8 build failed x86_64 defconfig gcc-13 build pass
First seen on the next-20240802 tag.
GOOD: next-20240730 BAD: next-20240802
Anders bisected this and found the first bad commit id as, aded4352f648 ("mm: swap: separate SSD allocation from scan_swap_map_slots()") first faulty one
- Naresh
On Mon, Aug 19, 2024 at 6:16 PM Naresh Kamboju naresh.kamboju@linaro.org wrote:
On Mon, 19 Aug 2024 at 13:58, Naresh Kamboju naresh.kamboju@linaro.org wrote:
The x86 builds failed with gcc-8 due to following build warnings / errors on Linux next-20240802 to next-20240819.
x86_64 defconfig gcc-8 build failed x86_64 defconfig gcc-13 build pass
First seen on the next-20240802 tag.
GOOD: next-20240730 BAD: next-20240802
Anders bisected this and found the first bad commit id as, aded4352f648 ("mm: swap: separate SSD allocation from scan_swap_map_slots()") first faulty one
- Naresh
Hi Naresh,
Thanks for the report, the problem will occur when CONFIG_THP_SWAP is disabled. Can you try the following patch? I can confirm it's fixed with my test.
On Mon, 19 Aug 2024 at 17:14, Kairui Song ryncsn@gmail.com wrote:
On Mon, Aug 19, 2024 at 6:16 PM Naresh Kamboju naresh.kamboju@linaro.org wrote:
On Mon, 19 Aug 2024 at 13:58, Naresh Kamboju naresh.kamboju@linaro.org wrote:
The x86 builds failed with gcc-8 due to following build warnings / errors on Linux next-20240802 to next-20240819.
x86_64 defconfig gcc-8 build failed x86_64 defconfig gcc-13 build pass
First seen on the next-20240802 tag.
GOOD: next-20240730 BAD: next-20240802
Anders bisected this and found the first bad commit id as, aded4352f648 ("mm: swap: separate SSD allocation from scan_swap_map_slots()") first faulty one
- Naresh
Hi Naresh,
Thanks for the report, the problem will occur when CONFIG_THP_SWAP is disabled. Can you try the following patch? I can confirm it's fixed with my test.
Thanks for the patch. I have tested your patch and it fixes the reported problem.
Tested-by: Linux Kernel Functional Testing lkft@linaro.org
- Naresh
On Mon, 19 Aug 2024 19:44:25 +0800 Kairui Song ryncsn@gmail.com wrote:
--- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -836,7 +836,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si, int o goto done; /* Order 0 stealing from higher order */
- for (int o = 1; o < PMD_ORDER; o++) {
- for (int o = 1; o < SWAP_NR_ORDERS; o++) { /*
- Clusters here have at least one usable slots and can't fail order 0
- allocation, but reclaim may drop si->lock and race with another user.
OK, I got that landed in the right place, but...
The definition of `o' within the for statement isn't typical kernel style - I'm surprised we didn't get a warning for this - maybe things have changed when I wasn't looking.
Also, this code makes no attempt to honor our "The preferred limit on the length of a single line is 80 columns" objective. There's just no reason for comment blocks to violate this.
So Chris, please attend to such things when preparing v6, which I assume is in the works.
On Mon, Aug 19, 2024 at 10:05 PM Andrew Morton akpm@linux-foundation.org wrote:
On Mon, 19 Aug 2024 19:44:25 +0800 Kairui Song ryncsn@gmail.com wrote:
--- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -836,7 +836,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si, int o goto done;
/* Order 0 stealing from higher order */
for (int o = 1; o < PMD_ORDER; o++) {
for (int o = 1; o < SWAP_NR_ORDERS; o++) { /* * Clusters here have at least one usable slots and can't fail order 0 * allocation, but reclaim may drop si->lock and race with another user.OK, I got that landed in the right place, but...
The definition of `o' within the for statement isn't typical kernel style - I'm surprised we didn't get a warning for this - maybe things have changed when I wasn't looking.
Noted.
I did use the checkpatch.pl and fixed all the warnings before I sent the patch out. The checkpatch.pl script did not complain about this. Sure I can stay away from it. BTW, I did a search on the kernel tree: $ rg 'for (int' | wc -l 970 $ It seems pretty common in the kernel tree now.
Also, this code makes no attempt to honor our "The preferred limit on the length of a single line is 80 columns" objective. There's just no reason for comment blocks to violate this.
I was wondering why the checkpatch.pl did not catch this, is there any config for checkpatch.pl I should apply?
I typically invoke:
./scripts/checkpatch.pl -g HEAD
Let me know if there is a better way to invoke checkpatch.pl to give more warnings.
Chris
On Tue, Aug 20, 2024 at 4:51 PM Chris Li chrisl@kernel.org wrote:
On Mon, Aug 19, 2024 at 10:05 PM Andrew Morton akpm@linux-foundation.org wrote:
On Mon, 19 Aug 2024 19:44:25 +0800 Kairui Song ryncsn@gmail.com wrote:
--- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -836,7 +836,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si, int o goto done;
/* Order 0 stealing from higher order */
for (int o = 1; o < PMD_ORDER; o++) {
for (int o = 1; o < SWAP_NR_ORDERS; o++) { /* * Clusters here have at least one usable slots and can't fail order 0 * allocation, but reclaim may drop si->lock and race with another user.OK, I got that landed in the right place, but...
The definition of `o' within the for statement isn't typical kernel style - I'm surprised we didn't get a warning for this - maybe things have changed when I wasn't looking.
Noted.
I did use the checkpatch.pl and fixed all the warnings before I sent the patch out. The checkpatch.pl script did not complain about this. Sure I can stay away from it. BTW, I did a search on the kernel tree: $ rg 'for (int' | wc -l 970 $ It seems pretty common in the kernel tree now.
Might be off topic from the issue...
I believe this issue it's not an upstream problem nowadays after e8c07082a810 ("Kbuild: move to -std=gnu11"), I did notice a GCC error after backporting these commits to an older kernel which still used c89, but for upstream this should be OK?
Also, this code makes no attempt to honor our "The preferred limit on the length of a single line is 80 columns" objective. There's just no reason for comment blocks to violate this.
I was wondering why the checkpatch.pl did not catch this, is there any config for checkpatch.pl I should apply?
I typically invoke:
./scripts/checkpatch.pl -g HEAD
I found checkpatch.pl stopped checking for 80 columns limit after commit bdc48fa11e46 ("checkpatch/coding-style: deprecate 80-column warning") 4 years ago. But the 80 column limit seems still preferred?
-
Andrew Morton -
Chris Li -
Kairui Song -
Naresh Kamboju