Re: [PATCH v4 7/9] Btrfs: add free space tree sanity tests

From: David Sterba <dsterba@suse.cz>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: Omar Sandoval <osandov@osandov.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v4 7/9] Btrfs: add free space tree sanity tests
Date: Wed, 11 Oct 2023 12:20:20 +0200	[thread overview]
Message-ID: <20231011102020.GF2211@twin.jikos.cz> (raw)
In-Reply-To: <3f04845f-b9b5-68b6-90a0-8921eaa53b4d@huawei.com>

On Wed, Oct 11, 2023 at 10:12:37AM +0800, Jinjie Ruan wrote:
> 
> 
> On 2015/9/30 11:50, Omar Sandoval wrote:
     ^^^^^^^^^

> > From: Omar Sandoval <osandov@fb.com>
> > 
> > This tests the operations on the free space tree trying to excercise all
> > of the main cases for both formats. Between this and xfstests, the free
> > space tree should have pretty good coverage.
> > 
> > Signed-off-by: Omar Sandoval <osandov@fb.com>

> > +static int test_empty_block_group(struct btrfs_trans_handle *trans,
> > +				  struct btrfs_fs_info *fs_info,
> > +				  struct btrfs_block_group_cache *cache,
> > +				  struct btrfs_path *path)
> > +{
> > +	struct free_space_extent extents[] = {
> > +		{cache->key.objectid, cache->key.offset},
> > +	};
> > +
> > +	return check_free_space_extents(trans, fs_info, cache, path,
> > +					extents, ARRAY_SIZE(extents));
> 
> Inject fault when probe btrfs.ko,

I wonder how valid is fault injection during self tests, there are lot
of stub structures that are half-initialized and hitting a NULL could be
possible while the tests would not need it and knowingly ignore it.

The whole transaction abort mechanism cannot work reliably, there's no
single start or commit of the transaction in the test/ code so
an abort (in the regular code) can't work.

> there is a null-ptr-deref as below, it
> seems that the transaction in btrfs_trans_handle struct has not been
> initialized but used in __btrfs_abort_transaction() when calling
> WRITE_ONCE(trans->transaction->aborted, errno).

Yeah, not all structures are initialized.