| Message ID | 20180313231907.215181-1-harshads@google.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | e2fsck: Release clusters only once in release_inode_blocks | expand |
On Tue, Mar 13, 2018 at 04:19:07PM -0700, harshads wrote: > While releasing inode blocks, if bigalloc feature is turned on, > release clusters only once. We do it by remembering the last released > cluster while iterating through blocks and releasing a cluster only if > it is not already released. > > Signed-off-by: Harshad Shirwadkar <harshads@google.com> Thanks, applied. For future reference, this is my local preference and a tiny nit-pick, but I prefer the first word after the colon to be uncapitalized (since it isn't necessarily, and in most cases shouldn't be, a full complete sentence). So I would prefer: e2fsck: release clusters only once in release_inode_blocks Also, either included in the same commit, or in a separate commit (especially if several commits in a sequence are needed to fix a bug), it's a really good idea to create a regression test. This is also useful because it means that you know you can create as small file system to use a simple, pure regression test. For example: commit a126f79a0eee38762d8e3d110281ae86322de444 Author: Theodore Ts'o <tytso@mit.edu> Date: Fri Mar 16 11:19:02 2018 -0400 tests: add new test f_bigalloc_orphan_list This is a regression test for the bugfix found in commit 707599bf: "e2fsck: release clusters only once in release_inode_blocks" Signed-off-by: Theodore Ts'o <tytso@mit.edu> diff --git a/tests/f_bigalloc_orphan_list/expect.1 b/tests/f_bigalloc_orphan_list/expect.1 new file mode 100644 index 000000000..622620d06 --- /dev/null +++ b/tests/f_bigalloc_orphan_list/expect.1 @@ -0,0 +1,10 @@ +Clearing orphaned inode 12 (uid=0, gid=0, mode=0100644, size=28672) +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information + +test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** +test_filesys: 11/64 files (0.0% non-contiguous), 96/1024 blocks +Exit status is 0 diff --git a/tests/f_bigalloc_orphan_list/expect.2 b/tests/f_bigalloc_orphan_list/expect.2 new file mode 100644 index 000000000..30392d428 --- /dev/null +++ b/tests/f_bigalloc_orphan_list/expect.2 @@ -0,0 +1,7 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information +test_filesys: 11/64 files (0.0% non-contiguous), 96/1024 blocks +Exit status is 0 diff --git a/tests/f_bigalloc_orphan_list/name b/tests/f_bigalloc_orphan_list/name new file mode 100644 index 000000000..5d9566ff9 --- /dev/null +++ b/tests/f_bigalloc_orphan_list/name @@ -0,0 +1 @@ +orphaned list handling with bigalloc file systems diff --git a/tests/f_bigalloc_orphan_list/script b/tests/f_bigalloc_orphan_list/script new file mode 100644 index 000000000..1508bf126 --- /dev/null +++ b/tests/f_bigalloc_orphan_list/script @@ -0,0 +1,28 @@ +if test -x $DEBUGFS_EXE; then + +SKIP_GUNZIP="true" +TEST_DATA="$test_name.tmp" + +dd if=$TEST_BITS of=$TEST_DATA bs=28k count=1 > /dev/null 2>&1 + +touch $TMPFILE +mke2fs -Fq -t ext4 -O bigalloc $TMPFILE 1M > /dev/null 2>&1 +debugfs -w $TMPFILE << EOF > /dev/null 2>&1 +write $TEST_DATA testfile +set_inode_field testfile links_count 0 +set_inode_field testfile bmap[0] 0 +set_inode_field testfile bmap[2] 0 +set_super_value last_orphan 12 +unlink testfile +quit +EOF + +. $cmd_dir/run_e2fsck + +rm -f $TEST_DATA + +unset E2FSCK_TIME TEST_DATA + +else #if test -x $DEBUGFS_EXE; then + echo "$test_name: $test_description: skipped" +fi
diff --git a/e2fsck/super.c b/e2fsck/super.c index 5501c9e2..5e29b64e 100644 --- a/e2fsck/super.c +++ b/e2fsck/super.c @@ -71,6 +71,7 @@ struct process_block_struct { int truncated_blocks; int abort; errcode_t errcode; + blk64_t last_cluster; }; static int release_inode_block(ext2_filsys fs, @@ -84,6 +85,7 @@ static int release_inode_block(ext2_filsys fs, e2fsck_t ctx; struct problem_context *pctx; blk64_t blk = *block_nr; + blk64_t cluster = EXT2FS_B2C(fs, *block_nr); int retval = 0; pb = (struct process_block_struct *) priv_data; @@ -96,6 +98,11 @@ static int release_inode_block(ext2_filsys fs, if (blk == 0) return 0; + if (pb->last_cluster == cluster) + return 0; + + pb->last_cluster = cluster; + if ((blk < fs->super->s_first_data_block) || (blk >= ext2fs_blocks_count(fs->super))) { fix_problem(ctx, PR_0_ORPHAN_ILLEGAL_BLOCK_NUM, pctx); @@ -188,6 +195,7 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino, pb.abort = 0; pb.errcode = 0; pb.pctx = pctx; + pb.last_cluster = 0; if (inode->i_links_count) { pb.truncating = 1; pb.truncate_block = (e2_blkcnt_t)
While releasing inode blocks, if bigalloc feature is turned on, release clusters only once. We do it by remembering the last released cluster while iterating through blocks and releasing a cluster only if it is not already released. Signed-off-by: Harshad Shirwadkar <harshads@google.com> --- e2fsck/super.c | 8 ++++++++ 1 file changed, 8 insertions(+)