| Message ID | cover.1671221596.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fixup uninitialized warnings and enable extra checks | expand |
On 2022/12/17 04:15, Josef Bacik wrote: > Hello, > > We had been failing the raid56 related scrub tests on our overnight tests since > November. Initially I asked Qu to look into these as I didn't have time to dig > in, and he was unable to reproduce. I assumed it was some oddity in my setup, > so I ignored it. However recently I got a report that I regressed some of these > tests with an unrelated change. When debugging it I found it was because of an > uninitialized return value, which would have been caught by more modern gcc's > with -Wmaybe-uninitialized. Any clue which patch is fixing the raid0/raid1 scrub failures? As locally, I found my aarch64/x86_64 VMs are all reporting scrub errors for all profiles, including RAID0/RAID1. (The failure happens after patch "btrfs: do not check header generation in btrfs_clean_tree_block"). I didn't notice any of the patches touching the scrub path, or is there some hidden paths involved? Thanks, Qu > > In order to avoid these sort of problems in the future lets fix up all the false > positivies that this warning brings, and then enable the option for btrfs so we > can avoid this style of failure in the future. Thanks, > > Josef > > Josef Bacik (8): > btrfs: fix uninit warning in run_one_async_start > btrfs: fix uninit warning in btrfs_cleanup_ordered_extents > btrfs: fix uninit warning from get_inode_gen usage > btrfs: fix uninit warning in btrfs_update_block_group > btrfs: fix uninit warning in __set_extent_bit and convert_extent_bit > btrfs: extract out zone cache usage into it's own helper > btrfs: fix uninit warning in btrfs_sb_log_location > btrfs: turn on -Wmaybe-uninitialized > > fs/btrfs/Makefile | 1 + > fs/btrfs/block-group.c | 2 +- > fs/btrfs/disk-io.c | 2 +- > fs/btrfs/extent-io-tree.c | 8 ++--- > fs/btrfs/inode.c | 2 +- > fs/btrfs/send.c | 8 ++--- > fs/btrfs/zoned.c | 75 ++++++++++++++++++++++++--------------- > 7 files changed, 57 insertions(+), 41 deletions(-) >
On 2022/12/17 07:55, Qu Wenruo wrote: > > > On 2022/12/17 04:15, Josef Bacik wrote: >> Hello, >> >> We had been failing the raid56 related scrub tests on our overnight >> tests since >> November. Initially I asked Qu to look into these as I didn't have >> time to dig >> in, and he was unable to reproduce. I assumed it was some oddity in >> my setup, >> so I ignored it. However recently I got a report that I regressed >> some of these >> tests with an unrelated change. When debugging it I found it was >> because of an >> uninitialized return value, which would have been caught by more >> modern gcc's >> with -Wmaybe-uninitialized. > > Any clue which patch is fixing the raid0/raid1 scrub failures? > > As locally, I found my aarch64/x86_64 VMs are all reporting scrub errors > for all profiles, including RAID0/RAID1. > (The failure happens after patch "btrfs: do not check header generation > in btrfs_clean_tree_block"). > > I didn't notice any of the patches touching the scrub path, or is there > some hidden paths involved? In fact, it's still reproducible reliably here, with all uninitialized fixes applied upon "btrfs: do not check header generation in btrfs_clean_tree_block". btrfs/072 30s ... - output mismatch (see /home/adam/xfstests/results//btrfs/072.out.bad) --- tests/btrfs/072.out 2022-05-11 09:55:30.736666664 +0800 +++ /home/adam/xfstests/results//btrfs/072.out.bad 2022-12-17 08:05:26.750000015 +0800 @@ -1,2 +1,9 @@ QA output created by 072 Silence is golden +Scrub find errors in "-m dup -d single" test +Scrub find errors in "-m raid0 -d raid0" test +Scrub find errors in "-m raid0 -d raid0" test +Scrub find errors in "-m raid1 -d raid0" test +Scrub find errors in "-m raid10 -d raid10" test ... (Run 'diff -u /home/adam/xfstests/tests/btrfs/072.out /home/adam/xfstests/results//btrfs/072.out.bad' to see the entire diff) btrfs/074 32s ... - output mismatch (see /home/adam/xfstests/results//btrfs/074.out.bad) --- tests/btrfs/074.out 2022-05-11 09:55:30.736666664 +0800 +++ /home/adam/xfstests/results//btrfs/074.out.bad 2022-12-17 08:06:00.036666681 +0800 @@ -1,2 +1,3 @@ QA output created by 074 Silence is golden +Scrub find errors in "-m raid1 -d raid1" test ... (Run 'diff -u /home/adam/xfstests/tests/btrfs/074.out /home/adam/xfstests/results//btrfs/074.out.bad' to see the entire diff) Ran: btrfs/072 btrfs/074 Failures: btrfs/072 btrfs/074 Thanks, Qu > > Thanks, > Qu > >> >> In order to avoid these sort of problems in the future lets fix up all >> the false >> positivies that this warning brings, and then enable the option for >> btrfs so we >> can avoid this style of failure in the future. Thanks, >> >> Josef >> >> Josef Bacik (8): >> btrfs: fix uninit warning in run_one_async_start >> btrfs: fix uninit warning in btrfs_cleanup_ordered_extents >> btrfs: fix uninit warning from get_inode_gen usage >> btrfs: fix uninit warning in btrfs_update_block_group >> btrfs: fix uninit warning in __set_extent_bit and convert_extent_bit >> btrfs: extract out zone cache usage into it's own helper >> btrfs: fix uninit warning in btrfs_sb_log_location >> btrfs: turn on -Wmaybe-uninitialized >> >> fs/btrfs/Makefile | 1 + >> fs/btrfs/block-group.c | 2 +- >> fs/btrfs/disk-io.c | 2 +- >> fs/btrfs/extent-io-tree.c | 8 ++--- >> fs/btrfs/inode.c | 2 +- >> fs/btrfs/send.c | 8 ++--- >> fs/btrfs/zoned.c | 75 ++++++++++++++++++++++++--------------- >> 7 files changed, 57 insertions(+), 41 deletions(-) >>
On Fri, Dec 16, 2022 at 03:15:50PM -0500, Josef Bacik wrote: > Hello, > > We had been failing the raid56 related scrub tests on our overnight tests since > November. Initially I asked Qu to look into these as I didn't have time to dig > in, and he was unable to reproduce. I assumed it was some oddity in my setup, > so I ignored it. However recently I got a report that I regressed some of these > tests with an unrelated change. When debugging it I found it was because of an > uninitialized return value, which would have been caught by more modern gcc's > with -Wmaybe-uninitialized. > > In order to avoid these sort of problems in the future lets fix up all the false > positivies that this warning brings, and then enable the option for btrfs so we > can avoid this style of failure in the future. Thanks, > > Josef > > Josef Bacik (8): > btrfs: fix uninit warning in run_one_async_start > btrfs: fix uninit warning in btrfs_cleanup_ordered_extents > btrfs: fix uninit warning from get_inode_gen usage > btrfs: fix uninit warning in btrfs_update_block_group > btrfs: fix uninit warning in __set_extent_bit and convert_extent_bit > btrfs: extract out zone cache usage into it's own helper > btrfs: fix uninit warning in btrfs_sb_log_location > btrfs: turn on -Wmaybe-uninitialized I've applied all except 1, 6 and 8, so there are still 2 reported warnings with -Wmaybe-uninitialized that could be fixed in a slightly different way.
On Tue, Dec 20, 2022 at 08:37:06PM +0100, David Sterba wrote: > On Fri, Dec 16, 2022 at 03:15:50PM -0500, Josef Bacik wrote: > > Hello, > > > > We had been failing the raid56 related scrub tests on our overnight tests since > > November. Initially I asked Qu to look into these as I didn't have time to dig > > in, and he was unable to reproduce. I assumed it was some oddity in my setup, > > so I ignored it. However recently I got a report that I regressed some of these > > tests with an unrelated change. When debugging it I found it was because of an > > uninitialized return value, which would have been caught by more modern gcc's > > with -Wmaybe-uninitialized. > > > > In order to avoid these sort of problems in the future lets fix up all the false > > positivies that this warning brings, and then enable the option for btrfs so we > > can avoid this style of failure in the future. Thanks, > > > > Josef > > > > Josef Bacik (8): > > btrfs: fix uninit warning in run_one_async_start > > btrfs: fix uninit warning in btrfs_cleanup_ordered_extents > > btrfs: fix uninit warning from get_inode_gen usage > > btrfs: fix uninit warning in btrfs_update_block_group > > btrfs: fix uninit warning in __set_extent_bit and convert_extent_bit > > btrfs: extract out zone cache usage into it's own helper > > btrfs: fix uninit warning in btrfs_sb_log_location > > btrfs: turn on -Wmaybe-uninitialized > > I've applied all except 1, 6 and 8, so there are still 2 reported > warnings with -Wmaybe-uninitialized that could be fixed in a slightly > different way. Fixed versions of patches 1 and 6 have been applied so 8 has been applied too.