| Message ID | 20200321010303.124708-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: check/lowmem: Fix a false alert caused by hole beyond isize check | expand |
On Sat, Mar 21, 2020 at 09:03:03AM +0800, Qu Wenruo wrote: > Commit 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of > holes") makes lowmem mode check to skip hole detection after isize. > > However it also skipped the extent end update if the extent ends just at > isize. > > This caused fsck-test/001 to fail with false hole error report. > > Fix it by updating the @end parameter if we have an extent ends at inode > size. > > Fixes: 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of holes") > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > David, please fold the fix into the original patch. Folded, thanks for the fix. The lowmem mode tests still don't pass for me, have been failing since 5.1. I've now added a make target for it so it's easier to run them without setting up the env variables.
On 2020/3/24 上午3:38, David Sterba wrote: > On Sat, Mar 21, 2020 at 09:03:03AM +0800, Qu Wenruo wrote: >> Commit 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of >> holes") makes lowmem mode check to skip hole detection after isize. >> >> However it also skipped the extent end update if the extent ends just at >> isize. >> >> This caused fsck-test/001 to fail with false hole error report. >> >> Fix it by updating the @end parameter if we have an extent ends at inode >> size. >> >> Fixes: 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of holes") >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> David, please fold the fix into the original patch. > > Folded, thanks for the fix. The lowmem mode tests still don't pass for > me, have been failing since 5.1. I've now added a make target for it so > it's easier to run them without setting up the env variables. > Mind to provide the fsck-test-result.txt for me to further investigate the problem? Thanks, Qu
On 2020/3/24 上午7:15, Qu Wenruo wrote: > > > On 2020/3/24 上午3:38, David Sterba wrote: >> On Sat, Mar 21, 2020 at 09:03:03AM +0800, Qu Wenruo wrote: >>> Commit 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of >>> holes") makes lowmem mode check to skip hole detection after isize. >>> >>> However it also skipped the extent end update if the extent ends just at >>> isize. >>> >>> This caused fsck-test/001 to fail with false hole error report. >>> >>> Fix it by updating the @end parameter if we have an extent ends at inode >>> size. >>> >>> Fixes: 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of holes") >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> David, please fold the fix into the original patch. >> >> Folded, thanks for the fix. The lowmem mode tests still don't pass for >> me, have been failing since 5.1. I've now added a make target for it so >> it's easier to run them without setting up the env variables. >> > > Mind to provide the fsck-test-result.txt for me to further investigate > the problem? My bad, I didn't catch the problem in the pull request, and my test environments all failed to catch the problem. The lack of diversity makes it pretty hard to detect it, as it looks like all machines tends to zero the unintialized stack structure. Anyway, the proper fix for that long existing v5.1 bug is here: https://patchwork.kernel.org/patch/11454395/ And I'll look into the possibility to auto use valgrind in selftests to avoid similar bug to sneak in. Thanks, Qu > > Thanks, > Qu >
On Tue, Mar 24, 2020 at 03:07:28PM +0800, Qu Wenruo wrote: > On 2020/3/24 上午7:15, Qu Wenruo wrote: > > On 2020/3/24 上午3:38, David Sterba wrote: > >> On Sat, Mar 21, 2020 at 09:03:03AM +0800, Qu Wenruo wrote: > >>> Commit 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of > >>> holes") makes lowmem mode check to skip hole detection after isize. > >>> > >>> However it also skipped the extent end update if the extent ends just at > >>> isize. > >>> > >>> This caused fsck-test/001 to fail with false hole error report. > >>> > >>> Fix it by updating the @end parameter if we have an extent ends at inode > >>> size. > >>> > >>> Fixes: 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of holes") > >>> Signed-off-by: Qu Wenruo <wqu@suse.com> > >>> --- > >>> David, please fold the fix into the original patch. > >> > >> Folded, thanks for the fix. The lowmem mode tests still don't pass for > >> me, have been failing since 5.1. I've now added a make target for it so > >> it's easier to run them without setting up the env variables. > >> > > > > Mind to provide the fsck-test-result.txt for me to further investigate > > the problem? > > My bad, I didn't catch the problem in the pull request, and my test > environments all failed to catch the problem. > > The lack of diversity makes it pretty hard to detect it, as it looks > like all machines tends to zero the unintialized stack structure. > > Anyway, the proper fix for that long existing v5.1 bug is here: > https://patchwork.kernel.org/patch/11454395/ Thank you, that crash was probably the last big thing to do before a release. > And I'll look into the possibility to auto use valgrind in selftests to > avoid similar bug to sneak in. As you've found out, the valgrind or other instrumentation is there but there are some problems. For that the GCCs sanitizers are more lightweight option, but they make the test pass.
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c index 7f734f974d02..1d2df349a9bf 100644 --- a/check/mode-lowmem.c +++ b/check/mode-lowmem.c @@ -2165,8 +2165,12 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path, } } - if (fkey.offset + extent_num_bytes < isize) - *end = fkey.offset + extent_num_bytes; + /* + * Don't update extent end beyond rounded up isize. As holes + * after isize is not considered as missing holes. + */ + *end = min(round_up(isize, root->fs_info->sectorsize), + fkey.offset + extent_num_bytes); if (!is_hole) *size += extent_num_bytes;
Commit 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of holes") makes lowmem mode check to skip hole detection after isize. However it also skipped the extent end update if the extent ends just at isize. This caused fsck-test/001 to fail with false hole error report. Fix it by updating the @end parameter if we have an extent ends at inode size. Fixes: 91a12c0ddb00 ("btrfs-progs: fix lowmem check's handling of holes") Signed-off-by: Qu Wenruo <wqu@suse.com> --- David, please fold the fix into the original patch. --- check/mode-lowmem.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)