Re: [PATCH v2] btrfs: extent_io: Always return error pointer for extent buffer allocation failure

[David Sterba <dsterba@xxxxxxx>]

On Mon, Feb 25, 2019 at 12:38:01PM +0800, Qu Wenruo wrote:
>  		eb_rewin = alloc_dummy_extent_buffer(fs_info, eb->start);
> -		if (!eb_rewin) {
> +		if (IS_ERR(eb_rewin)) {
>  			btrfs_tree_read_unlock_blocking(eb);
>  			free_extent_buffer(eb);
>  			return NULL;
> @@ -1384,7 +1384,7 @@ get_old_root(struct btrfs_root *root, u64 time_seq)

A possible followup patch would be to convert the callchain of
get_old_root and the __tree_mod_log_rewind functions to ERR_PTR.

>  		free_extent_buffer(eb_root);
>  	}
>  
> -	if (!eb)
> +	if (IS_ERR(eb))
>  		return NULL;
>  	btrfs_tree_read_lock(eb);
>  	if (old_root) {
> --- a/fs/btrfs/tests/extent-buffer-tests.c
> +++ b/fs/btrfs/tests/extent-buffer-tests.c
> @@ -48,12 +48,14 @@ static int test_btrfs_split_item(u32 sectorsize, u32 nodesize)
>  		goto out;
>  	}
>  
> -	path->nodes[0] = eb = alloc_dummy_extent_buffer(fs_info, nodesize);
> -	if (!eb) {
> +	eb = alloc_dummy_extent_buffer(fs_info, nodesize);
> +	if (IS_ERR(eb)) {
> +		eb = NULL;
>  		test_err("could not allocate dummy buffer");
>  		ret = -ENOMEM;

This should also return PTR_ERR(eb)

>  		goto out;
>  	}
> +	path->nodes[0] = eb;
>  	path->slots[0] = 0;
>  
>  	key.objectid = 0;
> diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
> index 3c46d7f23456..d1f3b727fbf2 100644
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -396,7 +396,7 @@ static int test_eb_bitmaps(u32 sectorsize, u32 nodesize)
>  	}
>  
>  	eb = __alloc_dummy_extent_buffer(fs_info, 0, len);
> -	if (!eb) {
> +	if (IS_ERR(eb)) {
>  		test_err("couldn't allocate test extent buffer");
>  		kfree(bitmap);
>  		return -ENOMEM;

same

> @@ -409,7 +409,7 @@ static int test_eb_bitmaps(u32 sectorsize, u32 nodesize)
>  	/* Do it over again with an extent buffer which isn't page-aligned. */
>  	free_extent_buffer(eb);
>  	eb = __alloc_dummy_extent_buffer(NULL, nodesize / 2, len);
> -	if (!eb) {
> +	if (IS_ERR(eb)) {
>  		test_err("couldn't allocate test extent buffer");
>  		kfree(bitmap);
>  		return -ENOMEM;

same

> diff --git a/fs/btrfs/tests/free-space-tree-tests.c b/fs/btrfs/tests/free-space-tree-tests.c
> index 89346da890cf..025fce63959a 100644
> --- a/fs/btrfs/tests/free-space-tree-tests.c
> +++ b/fs/btrfs/tests/free-space-tree-tests.c
> @@ -462,7 +462,8 @@ static int run_test(test_func_t test_func, int bitmaps, u32 sectorsize,
>  	root->fs_info->tree_root = root;
>  
>  	root->node = alloc_test_extent_buffer(root->fs_info, nodesize);
> -	if (!root->node) {
> +	if (IS_ERR(root->node)) {
> +		root->node = NULL;
>  		test_err("couldn't allocate dummy buffer");
>  		ret = -ENOMEM;

same

>  		goto out;
> diff --git a/fs/btrfs/tests/inode-tests.c b/fs/btrfs/tests/inode-tests.c
> index af0c8e30d9e2..56a112a39211 100644
> --- a/fs/btrfs/tests/inode-tests.c
> +++ b/fs/btrfs/tests/inode-tests.c
> @@ -249,7 +249,8 @@ static noinline int test_btrfs_get_extent(u32 sectorsize, u32 nodesize)
>  	}
>  
>  	root->node = alloc_dummy_extent_buffer(fs_info, nodesize);
> -	if (!root->node) {
> +	if (IS_ERR(root->node)) {
> +		root->node = NULL;
>  		test_err("couldn't allocate dummy buffer");
>  		goto out;
>  	}
> @@ -850,7 +851,8 @@ static int test_hole_first(u32 sectorsize, u32 nodesize)
>  	}
>  
>  	root->node = alloc_dummy_extent_buffer(fs_info, nodesize);
> -	if (!root->node) {
> +	if (IS_ERR(root->node)) {
> +		root->node = NULL;
>  		test_err("couldn't allocate dummy buffer");
>  		goto out;
>  	}
> diff --git a/fs/btrfs/tests/qgroup-tests.c b/fs/btrfs/tests/qgroup-tests.c
> index 412b910b04cc..536600eb4d9d 100644
> --- a/fs/btrfs/tests/qgroup-tests.c
> +++ b/fs/btrfs/tests/qgroup-tests.c
> @@ -484,7 +484,8 @@ int btrfs_test_qgroups(u32 sectorsize, u32 nodesize)
>  	 * *cough*backref walking code*cough*
>  	 */
>  	root->node = alloc_test_extent_buffer(root->fs_info, nodesize);
> -	if (!root->node) {
> +	if (IS_ERR(root->node)) {
> +		root->node = NULL;
>  		test_err("couldn't allocate dummy buffer");
>  		ret = -ENOMEM;

same

>  		goto out;