* [PATCH] btrfs: initialize test inodes location
@ 2020-12-15 17:00 Josef Bacik
2020-12-17 12:41 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2020-12-15 17:00 UTC (permalink / raw)
To: linux-btrfs, kernel-team
While testing other things I was noticing that sometimes my VM would
fail to load the btrfs module because the self test failed like this
BTRFS: selftest: fs/btrfs/tests/inode-tests.c:963 miscount, wanted 1, got 0
This turned out to be because sometimes the btrfs ino would be the btree
inode number, and thus we'd skip calling the set extent delalloc bit
helper, and thus not adjust ->outstanding_extents. Fix this by making
sure we init test inodes with a valid inode number so that we don't get
random failures during self tests.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/tests/btrfs-tests.c | 7 ++++++-
fs/btrfs/tests/inode-tests.c | 9 ---------
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index 8ca334d554af..0fede1514a3e 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -55,8 +55,13 @@ struct inode *btrfs_new_test_inode(void)
struct inode *inode;
inode = new_inode(test_mnt->mnt_sb);
- if (inode)
+ if (inode) {
+ inode->i_mode = S_IFREG;
+ BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
+ BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
+ BTRFS_I(inode)->location.offset = 0;
inode_init_owner(inode, NULL, S_IFREG);
+ }
return inode;
}
diff --git a/fs/btrfs/tests/inode-tests.c b/fs/btrfs/tests/inode-tests.c
index 04022069761d..c9874b12d337 100644
--- a/fs/btrfs/tests/inode-tests.c
+++ b/fs/btrfs/tests/inode-tests.c
@@ -232,11 +232,6 @@ static noinline int test_btrfs_get_extent(u32 sectorsize, u32 nodesize)
return ret;
}
- inode->i_mode = S_IFREG;
- BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
- BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
- BTRFS_I(inode)->location.offset = 0;
-
fs_info = btrfs_alloc_dummy_fs_info(nodesize, sectorsize);
if (!fs_info) {
test_std_err(TEST_ALLOC_FS_INFO);
@@ -835,10 +830,6 @@ static int test_hole_first(u32 sectorsize, u32 nodesize)
return ret;
}
- BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
- BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
- BTRFS_I(inode)->location.offset = 0;
-
fs_info = btrfs_alloc_dummy_fs_info(nodesize, sectorsize);
if (!fs_info) {
test_std_err(TEST_ALLOC_FS_INFO);
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] btrfs: initialize test inodes location
2020-12-15 17:00 [PATCH] btrfs: initialize test inodes location Josef Bacik
@ 2020-12-17 12:41 ` David Sterba
2020-12-17 14:39 ` Josef Bacik
0 siblings, 1 reply; 4+ messages in thread
From: David Sterba @ 2020-12-17 12:41 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs, kernel-team
On Tue, Dec 15, 2020 at 12:00:26PM -0500, Josef Bacik wrote:
> While testing other things I was noticing that sometimes my VM would
> fail to load the btrfs module because the self test failed like this
>
> BTRFS: selftest: fs/btrfs/tests/inode-tests.c:963 miscount, wanted 1, got 0
>
> This turned out to be because sometimes the btrfs ino would be the btree
> inode number, and thus we'd skip calling the set extent delalloc bit
> helper, and thus not adjust ->outstanding_extents. Fix this by making
> sure we init test inodes with a valid inode number so that we don't get
> random failures during self tests.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/btrfs/tests/btrfs-tests.c | 7 ++++++-
> fs/btrfs/tests/inode-tests.c | 9 ---------
> 2 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
> index 8ca334d554af..0fede1514a3e 100644
> --- a/fs/btrfs/tests/btrfs-tests.c
> +++ b/fs/btrfs/tests/btrfs-tests.c
> @@ -55,8 +55,13 @@ struct inode *btrfs_new_test_inode(void)
> struct inode *inode;
>
> inode = new_inode(test_mnt->mnt_sb);
> - if (inode)
> + if (inode) {
> + inode->i_mode = S_IFREG;
> + BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
> + BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
> + BTRFS_I(inode)->location.offset = 0;
> inode_init_owner(inode, NULL, S_IFREG);
> + }
As this is adding more statements to the if-block, I'd rather rewrite it
as
inode = new();
if (!inode)
return NULL;
inode-> ...
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] btrfs: initialize test inodes location
2020-12-17 12:41 ` David Sterba
@ 2020-12-17 14:39 ` Josef Bacik
2020-12-17 14:41 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2020-12-17 14:39 UTC (permalink / raw)
To: dsterba, linux-btrfs, kernel-team
On 12/17/20 7:41 AM, David Sterba wrote:
> On Tue, Dec 15, 2020 at 12:00:26PM -0500, Josef Bacik wrote:
>> While testing other things I was noticing that sometimes my VM would
>> fail to load the btrfs module because the self test failed like this
>>
>> BTRFS: selftest: fs/btrfs/tests/inode-tests.c:963 miscount, wanted 1, got 0
>>
>> This turned out to be because sometimes the btrfs ino would be the btree
>> inode number, and thus we'd skip calling the set extent delalloc bit
>> helper, and thus not adjust ->outstanding_extents. Fix this by making
>> sure we init test inodes with a valid inode number so that we don't get
>> random failures during self tests.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> ---
>> fs/btrfs/tests/btrfs-tests.c | 7 ++++++-
>> fs/btrfs/tests/inode-tests.c | 9 ---------
>> 2 files changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
>> index 8ca334d554af..0fede1514a3e 100644
>> --- a/fs/btrfs/tests/btrfs-tests.c
>> +++ b/fs/btrfs/tests/btrfs-tests.c
>> @@ -55,8 +55,13 @@ struct inode *btrfs_new_test_inode(void)
>> struct inode *inode;
>>
>> inode = new_inode(test_mnt->mnt_sb);
>> - if (inode)
>> + if (inode) {
>> + inode->i_mode = S_IFREG;
>> + BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
>> + BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
>> + BTRFS_I(inode)->location.offset = 0;
>> inode_init_owner(inode, NULL, S_IFREG);
>> + }
>
> As this is adding more statements to the if-block, I'd rather rewrite it
> as
>
> inode = new();
> if (!inode)
> return NULL;
>
> inode-> ...
>
Agreed, I've updated it locally, I'll wait for comments for the rest of the
series and then resend. Thanks,
Josef
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] btrfs: initialize test inodes location
2020-12-17 14:39 ` Josef Bacik
@ 2020-12-17 14:41 ` David Sterba
0 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2020-12-17 14:41 UTC (permalink / raw)
To: Josef Bacik; +Cc: dsterba, linux-btrfs, kernel-team
On Thu, Dec 17, 2020 at 09:39:18AM -0500, Josef Bacik wrote:
> On 12/17/20 7:41 AM, David Sterba wrote:
> > On Tue, Dec 15, 2020 at 12:00:26PM -0500, Josef Bacik wrote:
> >> While testing other things I was noticing that sometimes my VM would
> >> fail to load the btrfs module because the self test failed like this
> >>
> >> BTRFS: selftest: fs/btrfs/tests/inode-tests.c:963 miscount, wanted 1, got 0
> >>
> >> This turned out to be because sometimes the btrfs ino would be the btree
> >> inode number, and thus we'd skip calling the set extent delalloc bit
> >> helper, and thus not adjust ->outstanding_extents. Fix this by making
> >> sure we init test inodes with a valid inode number so that we don't get
> >> random failures during self tests.
> >>
> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >> ---
> >> fs/btrfs/tests/btrfs-tests.c | 7 ++++++-
> >> fs/btrfs/tests/inode-tests.c | 9 ---------
> >> 2 files changed, 6 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
> >> index 8ca334d554af..0fede1514a3e 100644
> >> --- a/fs/btrfs/tests/btrfs-tests.c
> >> +++ b/fs/btrfs/tests/btrfs-tests.c
> >> @@ -55,8 +55,13 @@ struct inode *btrfs_new_test_inode(void)
> >> struct inode *inode;
> >>
> >> inode = new_inode(test_mnt->mnt_sb);
> >> - if (inode)
> >> + if (inode) {
> >> + inode->i_mode = S_IFREG;
> >> + BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
> >> + BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
> >> + BTRFS_I(inode)->location.offset = 0;
> >> inode_init_owner(inode, NULL, S_IFREG);
> >> + }
> >
> > As this is adding more statements to the if-block, I'd rather rewrite it
> > as
> >
> > inode = new();
> > if (!inode)
> > return NULL;
> >
> > inode-> ...
> >
>
> Agreed, I've updated it locally, I'll wait for comments for the rest of the
> series and then resend. Thanks,
No need to resend unless something else pops up, the change is trivial.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-17 14:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-15 17:00 [PATCH] btrfs: initialize test inodes location Josef Bacik
2020-12-17 12:41 ` David Sterba
2020-12-17 14:39 ` Josef Bacik
2020-12-17 14:41 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox