[PATCH 0/6] Debugging macro and kmalloc_obj* cleanups

[PATCH 0/6] Debugging macro and kmalloc_obj* cleanups

[David Sterba]

Replace some macros with existing or new API. Replacing ASSERT(0) in
free space tree updates error handling but it's for "impossible"
conditions.

David Sterba (6):
  btrfs: move condition to WARN_ON in btrfs_set_delalloc_extent()
  btrfs: replace open coded DEBUG_WARN in extent_writepage()
  btrfs: handle unexpected free-space-tree key types
  btrfs: lift assertions to beginning of insert_delayed_ref()
  btrfs: do more kmalloc_obj()/kmalloc_objs() conversions
  btrfs: convert kmalloc_array to kmalloc_objs in
    btrfs_calc_avail_data_space()

 fs/btrfs/delayed-ref.c     |  5 ++++-
 fs/btrfs/extent_io.c       |  2 +-
 fs/btrfs/free-space-tree.c | 18 +++++++++++++++---
 fs/btrfs/inode.c           |  3 +--
 fs/btrfs/relocation.c      | 10 +++++-----
 fs/btrfs/super.c           |  3 +--
 6 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.53.0

[PATCH 1/6] btrfs: move condition to WARN_ON in btrfs_set_delalloc_extent()

[David Sterba]

For a simple if + WARN_ON we should use the condition directly in the
macro.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1e8e9a3d2d3090..247299d0860139 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2656,8 +2656,7 @@ void btrfs_set_delalloc_extent(struct btrfs_inode *inode, struct extent_state *s
 
 	lockdep_assert_held(&inode->io_tree.lock);
 
-	if ((bits & EXTENT_DEFRAG) && !(bits & EXTENT_DELALLOC))
-		WARN_ON(1);
+	WARN_ON((bits & EXTENT_DEFRAG) && !(bits & EXTENT_DELALLOC));
 	/*
 	 * set_bit and clear bit hooks normally require _irqsave/restore
 	 * but in this case, we are only testing for the DELALLOC
-- 
2.53.0

[PATCH 2/6] btrfs: replace open coded DEBUG_WARN in extent_writepage()

[David Sterba]

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 3411c41993477c..0c15c9783c95da 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1873,7 +1873,7 @@ static int extent_writepage(struct folio *folio, struct btrfs_bio_ctrl *bio_ctrl
 	 */
 	if (IS_ENABLED(CONFIG_BTRFS_EXPERIMENTAL) &&
 	    unlikely(!folio_test_private(folio))) {
-		WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
+		DEBUG_WARN();
 		btrfs_err_rl(fs_info,
 	"root %lld ino %llu folio %llu is marked dirty without notifying the fs",
 			     btrfs_root_id(inode->root),
-- 
2.53.0

[PATCH 3/6] btrfs: handle unexpected free-space-tree key types

[David Sterba]

Replace the conditional assertions with proper error handling and
transaction abort if we find an unexpected key type in the free space
tree.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/free-space-tree.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 9efd1ec90f031f..472b3060e5ac32 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -259,7 +259,11 @@ int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
 				nr++;
 				path->slots[0]--;
 			} else {
-				ASSERT(0);
+				btrfs_err(fs_info, "unexpected free space tree key type %u",
+					  found_key.type);
+				ret = -EUCLEAN;
+				btrfs_abort_transaction(trans, ret);
+				goto out;
 			}
 		}
 
@@ -405,7 +409,11 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
 
 				nr++;
 			} else {
-				ASSERT(0);
+				btrfs_err(fs_info, "unexpected free space tree key type %u",
+					  found_key.type);
+				ret = -EUCLEAN;
+				btrfs_abort_transaction(trans, ret);
+				goto out;
 			}
 		}
 
@@ -1518,7 +1526,11 @@ int btrfs_remove_block_group_free_space(struct btrfs_trans_handle *trans,
 				nr++;
 				path->slots[0]--;
 			} else {
-				ASSERT(0);
+				btrfs_err(trans->fs_info, "unexpected free space tree key type %u",
+					  found_key.type);
+				ret = -EUCLEAN;
+				btrfs_abort_transaction(trans, ret);
+				return ret;
 			}
 		}
 
-- 
2.53.0

[PATCH 4/6] btrfs: lift assertions to beginning of insert_delayed_ref()

[David Sterba]

There are only two possible types of the delayed ref action, this can be
verified at the beginning for the whole function and not just one block.
Replace the assertion with a debugging warning just in case.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/delayed-ref.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 605858c2d9a952..8bc1929237f70a 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -615,6 +615,9 @@ static bool insert_delayed_ref(struct btrfs_trans_handle *trans,
 	struct btrfs_delayed_ref_node *exist;
 	int mod;
 
+	ASSERT(ref->action == BTRFS_ADD_DELAYED_REF ||
+	       ref->action == BTRFS_DROP_DELAYED_REF);
+
 	spin_lock(&href->lock);
 	exist = tree_insert(&href->ref_tree, ref);
 	if (!exist) {
@@ -641,7 +644,7 @@ static bool insert_delayed_ref(struct btrfs_trans_handle *trans,
 				ASSERT(!list_empty(&exist->add_list));
 				list_del_init(&exist->add_list);
 			} else {
-				ASSERT(0);
+				DEBUG_WARN();
 			}
 		} else
 			mod = -ref->ref_mod;
-- 
2.53.0

[PATCH 5/6] btrfs: do more kmalloc_obj()/kmalloc_objs() conversions

[David Sterba]

Do a few more (trivial) conversions that started in commit 69050f8d6d075d
("treewide: Replace kmalloc with kmalloc_obj for non-scalar types").

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 3ebaf5880125fa..9cbae3cf8bdd1e 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -590,7 +590,7 @@ static struct btrfs_root *create_reloc_root(struct btrfs_trans_handle *trans,
 	struct btrfs_key root_key;
 	int ret = 0;
 
-	root_item = kmalloc(sizeof(*root_item), GFP_NOFS);
+	root_item = kmalloc_obj(*root_item, GFP_NOFS);
 	if (!root_item)
 		return ERR_PTR(-ENOMEM);
 
@@ -2944,7 +2944,7 @@ static int relocate_file_extent_cluster(struct reloc_control *rc)
 	if (!cluster->nr)
 		return 0;
 
-	ra = kzalloc(sizeof(*ra), GFP_NOFS);
+	ra = kzalloc_obj(*ra, GFP_NOFS);
 	if (!ra)
 		return -ENOMEM;
 
@@ -3863,7 +3863,7 @@ static int add_remap_tree_entries(struct btrfs_trans_handle *trans, struct btrfs
 
 	max_items = BTRFS_LEAF_DATA_SIZE(trans->fs_info) / sizeof(struct btrfs_item);
 
-	data_sizes = kzalloc(sizeof(u32) * min_t(u32, num_entries, max_items), GFP_NOFS);
+	data_sizes = kzalloc_objs(u32, min_t(u32, num_entries, max_items), GFP_NOFS);
 	if (!data_sizes)
 		return -ENOMEM;
 
@@ -4454,7 +4454,7 @@ static int create_remap_tree_entries(struct btrfs_trans_handle *trans,
 
 	btrfs_release_path(path);
 
-	space_runs = kmalloc(sizeof(*space_runs) * extent_count, GFP_NOFS);
+	space_runs = kmalloc_objs(*space_runs, extent_count, GFP_NOFS);
 	if (!space_runs) {
 		mutex_unlock(&bg->free_space_lock);
 		return -ENOMEM;
@@ -4543,7 +4543,7 @@ static int create_remap_tree_entries(struct btrfs_trans_handle *trans,
 	mutex_unlock(&bg->free_space_lock);
 
 	max_entries = extent_count + 2;
-	entries = kmalloc(sizeof(*entries) * max_entries, GFP_NOFS);
+	entries = kmalloc_objs(*entries, max_entries, GFP_NOFS);
 	if (!entries) {
 		ret = -ENOMEM;
 		goto out;
-- 
2.53.0

[PATCH 6/6] btrfs: convert kmalloc_array to kmalloc_objs in btrfs_calc_avail_data_space()

[David Sterba]

There's one use of kmalloc_array() that can be transformed to
kmalloc_objs() in the same way as suggested in commit 69050f8d6d075d
("treewide: Replace kmalloc with kmalloc_obj for non-scalar types"),
swap the arguments and drop GFP flags. All the other cases of
kmalloc_array() do not use a simple type so this is the only one.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index b26aa9169e8388..f67a268f36a6d6 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1633,8 +1633,7 @@ static inline int btrfs_calc_avail_data_space(struct btrfs_fs_info *fs_info,
 		}
 	}
 
-	devices_info = kmalloc_array(nr_devices, sizeof(*devices_info),
-			       GFP_KERNEL);
+	devices_info = kmalloc_objs(*devices_info, nr_devices);
 	if (!devices_info)
 		return -ENOMEM;
 
-- 
2.53.0

Re: [PATCH 0/6] Debugging macro and kmalloc_obj* cleanups

[Johannes Thumshirn]

Looks good to me,

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>