Re: [PATCH] btrfs: initialize test inodes location

[David Sterba <dsterba@suse.cz>]

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.