Now it never fails when inserting a delay extent, so the return value in
ext4_es_insert_delayed_block is no longer necessary, let it return void.
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/extents_status.c | 10 ++++------
fs/ext4/extents_status.h | 4 ++--
fs/ext4/inode.c | 10 ++--------
3 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 2a394c40f4b7..b12c5cfdf601 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -2020,11 +2020,9 @@ bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk)
* @lblk - logical block to be added
* @allocated - indicates whether a physical cluster has been allocated for
* the logical cluster that contains the block
- *
- * Returns 0 on success, negative error code on failure.
*/
-int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
- bool allocated)
+void ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
+ bool allocated)
{
struct extent_status newes;
int err1 = 0;
@@ -2033,7 +2031,7 @@ int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
struct extent_status *es2 = NULL;
if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
- return 0;
+ return;
es_debug("add [%u/1) delayed to extent status tree of inode %lu\n",
lblk, inode->i_ino);
@@ -2075,7 +2073,7 @@ int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
ext4_es_print_tree(inode);
ext4_print_pending_tree(inode);
- return 0;
+ return;
}
/*
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 526a68890aa6..c22edb931f1b 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -249,8 +249,8 @@ extern void ext4_exit_pending(void);
extern void ext4_init_pending_tree(struct ext4_pending_tree *tree);
extern void ext4_remove_pending(struct inode *inode, ext4_lblk_t lblk);
extern bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk);
-extern int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
- bool allocated);
+extern void ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
+ bool allocated);
extern unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
ext4_lblk_t len);
extern void ext4_clear_inode_es(struct inode *inode);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index a0bfe77d5537..4221b2dafeb5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1641,9 +1641,8 @@ static void ext4_print_free_blocks(struct inode *inode)
static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
{
struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
- int ret;
+ int ret = 0;
bool allocated = false;
- bool reserved = false;
/*
* If the cluster containing lblk is shared with a delayed,
@@ -1660,7 +1659,6 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
ret = ext4_da_reserve_space(inode);
if (ret != 0) /* ENOSPC */
goto errout;
- reserved = true;
} else { /* bigalloc */
if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
if (!ext4_es_scan_clu(inode,
@@ -1673,7 +1671,6 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
ret = ext4_da_reserve_space(inode);
if (ret != 0) /* ENOSPC */
goto errout;
- reserved = true;
} else {
allocated = true;
}
@@ -1683,10 +1680,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
}
}
- ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
- if (ret && reserved)
- ext4_da_release_space(inode, 1);
-
+ ext4_es_insert_delayed_block(inode, lblk, allocated);
errout:
return ret;
}
--
2.31.1On Mon, Apr 24, 2023 at 11:38:44AM +0800, Baokun Li wrote:
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index a0bfe77d5537..4221b2dafeb5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1641,9 +1641,8 @@ static void ext4_print_free_blocks(struct inode *inode)
> static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> {
> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> - int ret;
> + int ret = 0;
> bool allocated = false;
> - bool reserved = false;
>
> /*
> * If the cluster containing lblk is shared with a delayed,
Unforuntately, the changes to ext4_insert_delayed_block() in this
patch were buggy, and were causing tests to hang when running
ext4/encrypt, ext4/bigalloc_4k, and ext4/bigalloc_1k test scenarios.
A bisect using "gce-xfstests -c ext4/bigalloc_4k -C 5 generic/579"
pinpointed the problem.
The problem is that ext4_clu_mapped can return a positive value, and
so there are times when we do need to release the space even though
there are no errors.
So I've fixed up your commit with the following changes. With this
change, the test regressions go away.
Cheers,
- Ted
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index fa38092ef868..3a10427240cb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1630,6 +1630,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
int ret = 0;
bool allocated = false;
+ bool reserved = false;
/*
* If the cluster containing lblk is shared with a delayed,
@@ -1646,6 +1647,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
ret = ext4_da_reserve_space(inode);
if (ret != 0) /* ENOSPC */
goto errout;
+ reserved = true;
} else { /* bigalloc */
if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
if (!ext4_es_scan_clu(inode,
@@ -1658,6 +1660,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
ret = ext4_da_reserve_space(inode);
if (ret != 0) /* ENOSPC */
goto errout;
+ reserved = true;
} else {
allocated = true;
}
@@ -1668,6 +1671,9 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
}
ext4_es_insert_delayed_block(inode, lblk, allocated);
+ if (ret && reserved)
+ ext4_da_release_space(inode, 1);
+
errout:
return ret;
}
On Sat, Jun 10, 2023 at 03:03:19PM -0400, Theodore Ts'o wrote:
> Unforuntately, the changes to ext4_insert_delayed_block() in this
> patch were buggy, and were causing tests to hang when running
> ext4/encrypt, ext4/bigalloc_4k, and ext4/bigalloc_1k test scenarios.
> A bisect using "gce-xfstests -c ext4/bigalloc_4k -C 5 generic/579"
> pinpointed the problem.
>
> The problem is that ext4_clu_mapped can return a positive value, and
> so there are times when we do need to release the space even though
> there are no errors.
>
> So I've fixed up your commit with the following changes. With this
> change, the test regressions go away.
It turns out my fix was not correct, because I misread the fundamental
problem with the patch. The issue was in the last patch hunk:
- ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
- if (ret && reserved)
- ext4_da_release_space(inode, 1);
-
+ ext4_es_insert_delayed_block(inode, lblk, allocated);
errout:
return ret;
}
The problem is that entering this code hunk, ret could be non-zero.
But when we made ext4_es_insert_delayed_block() to return void. So
the changes to fs/ext4/inode.c needed to be replaced by this:
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 129b9af53d62..7700db1782dd 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1630,7 +1630,6 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
int ret;
bool allocated = false;
- bool reserved = false;
/*
* If the cluster containing lblk is shared with a delayed,
@@ -1646,8 +1645,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
if (sbi->s_cluster_ratio == 1) {
ret = ext4_da_reserve_space(inode);
if (ret != 0) /* ENOSPC */
- goto errout;
- reserved = true;
+ return ret;
} else { /* bigalloc */
if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
if (!ext4_es_scan_clu(inode,
@@ -1655,12 +1653,11 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
ret = ext4_clu_mapped(inode,
EXT4_B2C(sbi, lblk));
if (ret < 0)
- goto errout;
+ return ret;
if (ret == 0) {
ret = ext4_da_reserve_space(inode);
if (ret != 0) /* ENOSPC */
- goto errout;
- reserved = true;
+ return ret;
} else {
allocated = true;
}
@@ -1670,12 +1667,8 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
}
}
- ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
- if (ret && reserved)
- ext4_da_release_space(inode, 1);
-
-errout:
- return ret;
+ ext4_es_insert_delayed_block(inode, lblk, allocated);
+ return 0;
}
/*
- Ted
On 2023/6/12 11:04, Theodore Ts'o wrote:
> On Sat, Jun 10, 2023 at 03:03:19PM -0400, Theodore Ts'o wrote:
>> Unforuntately, the changes to ext4_insert_delayed_block() in this
>> patch were buggy, and were causing tests to hang when running
>> ext4/encrypt, ext4/bigalloc_4k, and ext4/bigalloc_1k test scenarios.
>> A bisect using "gce-xfstests -c ext4/bigalloc_4k -C 5 generic/579"
>> pinpointed the problem.
I'm very sorry, I didn't turn on encrypt or bigalloc when I tested it.
>>
>> The problem is that ext4_clu_mapped can return a positive value, and
>> so there are times when we do need to release the space even though
>> there are no errors.
Yes, ext4_clu_mapped may return a positive value,
but when it does, reserved is false and we never need to release the space.
>> So I've fixed up your commit with the following changes. With this
>> change, the test regressions go away.
The previous reply was very confusing to me because the changes
in the previous reply have nothing to do with ext4_clu_mapped
and ret is always 0 when reserved is true, so we don't need
ext4_da_release_space to perform a rollback.
> It turns out my fix was not correct, because I misread the fundamental
> problem with the patch. The issue was in the last patch hunk:
>
> - ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
> - if (ret && reserved)
> - ext4_da_release_space(inode, 1);
> -
> + ext4_es_insert_delayed_block(inode, lblk, allocated);
> errout:
> return ret;
> }
Indeed, there is a behavioral change in ret here.
Before modification:
ext4_da_map_blocks --> return 0
ext4_insert_delayed_block --> return 0
ext4_clu_mapped --> return 1
ext4_es_insert_delayed_block --> return 0
After modification:
ext4_da_map_blocks --> return 1
ext4_insert_delayed_block --> return 1
ext4_clu_mapped --> return 1
ext4_es_insert_delayed_block --> void
>
> The problem is that entering this code hunk, ret could be non-zero.
> But when we made ext4_es_insert_delayed_block() to return void. So
> the changes to fs/ext4/inode.c needed to be replaced by this:
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 129b9af53d62..7700db1782dd 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1630,7 +1630,6 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> int ret;
> bool allocated = false;
> - bool reserved = false;
>
> /*
> * If the cluster containing lblk is shared with a delayed,
> @@ -1646,8 +1645,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> if (sbi->s_cluster_ratio == 1) {
> ret = ext4_da_reserve_space(inode);
> if (ret != 0) /* ENOSPC */
> - goto errout;
> - reserved = true;
> + return ret;
> } else { /* bigalloc */
> if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
> if (!ext4_es_scan_clu(inode,
> @@ -1655,12 +1653,11 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> ret = ext4_clu_mapped(inode,
> EXT4_B2C(sbi, lblk));
> if (ret < 0)
> - goto errout;
> + return ret;
> if (ret == 0) {
> ret = ext4_da_reserve_space(inode);
> if (ret != 0) /* ENOSPC */
> - goto errout;
> - reserved = true;
> + return ret;
> } else {
> allocated = true;
> }
> @@ -1670,12 +1667,8 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> }
> }
>
> - ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
> - if (ret && reserved)
> - ext4_da_release_space(inode, 1);
> -
> -errout:
> - return ret;
> + ext4_es_insert_delayed_block(inode, lblk, allocated);
> + return 0;
> }
>
> /*
>
> - Ted
>
Yes, it looks very good！A million thanks for the fix!
I am very sorry for taking your time to locate and fix this issue!
I will do more checks later.
--
With Best Regards,
Baokun Li
.
On Mon, Jun 12, 2023 at 11:47:07AM +0800, Baokun Li wrote:
> I'm very sorry, I didn't turn on encrypt or bigalloc when I tested it.
For complex series, it's helpful if you could run the equivalent of:
{gce,kvm}-xfstests -c ext4/all -g auto
It takes about 24 hours (plus or minus, depending on the speed of your
storage device; it will also take about twice as long if
CONFIG_LOCKDEP is enabled) if you use kvm-xfstests. Using
gce-xfstests with the ltm takes about around 1.75 hours (w/o LOCKDEP)
since it runs the tests in parallel, using separate VM's for each file
system config.
There are a small number of failures (especially flaky test failures),
however, (a) if a VM ever crashes, that's definitely a problem, and
(b) the ext4/4k config should be failure-free. For example, here's a
current "good" test run that I'm checking the dev branch against.
(Currently, we have some kind of issue with -c ext4/adv generic/475
that I'm still chasing down.)
- Ted
The failures seen below are known failures that we need to work
through. Bill Whitney is working on the bigalloc_1k shared/298
failure, for example. If you would like to work on one of the test
failures, especially if it's a file system config that you use in
production, please feel free to do so. :-) Also, if you are
interested in adapting the xfstests-bld codebase to support other
cloud services beyond Google Cloud Engine, again, let me know.
The gce-xfstests run below was generated using:
% gce-xfstests install-kconfig --lockdep
% gce-xfstests kbuild --dpkg
% gce-xfstests launch-ltm
% gce-xfstests ltm full
(Using the --dpkg is needed because is because there is a kexec bug
showing up when running on a Google Cloud VM's that I haven't been
able to fix, and it's been easier to just work around the kexec
problem. Kexec works just fine on kvm-xfstests, though, so there's no
need to use kbuild --dpkg if you are just using kvm-xfstests.)
TESTRUNID: ltm-20230611154922
KERNEL: kernel 6.4.0-rc5-xfstests-lockdep-00002-gdea9d8f7643f #170 SMP PREEMPT_DYNAMIC Sun Jun 11 15:21:52 EDT 2023 x86_64
CMDLINE: full
CPUS: 2
MEM: 7680
ext4/4k: 549 tests, 51 skipped, 6895 seconds
ext4/1k: 545 tests, 54 skipped, 10730 seconds
ext4/ext3: 541 tests, 140 skipped, 8547 seconds
ext4/encrypt: 527 tests, 3 failures, 158 skipped, 5783 seconds
Failures: generic/681 generic/682 generic/691
ext4/nojournal: 544 tests, 3 failures, 119 skipped, 8394 seconds
Failures: ext4/301 ext4/304 generic/455
ext4/ext3conv: 546 tests, 52 skipped, 9024 seconds
ext4/adv: 546 tests, 2 failures, 59 skipped, 8454 seconds
Failures: generic/477
Flaky: generic/475: 60% (3/5)
ext4/dioread_nolock: 547 tests, 51 skipped, 7883 seconds
ext4/data_journal: 545 tests, 2 failures, 119 skipped, 7605 seconds
Failures: generic/455 generic/484
ext4/bigalloc_4k: 521 tests, 56 skipped, 6650 seconds
ext4/bigalloc_1k: 521 tests, 1 failures, 64 skipped, 8074 seconds
Failures: shared/298
ext4/dax: 536 tests, 154 skipped, 5118 seconds
Totals: 6512 tests, 1077 skipped, 53 failures, 0 errors, 82214s
FSTESTIMG: gce-xfstests/xfstests-amd64-202305310154
FSTESTPRJ: gce-xfstests
FSTESTVER: blktests 676d42c (Thu, 2 Mar 2023 15:25:44 +0900)
FSTESTVER: e2fsprogs 1.47.0-2-4-gd4745c4a (Tue, 30 May 2023 16:20:44 -0400)
FSTESTVER: fio fio-3.31 (Tue, 9 Aug 2022 14:41:25 -0600)
FSTESTVER: fsverity v1.5-6-g5d6f7c4 (Mon, 30 Jan 2023 23:22:45 -0800)
FSTESTVER: ima-evm-utils v1.3.2 (Wed, 28 Oct 2020 13:18:08 -0400)
FSTESTVER: nvme-cli v1.16 (Thu, 11 Nov 2021 13:09:06 -0800)
FSTESTVER: quota v4.05-53-gd90b7d5 (Tue, 6 Dec 2022 12:59:03 +0100)
FSTESTVER: util-linux v2.38.1 (Thu, 4 Aug 2022 11:06:21 +0200)
FSTESTVER: xfsprogs v6.1.1 (Fri, 13 Jan 2023 19:06:37 +0100)
FSTESTVER: xfstests-bld 6599baba-dirty (Wed, 19 Apr 2023 23:16:10 -0400)
FSTESTVER: xfstests v2023.04.09-8-g2525b7af5-dirty (Wed, 19 Apr 2023 13:42:14 -0400)
FSTESTVER: zz_build-distro bullseye
FSTESTSET: -g auto
FSTESTOPT: aex
On 2023/6/12 23:26, Theodore Ts'o wrote:
> On Mon, Jun 12, 2023 at 11:47:07AM +0800, Baokun Li wrote:
>> I'm very sorry, I didn't turn on encrypt or bigalloc when I tested it.
> For complex series, it's helpful if you could run the equivalent of:
>
> {gce,kvm}-xfstests -c ext4/all -g auto
>
> It takes about 24 hours (plus or minus, depending on the speed of your
> storage device; it will also take about twice as long if
> CONFIG_LOCKDEP is enabled) if you use kvm-xfstests. Using
> gce-xfstests with the ltm takes about around 1.75 hours (w/o LOCKDEP)
> since it runs the tests in parallel, using separate VM's for each file
> system config.
>
> There are a small number of failures (especially flaky test failures),
> however, (a) if a VM ever crashes, that's definitely a problem, and
> (b) the ext4/4k config should be failure-free. For example, here's a
> current "good" test run that I'm checking the dev branch against.
> (Currently, we have some kind of issue with -c ext4/adv generic/475
> that I'm still chasing down.)
>
> - Ted
>
> The failures seen below are known failures that we need to work
> through. Bill Whitney is working on the bigalloc_1k shared/298
> failure, for example. If you would like to work on one of the test
> failures, especially if it's a file system config that you use in
> production, please feel free to do so. :-) Also, if you are
> interested in adapting the xfstests-bld codebase to support other
> cloud services beyond Google Cloud Engine, again, let me know.
It looks very good and I'll try to use it.
I'll try to locate some test case failures when I get free.
>
> The gce-xfstests run below was generated using:
>
> % gce-xfstests install-kconfig --lockdep
> % gce-xfstests kbuild --dpkg
> % gce-xfstests launch-ltm
> % gce-xfstests ltm full
>
> (Using the --dpkg is needed because is because there is a kexec bug
> showing up when running on a Google Cloud VM's that I haven't been
> able to fix, and it's been easier to just work around the kexec
> problem. Kexec works just fine on kvm-xfstests, though, so there's no
> need to use kbuild --dpkg if you are just using kvm-xfstests.)
>
> TESTRUNID: ltm-20230611154922
> KERNEL: kernel 6.4.0-rc5-xfstests-lockdep-00002-gdea9d8f7643f #170 SMP PREEMPT_DYNAMIC Sun Jun 11 15:21:52 EDT 2023 x86_64
> CMDLINE: full
> CPUS: 2
> MEM: 7680
>
> ext4/4k: 549 tests, 51 skipped, 6895 seconds
> ext4/1k: 545 tests, 54 skipped, 10730 seconds
> ext4/ext3: 541 tests, 140 skipped, 8547 seconds
> ext4/encrypt: 527 tests, 3 failures, 158 skipped, 5783 seconds
> Failures: generic/681 generic/682 generic/691
> ext4/nojournal: 544 tests, 3 failures, 119 skipped, 8394 seconds
> Failures: ext4/301 ext4/304 generic/455
> ext4/ext3conv: 546 tests, 52 skipped, 9024 seconds
> ext4/adv: 546 tests, 2 failures, 59 skipped, 8454 seconds
> Failures: generic/477
> Flaky: generic/475: 60% (3/5)
> ext4/dioread_nolock: 547 tests, 51 skipped, 7883 seconds
> ext4/data_journal: 545 tests, 2 failures, 119 skipped, 7605 seconds
> Failures: generic/455 generic/484
> ext4/bigalloc_4k: 521 tests, 56 skipped, 6650 seconds
> ext4/bigalloc_1k: 521 tests, 1 failures, 64 skipped, 8074 seconds
> Failures: shared/298
> ext4/dax: 536 tests, 154 skipped, 5118 seconds
> Totals: 6512 tests, 1077 skipped, 53 failures, 0 errors, 82214s
>
> FSTESTIMG: gce-xfstests/xfstests-amd64-202305310154
> FSTESTPRJ: gce-xfstests
> FSTESTVER: blktests 676d42c (Thu, 2 Mar 2023 15:25:44 +0900)
> FSTESTVER: e2fsprogs 1.47.0-2-4-gd4745c4a (Tue, 30 May 2023 16:20:44 -0400)
> FSTESTVER: fio fio-3.31 (Tue, 9 Aug 2022 14:41:25 -0600)
> FSTESTVER: fsverity v1.5-6-g5d6f7c4 (Mon, 30 Jan 2023 23:22:45 -0800)
> FSTESTVER: ima-evm-utils v1.3.2 (Wed, 28 Oct 2020 13:18:08 -0400)
> FSTESTVER: nvme-cli v1.16 (Thu, 11 Nov 2021 13:09:06 -0800)
> FSTESTVER: quota v4.05-53-gd90b7d5 (Tue, 6 Dec 2022 12:59:03 +0100)
> FSTESTVER: util-linux v2.38.1 (Thu, 4 Aug 2022 11:06:21 +0200)
> FSTESTVER: xfsprogs v6.1.1 (Fri, 13 Jan 2023 19:06:37 +0100)
> FSTESTVER: xfstests-bld 6599baba-dirty (Wed, 19 Apr 2023 23:16:10 -0400)
> FSTESTVER: xfstests v2023.04.09-8-g2525b7af5-dirty (Wed, 19 Apr 2023 13:42:14 -0400)
> FSTESTVER: zz_build-distro bullseye
> FSTESTSET: -g auto
> FSTESTOPT: aex
>
I was using native xfstests for my previous tests, and I feel that
gce-xfstests is much easier to use and the results are very clear,
thanks for the great recommendation!
--
With Best Regards,
Baokun Li
.
On Mon 24-04-23 11:38:44, Baokun Li wrote:
> Now it never fails when inserting a delay extent, so the return value in
> ext4_es_insert_delayed_block is no longer necessary, let it return void.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Nice. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents_status.c | 10 ++++------
> fs/ext4/extents_status.h | 4 ++--
> fs/ext4/inode.c | 10 ++--------
> 3 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 2a394c40f4b7..b12c5cfdf601 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -2020,11 +2020,9 @@ bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk)
> * @lblk - logical block to be added
> * @allocated - indicates whether a physical cluster has been allocated for
> * the logical cluster that contains the block
> - *
> - * Returns 0 on success, negative error code on failure.
> */
> -int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
> - bool allocated)
> +void ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
> + bool allocated)
> {
> struct extent_status newes;
> int err1 = 0;
> @@ -2033,7 +2031,7 @@ int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
> struct extent_status *es2 = NULL;
>
> if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
> - return 0;
> + return;
>
> es_debug("add [%u/1) delayed to extent status tree of inode %lu\n",
> lblk, inode->i_ino);
> @@ -2075,7 +2073,7 @@ int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
>
> ext4_es_print_tree(inode);
> ext4_print_pending_tree(inode);
> - return 0;
> + return;
> }
>
> /*
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index 526a68890aa6..c22edb931f1b 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -249,8 +249,8 @@ extern void ext4_exit_pending(void);
> extern void ext4_init_pending_tree(struct ext4_pending_tree *tree);
> extern void ext4_remove_pending(struct inode *inode, ext4_lblk_t lblk);
> extern bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk);
> -extern int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
> - bool allocated);
> +extern void ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
> + bool allocated);
> extern unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
> ext4_lblk_t len);
> extern void ext4_clear_inode_es(struct inode *inode);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index a0bfe77d5537..4221b2dafeb5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1641,9 +1641,8 @@ static void ext4_print_free_blocks(struct inode *inode)
> static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> {
> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> - int ret;
> + int ret = 0;
> bool allocated = false;
> - bool reserved = false;
>
> /*
> * If the cluster containing lblk is shared with a delayed,
> @@ -1660,7 +1659,6 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> ret = ext4_da_reserve_space(inode);
> if (ret != 0) /* ENOSPC */
> goto errout;
> - reserved = true;
> } else { /* bigalloc */
> if (!ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk)) {
> if (!ext4_es_scan_clu(inode,
> @@ -1673,7 +1671,6 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> ret = ext4_da_reserve_space(inode);
> if (ret != 0) /* ENOSPC */
> goto errout;
> - reserved = true;
> } else {
> allocated = true;
> }
> @@ -1683,10 +1680,7 @@ static int ext4_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk)
> }
> }
>
> - ret = ext4_es_insert_delayed_block(inode, lblk, allocated);
> - if (ret && reserved)
> - ext4_da_release_space(inode, 1);
> -
> + ext4_es_insert_delayed_block(inode, lblk, allocated);
> errout:
> return ret;
> }
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
© 2016 - 2025 Red Hat, Inc.