* bug with large_dir in 5.12.17 @ 2021-07-22 14:23 Carlos Carvalho 2021-07-27 6:22 ` Andreas Dilger ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Carlos Carvalho @ 2021-07-22 14:23 UTC (permalink / raw) To: linux-ext4 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-22 14:23 bug with large_dir in 5.12.17 Carlos Carvalho @ 2021-07-27 6:22 ` Andreas Dilger 2021-07-27 19:07 ` Carlos Carvalho 2021-07-28 16:07 ` Благодаренко Артём 2021-07-28 13:56 ` Благодаренко Артём 2021-07-29 19:23 ` Благодаренко Артём 2 siblings, 2 replies; 11+ messages in thread From: Andreas Dilger @ 2021-07-27 6:22 UTC (permalink / raw) To: Carlos Carvalho; +Cc: linux-ext4 [-- Attachment #1: Type: text/plain, Size: 979 bytes --] On Jul 22, 2021, at 8:23 AM, Carlos Carvalho <carlos@fisica.ufpr.br> 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. Hi Carlos, are you able to reproduce this error on a new directory that did not hit the 2-level htree limit before enabling large_dir, or did you only see this with directories that hit the 2-level htree limit before the update? Did you test on any newer kernels than 5.2.17? Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP --] [-- Type: application/pgp-signature, Size: 873 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-27 6:22 ` Andreas Dilger @ 2021-07-27 19:07 ` Carlos Carvalho 2021-07-28 0:38 ` Carlos Carvalho 2021-07-28 16:07 ` Благодаренко Артём 1 sibling, 1 reply; 11+ messages in thread From: Carlos Carvalho @ 2021-07-27 19:07 UTC (permalink / raw) To: Andreas Dilger; +Cc: linux-ext4 Andreas Dilger (adilger@dilger.ca) wrote on Tue, Jul 27, 2021 at 03:22:10AM -03: > On Jul 22, 2021, at 8:23 AM, Carlos Carvalho <carlos@fisica.ufpr.br> 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. > > Hi Carlos, > are you able to reproduce this error on a new directory that did not hit > the 2-level htree limit before enabling large_dir, or did you only see this > with directories that hit the 2-level htree limit before the update? I removed all directories where the error happens (several hours...) and ran the backup again, after fsck, and the error was the same, so it also happens in new ones. > Did you test on any newer kernels than 5.2.17? Not yet. The machine is running 5.13.5 now but this particular backup hasn't run yet. I'm doing fsck now (takes 4h30) and will launch it again. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-27 19:07 ` Carlos Carvalho @ 2021-07-28 0:38 ` Carlos Carvalho 0 siblings, 0 replies; 11+ messages in thread From: Carlos Carvalho @ 2021-07-28 0:38 UTC (permalink / raw) To: Andreas Dilger; +Cc: linux-ext4 To Andreas Dilger (carlos@fisica.ufpr.br) wrote on Tue, Jul 27, 2021 at 04:07:06PM -03: > > Did you test on any newer kernels than 5.2.17? > > Not yet. The machine is running 5.13.5 now but this particular backup hasn't > run yet. I'm doing fsck now (takes 4h30) and will launch it again. The same happens with 5.13.5 after fsck. I didn't check with new directories, just ran rsync. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-27 6:22 ` Andreas Dilger 2021-07-27 19:07 ` Carlos Carvalho @ 2021-07-28 16:07 ` Благодаренко Артём 1 sibling, 0 replies; 11+ messages in thread From: Благодаренко Артём @ 2021-07-28 16:07 UTC (permalink / raw) To: Andreas Dilger; +Cc: Carlos Carvalho, linux-ext4 Hello Andreas, This issue is reproducible in case the “large_dir” option is set initially. The partition is fresh, created just before the test started. 1560000 names created ln: failed to create hard link 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001571063': File exists ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001571384': Structure needs cleaning ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001572335': Structure needs cleaning ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001573095': Structure needs cleaning ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001574809': Structure needs cleaning ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001576070': Structure needs cleaning Best regards, Artem Blagodarenko > On 27 Jul 2021, at 09:22, Andreas Dilger <adilger@dilger.ca> wrote: > > On Jul 22, 2021, at 8:23 AM, Carlos Carvalho <carlos@fisica.ufpr.br> 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. > > Hi Carlos, > are you able to reproduce this error on a new directory that did not hit > the 2-level htree limit before enabling large_dir, or did you only see this > with directories that hit the 2-level htree limit before the update? > Did you test on any newer kernels than 5.2.17? > > Cheers, Andreas > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-22 14:23 bug with large_dir in 5.12.17 Carlos Carvalho 2021-07-27 6:22 ` Andreas Dilger @ 2021-07-28 13:56 ` Благодаренко Артём 2021-07-29 19:23 ` Благодаренко Артём 2 siblings, 0 replies; 11+ messages in thread From: Благодаренко Артём @ 2021-07-28 13:56 UTC (permalink / raw) To: Carlos Carvalho; +Cc: linux-ext4 Hello, I have reproduced the issue on the master head # git rev-parse HEAD f158f8962ed7e884fa168f354c488f3afa3eb6db Here is a reproducer. Not perfect bash script, but reproduce the problem well #!/bin/bash dir_path=/mnt/ext4/ cd $dir_path i=0 target_file=$dir_path/foofile$i touch $target_file while test $i -lt 100000000 ; do if test $((i % 40000)) -eq 0; then echo "$i names created" target_file=$dir_path/foofile$i touch $target_file fi lncom=`printf "ln $target_file n%0253u\n" $i` `$lncom` i=$((i + 1)) done Without large_dir option 1504565 files created and then I got no space 1480000 names created ln: failed to create hard link 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001504566': No space left on device ln: failed to create hard link [root@CO82 ~]# ls /mnt/ext4/ | wc -l 1510070 [root@CO82 ~]# When I enabled large_dir and used the same script, but started from I=1600000 Some amount (36526) of names were created and finally this error happened. sh ~/filldir.sh 1600000 names created ln: failed to create hard link 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001636527' => '/mnt/ext4//foofile1600000': Structure needs cleaning ln: failed to access 'n0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001636797': Structure needs cleaning From dimes [27927.219844] EXT4-fs (loop1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: none. [28058.315171] EXT4-fs error (device loop1): dx_probe:887: inode #2: block 147359: comm ln: directory leaf block found instead of index block [28059.414053] EXT4-fs error (device loop1): dx_probe:887: inode #2: block 147359: comm ln: directory leaf block found instead of index block [28059.627589] EXT4-fs error (device loop1): dx_probe:887: inode #2: block 147359: comm ln: directory leaf block found instead of index block >are you able to reproduce this error on a new directory that did not hit the 2-level htree limit before enabling large_dir, or did you only see this with directories that hit the 2-level htree limit before the update? I am executing this test now. Will report shortly. Best regards, Artem Blagodarenko > On 22 Jul 2021, at 17:23, Carlos Carvalho <carlos@fisica.ufpr.br> 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-22 14:23 bug with large_dir in 5.12.17 Carlos Carvalho 2021-07-27 6:22 ` Andreas Dilger 2021-07-28 13:56 ` Благодаренко Артём @ 2021-07-29 19:23 ` Благодаренко Артём [not found] ` <7f781a3cd7114db0842dc3f291cd3f6cd826917f.camel@voxelsoft.com> 2021-08-04 19:25 ` Theodore Ts'o 2 siblings, 2 replies; 11+ messages in thread From: Благодаренко Артём @ 2021-07-29 19:23 UTC (permalink / raw) To: Carlos Carvalho; +Cc: linux-ext4, Theodore Tso, Andreas Dilger 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 <carlos@fisica.ufpr.br> 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <7f781a3cd7114db0842dc3f291cd3f6cd826917f.camel@voxelsoft.com>]
* Re: bug with large_dir in 5.12.17 [not found] ` <7f781a3cd7114db0842dc3f291cd3f6cd826917f.camel@voxelsoft.com> @ 2021-07-31 5:13 ` Theodore Ts'o 0 siblings, 0 replies; 11+ messages in thread From: Theodore Ts'o @ 2021-07-31 5:13 UTC (permalink / raw) To: Denis Cc: Благодаренко Артём, linux-ext4 On Fri, Jul 30, 2021 at 02:15:12AM +0100, Denis wrote: > > My emails to the list have been silently dropped and postmaster does > not respond. I'm not sure why your e-mails are getting dropped by vger.kernel.org. I will note that voxelsoft.com appears to be on the a barracuda spam blacklist: https://mxtoolbox.com/SuperTool.aspx?action=blacklist%3avoxelsoft.com&run=toolpage > http://voxelsoft.com/2021/ext4_large_dir_corruption.html I've looked your analysis, and while I appreciate your goals: * a naïve approach would be to reinstate the missing goto, but 1. such approach is likely to contribute to another mistake in the future 2. goto considered harmful * instead refactor the outer scope to handle both restart conditions (goto agnostic) I've looked at your proposed fix, and I think a simpler fix might be: 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; It is similarly "goto agnostic" in that it doesn't actually add (or remove) any gotos. I don't think your proposed fix improves the maintainability or understandability for the code, so my bias would be to go for the simpler fix. If we were going to clean up the code, the main issues that I'd want to address are: *) Goto statements aren't necessarilyy evil; using "goto errout" is pretty common idiom, and I don't have a problem with that. Occasionally having a "goto again" when we need to retry some operation can actually be the cleanest way to code something. But the combination of the two --- when "goto cleanup" and "goto journal_error" is used for both purposes --- is super confusing, and that is caused the bug in the first place. So for example, this diff increase the goto count by one, but the code is much more readable, because each goto and goto label do exactly one thing: fs/ext4/namei.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 5fd56f616cf0..58fca88d7459 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -2398,14 +2398,15 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname, { struct dx_frame frames[EXT4_HTREE_LEVEL], *frame; struct dx_entry *entries, *at; - struct buffer_head *bh; + struct buffer_head *bh = NULL; struct super_block *sb = dir->i_sb; struct ext4_dir_entry_2 *de; - int restart; int err; -again: - restart = 0; + frames[0].bh = NULL; +recalc_htree_path: + brelse(bh); + dx_release(frames); frame = dx_probe(fname, dir, NULL, frames); if (IS_ERR(frame)) return PTR_ERR(frame); @@ -2436,7 +2437,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname, ext4_lblk_t newblock; int levels = frame - frames + 1; unsigned int icount; - int add_level = 1; + int add_level = 1, restart = 0; struct dx_entry *entries2; struct dx_node *node2; struct buffer_head *bh2; @@ -2508,9 +2509,9 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname, dxtrace(dx_show_index("node", ((struct dx_node *) bh2->b_data)->entries)); err = ext4_handle_dirty_dx_node(handle, dir, bh2); + brelse (bh2); if (err) goto journal_error; - brelse (bh2); err = ext4_handle_dirty_dx_node(handle, dir, (frame - 1)->bh); if (err) @@ -2519,6 +2520,8 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname, frame->bh); if (err) goto journal_error; + if (restart) + goto recalc_htree_path; } else { struct dx_root *dxroot; memcpy((char *) entries2, (char *) entries, @@ -2538,8 +2541,9 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname, goto journal_error; err = ext4_handle_dirty_dx_node(handle, dir, bh2); brelse(bh2); - restart = 1; - goto journal_error; + if (err) + goto journal_error; + goto recalc_htree_path; } } de = do_split(handle, dir, &bh, frame, &fname->hinfo); @@ -2555,11 +2559,6 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname, cleanup: brelse(bh); dx_release(frames); - /* @restart is true means htree-path has been changed, we need to - * repeat dx_probe() to find out valid htree-path - */ - if (restart && err == 0) - goto again; return err; } *) The ext4_dx_add_entry() function is just too big. It was borderline unwieldy before we added the large_dir feature, and that large_dir feature just tip it over the line. We probably should to refactor separate functions for "do_split_index()" and "add_htree_level()". *) The control avariables "add_level" and "restart" are an additional cause of complexity. It may be that refactoring some of the separate functions, and simply forcing that the hree path get recalculated if an index node is split or the tree depth increases, would allow us to get rid of these two variables. Denis, Artem, what do you think? - Ted ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-07-29 19:23 ` Благодаренко Артём [not found] ` <7f781a3cd7114db0842dc3f291cd3f6cd826917f.camel@voxelsoft.com> @ 2021-08-04 19:25 ` Theodore Ts'o 2021-08-04 21:15 ` Благодаренко Артём 1 sibling, 1 reply; 11+ messages in thread From: Theodore Ts'o @ 2021-08-04 19:25 UTC (permalink / raw) To: Благодаренко Артём Cc: Carlos Carvalho, linux-ext4, Theodore Tso, Andreas Dilger 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 <tytso@mit.edu> 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: stable@kernel.org # v5.11 Cc: Благодаренко Артём <artem.blagodarenko@gmail.com> Fixes: b5776e7524af ("ext4: fix potential htree index checksum corruption) Reported-by: Denis <denis@voxelsoft.com> Signed-off-by: Theodore Ts'o <tytso@mit.edu> --- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-08-04 19:25 ` Theodore Ts'o @ 2021-08-04 21:15 ` Благодаренко Артём 2021-08-05 14:39 ` Theodore Ts'o 0 siblings, 1 reply; 11+ messages in thread From: Благодаренко Артём @ 2021-08-04 21:15 UTC (permalink / raw) To: Theodore Ts'o Cc: Carlos Carvalho, linux-ext4, Theodore Tso, Andreas Dilger 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 <tytso@mit.edu> 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 <tytso@mit.edu> > 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: stable@kernel.org # v5.11 > Cc: Благодаренко Артём <artem.blagodarenko@gmail.com> > Fixes: b5776e7524af ("ext4: fix potential htree index checksum corruption) > Reported-by: Denis <denis@voxelsoft.com> > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > --- > 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: bug with large_dir in 5.12.17 2021-08-04 21:15 ` Благодаренко Артём @ 2021-08-05 14:39 ` Theodore Ts'o 0 siblings, 0 replies; 11+ messages in thread From: Theodore Ts'o @ 2021-08-05 14:39 UTC (permalink / raw) To: Благодаренко Артём Cc: Carlos Carvalho, linux-ext4, Theodore Tso, Andreas Dilger 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 <tytso@mit.edu> Date: Thu Aug 5 09:49:40 2021 -0400 ext4: add test to validate the large_dir feature Signed-off-by: Theodore Ts'o <tytso@mit.edu> 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-08-05 14:40 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-22 14:23 bug with large_dir in 5.12.17 Carlos Carvalho 2021-07-27 6:22 ` Andreas Dilger 2021-07-27 19:07 ` Carlos Carvalho 2021-07-28 0:38 ` Carlos Carvalho 2021-07-28 16:07 ` Благодаренко Артём 2021-07-28 13:56 ` Благодаренко Артём 2021-07-29 19:23 ` Благодаренко Артём [not found] ` <7f781a3cd7114db0842dc3f291cd3f6cd826917f.camel@voxelsoft.com> 2021-07-31 5:13 ` Theodore Ts'o 2021-08-04 19:25 ` Theodore Ts'o 2021-08-04 21:15 ` Благодаренко Артём 2021-08-05 14:39 ` Theodore Ts'o
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.