Hello,
It looks like the fix b5776e7524afbd4569978ff790864755c438bba7 "ext4: fix potential htree index checksum corruption” introduced this regression.
I reverted it and my test from previous message passed the dangerous level of 1570000 names count.
Now test is still in progress. 2520000 names are already created.
I am searching the way to fix this.
Best regards,
Artem Blagodarenko.
> On 22 Jul 2021, at 17:23, Carlos Carvalho <[email protected]> wrote:
>
> There is a bug when enabling large_dir in 5.12.17. I got this during a backup:
>
> index full, reach max htree level :2
> Large directory feature is not enabled on this filesystem
>
> So I unmounted, ran tune2fs -O large_dir /dev/device and mounted again. However
> this error appeared:
>
> dx_probe:864: inode #576594294: block 144245: comm rsync: directory leaf block found instead of index block
>
> I unmounted, ran fsck and it "salvaged" a bunch of directories. However at the
> next backup run the same errors appeared again.
>
> This is with vanilla 5.2.17.
On Thu, Jul 29, 2021 at 10:23:35PM +0300, Благодаренко Артём wrote:
> Hello,
>
> It looks like the fix b5776e7524afbd4569978ff790864755c438bba7 "ext4: fix potential htree index checksum corruption” introduced this regression.
> I reverted it and my test from previous message passed the dangerous level of 1570000 names count.
> Now test is still in progress. 2520000 names are already created.
>
> I am searching the way to fix this.
>
> Best regards,
> Artem Blagodarenko.
Hi Artem, did you have a chance to take a look at some of the possible
fixes which I floated on this thread?
Do you have any objections if I take this and send it to Linus?
Thanks,
- Ted
From fa8db30806b4e83981c65f18f98de33f804012d9 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Wed, 4 Aug 2021 14:23:55 -0400
Subject: [PATCH] ext4: fix potential htree correuption when growing large_dir
directories
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit b5776e7524af ("ext4: fix potential htree index checksum
corruption) removed a required restart when multiple levels of index
nodes need to be split. Fix this to avoid directory htree corruptions
when using the large_dir feature.
Cc: [email protected] # v5.11
Cc: Благодаренко Артём <[email protected]>
Fixes: b5776e7524af ("ext4: fix potential htree index checksum corruption)
Reported-by: Denis <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
---
fs/ext4/namei.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 5fd56f616cf0..f3bbcd4efb56 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2517,7 +2517,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
goto journal_error;
err = ext4_handle_dirty_dx_node(handle, dir,
frame->bh);
- if (err)
+ if (restart || err)
goto journal_error;
} else {
struct dx_root *dxroot;
--
2.31.0
Hello Teodore,
Your one-line fix looks good.
I have tested it. 1560000 names created successfully.
But the patch with refactoring doesn’t work. I got this messages
1480000 names created
1520000 names created
ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001520519': Bad message
ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001520520': Bad message
[ 7699.212018] EXT4-fs error (device loop0): dx_probe:887: inode #2: block 144843: comm ln: Directory index failed checksum
[ 7699.216001] EXT4-fs error (device loop0): dx_probe:887: inode #2: block 144843: comm ln: Directory index failed checksum
I have no objections to send your one-line fix, but we need to double check refactoring.
Best regards,
Artem Blagodarenko
> On 4 Aug 2021, at 22:25, Theodore Ts'o <[email protected]> wrote:
>
> On Thu, Jul 29, 2021 at 10:23:35PM +0300, Благодаренко Артём wrote:
>> Hello,
>>
>> It looks like the fix b5776e7524afbd4569978ff790864755c438bba7 "ext4: fix potential htree index checksum corruption” introduced this regression.
>> I reverted it and my test from previous message passed the dangerous level of 1570000 names count.
>> Now test is still in progress. 2520000 names are already created.
>>
>> I am searching the way to fix this.
>>
>> Best regards,
>> Artem Blagodarenko.
>
> Hi Artem, did you have a chance to take a look at some of the possible
> fixes which I floated on this thread?
>
> Do you have any objections if I take this and send it to Linus?
>
> Thanks,
>
> - Ted
>
> From fa8db30806b4e83981c65f18f98de33f804012d9 Mon Sep 17 00:00:00 2001
> From: Theodore Ts'o <[email protected]>
> Date: Wed, 4 Aug 2021 14:23:55 -0400
> Subject: [PATCH] ext4: fix potential htree correuption when growing large_dir
> directories
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Commit b5776e7524af ("ext4: fix potential htree index checksum
> corruption) removed a required restart when multiple levels of index
> nodes need to be split. Fix this to avoid directory htree corruptions
> when using the large_dir feature.
>
> Cc: [email protected] # v5.11
> Cc: Благодаренко Артём <[email protected]>
> Fixes: b5776e7524af ("ext4: fix potential htree index checksum corruption)
> Reported-by: Denis <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
> ---
> fs/ext4/namei.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 5fd56f616cf0..f3bbcd4efb56 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2517,7 +2517,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
> goto journal_error;
> err = ext4_handle_dirty_dx_node(handle, dir,
> frame->bh);
> - if (err)
> + if (restart || err)
> goto journal_error;
> } else {
> struct dx_root *dxroot;
> --
> 2.31.0
>
On Thu, Aug 05, 2021 at 12:15:41AM +0300, Благодаренко Артём wrote:
> Hello Teodore,
>
> Your one-line fix looks good.
>
> I have tested it. 1560000 names created successfully.
>
> But the patch with refactoring doesn’t work.
Thanks for testing both patches. I was more intersted in the one-line
fix so I hadn't tried testing the refactoring patch. That's for a
later cleanup.
In any case, I've come up with a test to automate testing and to make
sure we don't regress in the future. It runs pretty quickly, for
either failure or success, so it's good as a smoke test. We may want
to consider a longer stress test as well. Artem, do you have any
suggestions?
- Ted
commit 5c156367952b22431850dfad8b2b2c8b753a3e91
Author: Theodore Ts'o <[email protected]>
Date: Thu Aug 5 09:49:40 2021 -0400
ext4: add test to validate the large_dir feature
Signed-off-by: Theodore Ts'o <[email protected]>
diff --git a/src/dirstress.c b/src/dirstress.c
index 615cb6e3..ec28d643 100644
--- a/src/dirstress.c
+++ b/src/dirstress.c
@@ -16,6 +16,7 @@ int verbose;
int pid;
int checkflag=0;
+int create_only=0;
#define MKNOD_DEV 0
@@ -51,7 +52,7 @@ main(
nprocs_per_dir = 1;
keep = 0;
verbose = 0;
- while ((c = getopt(argc, argv, "d:p:f:s:n:kvc")) != EOF) {
+ while ((c = getopt(argc, argv, "d:p:f:s:n:kvcC")) != EOF) {
switch(c) {
case 'p':
nprocs = atoi(optarg);
@@ -80,6 +81,9 @@ main(
case 'c':
checkflag++;
break;
+ case 'C':
+ create_only++;
+ break;
}
}
if (errflg || (dirname == NULL)) {
@@ -170,6 +174,7 @@ dirstress(
if (create_entries(nfiles)) {
printf("!! [%d] create failed\n", pid);
} else {
+ if (create_only) return 0;
if (verbose) fprintf(stderr,"** [%d] scramble entries\n", pid);
if (scramble_entries(nfiles)) {
printf("!! [%d] scramble failed\n", pid);
diff --git a/tests/ext4/051 b/tests/ext4/051
new file mode 100755
index 00000000..387e2518
--- /dev/null
+++ b/tests/ext4/051
@@ -0,0 +1,62 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# FS QA Test 051
+#
+# Test ext4's large_dir feature
+#
+. ./common/preamble
+_begin_fstest auto quick dir
+
+# Override the default cleanup function.
+_cleanup()
+{
+ cd /
+ rm -r -f $tmp.*
+ if [ ! -z "$loop_mnt" ]; then
+ $UMOUNT_PROG $loop_mnt
+ rm -rf $loop_mnt
+ fi
+ [ ! -z "$fs_img" ] && rm -rf $fs_img
+}
+
+# Import common functions.
+# . ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs ext4
+_require_test
+_require_loop
+_require_scratch_ext4_feature "large_dir"
+
+echo "Silence is golden"
+
+loop_mnt=$TEST_DIR/$seq.mnt
+fs_img=$TEST_DIR/$seq.img
+status=0
+
+cp /dev/null $fs_img
+${MKFS_PROG} -t ${FSTYP} -b 1024 -N 600020 -O large_dir,^has_journal \
+ $fs_img 40G >> $seqres.full 2>&1 || _fail "mkfs failed"
+
+mkdir -p $loop_mnt
+_mount -o loop $fs_img $loop_mnt > /dev/null 2>&1 || \
+ _fail "Couldn't do initial mount"
+
+/root/xfstests/src/dirstress -c -d /tmpmnt -p 1 -f 400000 -C \
+ >> $seqres.full 2>&1
+
+if ! $here/src/dirstress -c -d $loop_mnt -p 1 -f 400000 -C >$tmp.out 2>&1
+then
+ echo " dirstress failed"
+ cat $tmp.out >> $seqres.full
+ echo " dirstress failed" >> $seqres.full
+ status=1
+fi
+
+$UMOUNT_PROG $loop_mnt || _fail "umount failed"
+loop_mnt=
+
+$E2FSCK_PROG -fn $fs_img >> $seqres.full 2>&1 || _fail "file system corrupted"
diff --git a/tests/ext4/051.out b/tests/ext4/051.out
new file mode 100644
index 00000000..32f74d89
--- /dev/null
+++ b/tests/ext4/051.out
@@ -0,0 +1,2 @@
+QA output created by 051
+Silence is golden