All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff
@ 2022-09-14 15:06 Josef Bacik
  2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
                   ` (17 more replies)
  0 siblings, 18 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

Hello,

This is the first part in what will probably be very many parts in my work to
cleanup ctree.h.  These are the smaller changes, mostly removing code that's not
used anymore, moving some stuff that's local to C files that don't need to be in
the header at all, and moving the rest of the on-disk definition stuff to
btrfs_tree.h.  There's a lot of patche here, but they're all relatively small,
the largest being the patch to move the on-disk definitions to btrfs_tree.h,
which is not very large compared to patches in the next several series.  This
has been built and tested, it's relatively low risk.  Thanks,

Josef

Josef Bacik (17):
  btrfs: remove set/clear_pending_info helpers
  btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH
  btrfs: remove BTRFS_IOPRIO_READA
  btrfs: move btrfs on disk definitions out of ctree.h
  btrfs: move btrfs_get_block_group helper out of disk-io.h
  btrfs: move maximum limits to btrfs_tree.h
  btrfs: move BTRFS_MAX_MIRRORS into scrub.c
  btrfs: move discard stat defs to free-space-cache.h
  btrfs: move btrfs_chunk_item_size out of ctree.h
  btrfs: move btrfs_should_fragment_free_space into block-group.c
  btrfs: move flush related definitions to space-info.h
  btrfs: move btrfs_print_data_csum_error into inode.c
  btrfs: move trans_handle_cachep out of ctree.h
  btrfs: move btrfs_path_cachep out of ctree.h
  btrfs: move free space cachep's out of ctree.h
  btrfs: move btrfs_next_old_item into ctree.c
  btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h

 fs/btrfs/block-group.c          |  12 +
 fs/btrfs/block-group.h          |  11 +-
 fs/btrfs/btrfs_inode.h          |  26 ---
 fs/btrfs/ctree.c                |  26 +++
 fs/btrfs/ctree.h                | 388 ++------------------------------
 fs/btrfs/delayed-inode.c        |   1 +
 fs/btrfs/disk-io.c              |   7 +
 fs/btrfs/disk-io.h              |   8 +-
 fs/btrfs/free-space-cache.c     |  28 +++
 fs/btrfs/free-space-cache.h     |  11 +
 fs/btrfs/inode-item.c           |   1 +
 fs/btrfs/inode.c                |  57 ++---
 fs/btrfs/props.c                |   1 +
 fs/btrfs/relocation.c           |   1 +
 fs/btrfs/scrub.c                |  11 +
 fs/btrfs/space-info.h           |  59 +++++
 fs/btrfs/super.c                |  24 +-
 fs/btrfs/transaction.c          |  17 ++
 fs/btrfs/transaction.h          |   2 +
 fs/btrfs/volumes.c              |   7 +
 fs/btrfs/volumes.h              |   2 +-
 include/uapi/linux/btrfs_tree.h | 226 +++++++++++++++++++
 22 files changed, 476 insertions(+), 450 deletions(-)

-- 
2.26.3


^ permalink raw reply	[flat|nested] 61+ messages in thread

* [PATCH 01/17] btrfs: remove set/clear_pending_info helpers
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
                     ` (2 more replies)
  2022-09-14 15:06 ` [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH Josef Bacik
                   ` (16 subsequent siblings)
  17 siblings, 3 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

The last users of these helpers were removed in

5297199a8bca ("btrfs: remove inode number cache feature")

so delete these helpers.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8b7b7a212da0..0003ba925d93 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1644,30 +1644,6 @@ do {									\
 #define btrfs_clear_pending(info, opt)	\
 	clear_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)
 
-/*
- * Helpers for setting pending mount option changes.
- *
- * Expects corresponding macros
- * BTRFS_PENDING_SET_ and CLEAR_ + short mount option name
- */
-#define btrfs_set_pending_and_info(info, opt, fmt, args...)            \
-do {                                                                   \
-       if (!btrfs_raw_test_opt((info)->mount_opt, opt)) {              \
-               btrfs_info((info), fmt, ##args);                        \
-               btrfs_set_pending((info), SET_##opt);                   \
-               btrfs_clear_pending((info), CLEAR_##opt);               \
-       }                                                               \
-} while(0)
-
-#define btrfs_clear_pending_and_info(info, opt, fmt, args...)          \
-do {                                                                   \
-       if (btrfs_raw_test_opt((info)->mount_opt, opt)) {               \
-               btrfs_info((info), fmt, ##args);                        \
-               btrfs_set_pending((info), CLEAR_##opt);                 \
-               btrfs_clear_pending((info), SET_##opt);                 \
-       }                                                               \
-} while(0)
-
 /*
  * Inode flags
  */
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
  2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
  2022-09-15 14:11   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA Josef Bacik
                   ` (15 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This hasn't been used since

138a12d86574 ("btrfs: rip out btrfs_space_info::total_bytes_pinned")

so it is safe to remove.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0003ba925d93..3936bb95331d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -87,14 +87,6 @@ struct btrfs_ioctl_encoded_io_args;
 
 #define BTRFS_DIRTY_METADATA_THRESH	SZ_32M
 
-/*
- * Use large batch size to reduce overhead of metadata updates.  On the reader
- * side, we only read it when we are close to ENOSPC and the read overhead is
- * mostly related to the number of CPUs, so it is OK to use arbitrary large
- * value here.
- */
-#define BTRFS_TOTAL_BYTES_PINNED_BATCH	SZ_128M
-
 #define BTRFS_MAX_EXTENT_SIZE SZ_128M
 
 /*
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
  2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
  2022-09-14 15:06 ` [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
  2022-09-15 14:12   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h Josef Bacik
                   ` (14 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

The last user of this definition was removed in patch

f26c92386028 ("btrfs: remove reada infrastructure")

so we can remove this definition.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3936bb95331d..5cf18a120dff 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -82,9 +82,6 @@ struct btrfs_ioctl_encoded_io_args;
 
 #define BTRFS_EMPTY_DIR_SIZE 0
 
-/* ioprio of readahead is set to idle */
-#define BTRFS_IOPRIO_READA (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_IDLE, 0))
-
 #define BTRFS_DIRTY_METADATA_THRESH	SZ_32M
 
 #define BTRFS_MAX_EXTENT_SIZE SZ_128M
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (2 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:07   ` Qu Wenruo
  2022-09-14 15:06 ` [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h Josef Bacik
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

The bulk of our on-disk definitions exist in btrfs_tree.h, which user
space can use.  Keep things consistent and move the rest of the on disk
definitions out of ctree.h into btrfs_tree.h.  Note I did have to update
all u8's to __u8, but otherwise this is a strict copy and paste.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h                | 215 +-------------------------------
 include/uapi/linux/btrfs_tree.h | 213 +++++++++++++++++++++++++++++++
 2 files changed, 214 insertions(+), 214 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5cf18a120dff..c3a8440d3223 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -50,8 +50,6 @@ struct btrfs_ref;
 struct btrfs_bio;
 struct btrfs_ioctl_encoded_io_args;
 
-#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
-
 /*
  * Maximum number of mirrors that can be available for all profiles counting
  * the target device of dev-replace as one. During an active device replace
@@ -63,8 +61,6 @@ struct btrfs_ioctl_encoded_io_args;
  */
 #define BTRFS_MAX_MIRRORS (4 + 1)
 
-#define BTRFS_MAX_LEVEL 8
-
 #define BTRFS_OLDEST_GENERATION	0ULL
 
 /*
@@ -133,81 +129,9 @@ enum {
 	BTRFS_FS_STATE_COUNT
 };
 
-#define BTRFS_BACKREF_REV_MAX		256
-#define BTRFS_BACKREF_REV_SHIFT		56
-#define BTRFS_BACKREF_REV_MASK		(((u64)BTRFS_BACKREF_REV_MAX - 1) << \
-					 BTRFS_BACKREF_REV_SHIFT)
-
-#define BTRFS_OLD_BACKREF_REV		0
-#define BTRFS_MIXED_BACKREF_REV		1
-
-/*
- * every tree block (leaf or node) starts with this header.
- */
-struct btrfs_header {
-	/* these first four must match the super block */
-	u8 csum[BTRFS_CSUM_SIZE];
-	u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
-	__le64 bytenr; /* which block this node is supposed to live in */
-	__le64 flags;
-
-	/* allowed to be different from the super from here on down */
-	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
-	__le64 generation;
-	__le64 owner;
-	__le32 nritems;
-	u8 level;
-} __attribute__ ((__packed__));
-
-/*
- * this is a very generous portion of the super block, giving us
- * room to translate 14 chunks with 3 stripes each.
- */
-#define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
-
-/*
- * just in case we somehow lose the roots and are not able to mount,
- * we store an array of the roots from previous transactions
- * in the super.
- */
-#define BTRFS_NUM_BACKUP_ROOTS 4
-struct btrfs_root_backup {
-	__le64 tree_root;
-	__le64 tree_root_gen;
-
-	__le64 chunk_root;
-	__le64 chunk_root_gen;
-
-	__le64 extent_root;
-	__le64 extent_root_gen;
-
-	__le64 fs_root;
-	__le64 fs_root_gen;
-
-	__le64 dev_root;
-	__le64 dev_root_gen;
-
-	__le64 csum_root;
-	__le64 csum_root_gen;
-
-	__le64 total_bytes;
-	__le64 bytes_used;
-	__le64 num_devices;
-	/* future */
-	__le64 unused_64[4];
-
-	u8 tree_root_level;
-	u8 chunk_root_level;
-	u8 extent_root_level;
-	u8 fs_root_level;
-	u8 dev_root_level;
-	u8 csum_root_level;
-	/* future and to align */
-	u8 unused_8[10];
-} __attribute__ ((__packed__));
-
 #define BTRFS_SUPER_INFO_OFFSET			SZ_64K
 #define BTRFS_SUPER_INFO_SIZE			4096
+static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
 
 /*
  * The reserved space at the beginning of each device.
@@ -216,69 +140,6 @@ struct btrfs_root_backup {
  */
 #define BTRFS_DEVICE_RANGE_RESERVED			(SZ_1M)
 
-/*
- * the super block basically lists the main trees of the FS
- * it currently lacks any block count etc etc
- */
-struct btrfs_super_block {
-	/* the first 4 fields must match struct btrfs_header */
-	u8 csum[BTRFS_CSUM_SIZE];
-	/* FS specific UUID, visible to user */
-	u8 fsid[BTRFS_FSID_SIZE];
-	__le64 bytenr; /* this block number */
-	__le64 flags;
-
-	/* allowed to be different from the btrfs_header from here own down */
-	__le64 magic;
-	__le64 generation;
-	__le64 root;
-	__le64 chunk_root;
-	__le64 log_root;
-
-	/*
-	 * This member has never been utilized since the very beginning, thus
-	 * it's always 0 regardless of kernel version.  We always use
-	 * generation + 1 to read log tree root.  So here we mark it deprecated.
-	 */
-	__le64 __unused_log_root_transid;
-	__le64 total_bytes;
-	__le64 bytes_used;
-	__le64 root_dir_objectid;
-	__le64 num_devices;
-	__le32 sectorsize;
-	__le32 nodesize;
-	__le32 __unused_leafsize;
-	__le32 stripesize;
-	__le32 sys_chunk_array_size;
-	__le64 chunk_root_generation;
-	__le64 compat_flags;
-	__le64 compat_ro_flags;
-	__le64 incompat_flags;
-	__le16 csum_type;
-	u8 root_level;
-	u8 chunk_root_level;
-	u8 log_root_level;
-	struct btrfs_dev_item dev_item;
-
-	char label[BTRFS_LABEL_SIZE];
-
-	__le64 cache_generation;
-	__le64 uuid_tree_generation;
-
-	/* the UUID written into btree blocks */
-	u8 metadata_uuid[BTRFS_FSID_SIZE];
-
-	/* future expansion */
-	u8 reserved8[8];
-	__le64 reserved[27];
-	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
-	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
-
-	/* Padded to 4096 bytes */
-	u8 padding[565];
-} __attribute__ ((__packed__));
-static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
-
 /*
  * Compat flags that we support.  If any incompat flags are set other than the
  * ones specified below then we will fail to mount
@@ -336,43 +197,6 @@ static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
 	(BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF)
 #define BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR		0ULL
 
-/*
- * A leaf is full of items. offset and size tell us where to find
- * the item in the leaf (relative to the start of the data area)
- */
-struct btrfs_item {
-	struct btrfs_disk_key key;
-	__le32 offset;
-	__le32 size;
-} __attribute__ ((__packed__));
-
-/*
- * leaves have an item area and a data area:
- * [item0, item1....itemN] [free space] [dataN...data1, data0]
- *
- * The data is separate from the items to get the keys closer together
- * during searches.
- */
-struct btrfs_leaf {
-	struct btrfs_header header;
-	struct btrfs_item items[];
-} __attribute__ ((__packed__));
-
-/*
- * all non-leaf blocks are nodes, they hold only keys and pointers to
- * other blocks
- */
-struct btrfs_key_ptr {
-	struct btrfs_disk_key key;
-	__le64 blockptr;
-	__le64 generation;
-} __attribute__ ((__packed__));
-
-struct btrfs_node {
-	struct btrfs_header header;
-	struct btrfs_key_ptr ptrs[];
-} __attribute__ ((__packed__));
-
 /* Read ahead values for struct btrfs_path.reada */
 enum {
 	READA_NONE,
@@ -1633,43 +1457,6 @@ do {									\
 #define btrfs_clear_pending(info, opt)	\
 	clear_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)
 
-/*
- * Inode flags
- */
-#define BTRFS_INODE_NODATASUM		(1U << 0)
-#define BTRFS_INODE_NODATACOW		(1U << 1)
-#define BTRFS_INODE_READONLY		(1U << 2)
-#define BTRFS_INODE_NOCOMPRESS		(1U << 3)
-#define BTRFS_INODE_PREALLOC		(1U << 4)
-#define BTRFS_INODE_SYNC		(1U << 5)
-#define BTRFS_INODE_IMMUTABLE		(1U << 6)
-#define BTRFS_INODE_APPEND		(1U << 7)
-#define BTRFS_INODE_NODUMP		(1U << 8)
-#define BTRFS_INODE_NOATIME		(1U << 9)
-#define BTRFS_INODE_DIRSYNC		(1U << 10)
-#define BTRFS_INODE_COMPRESS		(1U << 11)
-
-#define BTRFS_INODE_ROOT_ITEM_INIT	(1U << 31)
-
-#define BTRFS_INODE_FLAG_MASK						\
-	(BTRFS_INODE_NODATASUM |					\
-	 BTRFS_INODE_NODATACOW |					\
-	 BTRFS_INODE_READONLY |						\
-	 BTRFS_INODE_NOCOMPRESS |					\
-	 BTRFS_INODE_PREALLOC |						\
-	 BTRFS_INODE_SYNC |						\
-	 BTRFS_INODE_IMMUTABLE |					\
-	 BTRFS_INODE_APPEND |						\
-	 BTRFS_INODE_NODUMP |						\
-	 BTRFS_INODE_NOATIME |						\
-	 BTRFS_INODE_DIRSYNC |						\
-	 BTRFS_INODE_COMPRESS |						\
-	 BTRFS_INODE_ROOT_ITEM_INIT)
-
-#define BTRFS_INODE_RO_VERITY		(1U << 0)
-
-#define BTRFS_INODE_RO_FLAG_MASK	(BTRFS_INODE_RO_VERITY)
-
 struct btrfs_map_token {
 	struct extent_buffer *eb;
 	char *kaddr;
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 1f7a38ec6ac3..e6bf902b9c92 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -10,6 +10,10 @@
 #include <stddef.h>
 #endif
 
+#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
+
+#define BTRFS_MAX_LEVEL 8
+
 /*
  * This header contains the structure definitions and constants used
  * by file system objects that can be retrieved using
@@ -360,6 +364,43 @@ enum btrfs_csum_type {
 #define BTRFS_FT_XATTR		8
 #define BTRFS_FT_MAX		9
 
+/*
+ * Inode flags
+ */
+#define BTRFS_INODE_NODATASUM		(1U << 0)
+#define BTRFS_INODE_NODATACOW		(1U << 1)
+#define BTRFS_INODE_READONLY		(1U << 2)
+#define BTRFS_INODE_NOCOMPRESS		(1U << 3)
+#define BTRFS_INODE_PREALLOC		(1U << 4)
+#define BTRFS_INODE_SYNC		(1U << 5)
+#define BTRFS_INODE_IMMUTABLE		(1U << 6)
+#define BTRFS_INODE_APPEND		(1U << 7)
+#define BTRFS_INODE_NODUMP		(1U << 8)
+#define BTRFS_INODE_NOATIME		(1U << 9)
+#define BTRFS_INODE_DIRSYNC		(1U << 10)
+#define BTRFS_INODE_COMPRESS		(1U << 11)
+
+#define BTRFS_INODE_ROOT_ITEM_INIT	(1U << 31)
+
+#define BTRFS_INODE_FLAG_MASK						\
+	(BTRFS_INODE_NODATASUM |					\
+	 BTRFS_INODE_NODATACOW |					\
+	 BTRFS_INODE_READONLY |						\
+	 BTRFS_INODE_NOCOMPRESS |					\
+	 BTRFS_INODE_PREALLOC |						\
+	 BTRFS_INODE_SYNC |						\
+	 BTRFS_INODE_IMMUTABLE |					\
+	 BTRFS_INODE_APPEND |						\
+	 BTRFS_INODE_NODUMP |						\
+	 BTRFS_INODE_NOATIME |						\
+	 BTRFS_INODE_DIRSYNC |						\
+	 BTRFS_INODE_COMPRESS |						\
+	 BTRFS_INODE_ROOT_ITEM_INIT)
+
+#define BTRFS_INODE_RO_VERITY		(1U << 0)
+
+#define BTRFS_INODE_RO_FLAG_MASK	(BTRFS_INODE_RO_VERITY)
+
 /*
  * The key defines the order in the tree, and so it also defines (optimal)
  * block layout.
@@ -389,6 +430,108 @@ struct btrfs_key {
 	__u64 offset;
 } __attribute__ ((__packed__));
 
+/*
+ * every tree block (leaf or node) starts with this header.
+ */
+struct btrfs_header {
+	/* these first four must match the super block */
+	__u8 csum[BTRFS_CSUM_SIZE];
+	__u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
+	__le64 bytenr; /* which block this node is supposed to live in */
+	__le64 flags;
+
+	/* allowed to be different from the super from here on down */
+	__u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
+	__le64 generation;
+	__le64 owner;
+	__le32 nritems;
+	__u8 level;
+} __attribute__ ((__packed__));
+
+/*
+ * this is a very generous portion of the super block, giving us
+ * room to translate 14 chunks with 3 stripes each.
+ */
+#define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
+
+/*
+ * just in case we somehow lose the roots and are not able to mount,
+ * we store an array of the roots from previous transactions
+ * in the super.
+ */
+#define BTRFS_NUM_BACKUP_ROOTS 4
+struct btrfs_root_backup {
+	__le64 tree_root;
+	__le64 tree_root_gen;
+
+	__le64 chunk_root;
+	__le64 chunk_root_gen;
+
+	__le64 extent_root;
+	__le64 extent_root_gen;
+
+	__le64 fs_root;
+	__le64 fs_root_gen;
+
+	__le64 dev_root;
+	__le64 dev_root_gen;
+
+	__le64 csum_root;
+	__le64 csum_root_gen;
+
+	__le64 total_bytes;
+	__le64 bytes_used;
+	__le64 num_devices;
+	/* future */
+	__le64 unused_64[4];
+
+	__u8 tree_root_level;
+	__u8 chunk_root_level;
+	__u8 extent_root_level;
+	__u8 fs_root_level;
+	__u8 dev_root_level;
+	__u8 csum_root_level;
+	/* future and to align */
+	__u8 unused_8[10];
+} __attribute__ ((__packed__));
+
+/*
+ * A leaf is full of items. offset and size tell us where to find
+ * the item in the leaf (relative to the start of the data area)
+ */
+struct btrfs_item {
+	struct btrfs_disk_key key;
+	__le32 offset;
+	__le32 size;
+} __attribute__ ((__packed__));
+
+/*
+ * leaves have an item area and a data area:
+ * [item0, item1....itemN] [free space] [dataN...data1, data0]
+ *
+ * The data is separate from the items to get the keys closer together
+ * during searches.
+ */
+struct btrfs_leaf {
+	struct btrfs_header header;
+	struct btrfs_item items[];
+} __attribute__ ((__packed__));
+
+/*
+ * all non-leaf blocks are nodes, they hold only keys and pointers to
+ * other blocks
+ */
+struct btrfs_key_ptr {
+	struct btrfs_disk_key key;
+	__le64 blockptr;
+	__le64 generation;
+} __attribute__ ((__packed__));
+
+struct btrfs_node {
+	struct btrfs_header header;
+	struct btrfs_key_ptr ptrs[];
+} __attribute__ ((__packed__));
+
 struct btrfs_dev_item {
 	/* the internal btrfs device id */
 	__le64 devid;
@@ -472,6 +615,68 @@ struct btrfs_chunk {
 	/* additional stripes go here */
 } __attribute__ ((__packed__));
 
+/*
+ * the super block basically lists the main trees of the FS
+ * it currently lacks any block count etc etc
+ */
+struct btrfs_super_block {
+	/* the first 4 fields must match struct btrfs_header */
+	__u8 csum[BTRFS_CSUM_SIZE];
+	/* FS specific UUID, visible to user */
+	__u8 fsid[BTRFS_FSID_SIZE];
+	__le64 bytenr; /* this block number */
+	__le64 flags;
+
+	/* allowed to be different from the btrfs_header from here own down */
+	__le64 magic;
+	__le64 generation;
+	__le64 root;
+	__le64 chunk_root;
+	__le64 log_root;
+
+	/*
+	 * This member has never been utilized since the very beginning, thus
+	 * it's always 0 regardless of kernel version.  We always use
+	 * generation + 1 to read log tree root.  So here we mark it deprecated.
+	 */
+	__le64 __unused_log_root_transid;
+	__le64 total_bytes;
+	__le64 bytes_used;
+	__le64 root_dir_objectid;
+	__le64 num_devices;
+	__le32 sectorsize;
+	__le32 nodesize;
+	__le32 __unused_leafsize;
+	__le32 stripesize;
+	__le32 sys_chunk_array_size;
+	__le64 chunk_root_generation;
+	__le64 compat_flags;
+	__le64 compat_ro_flags;
+	__le64 incompat_flags;
+	__le16 csum_type;
+	__u8 root_level;
+	__u8 chunk_root_level;
+	__u8 log_root_level;
+	struct btrfs_dev_item dev_item;
+
+	char label[BTRFS_LABEL_SIZE];
+
+	__le64 cache_generation;
+	__le64 uuid_tree_generation;
+
+	/* the UUID written into btree blocks */
+	__u8 metadata_uuid[BTRFS_FSID_SIZE];
+
+	/* future expansion */
+	__u8 reserved8[8];
+	__le64 reserved[27];
+	__u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
+	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
+
+	/* Padded to 4096 bytes */
+	__u8 padding[565];
+} __attribute__ ((__packed__));
+
 #define BTRFS_FREE_SPACE_EXTENT	1
 #define BTRFS_FREE_SPACE_BITMAP	2
 
@@ -526,6 +731,14 @@ struct btrfs_extent_item_v0 {
 /* use full backrefs for extent pointers in the block */
 #define BTRFS_BLOCK_FLAG_FULL_BACKREF	(1ULL << 8)
 
+#define BTRFS_BACKREF_REV_MAX		256
+#define BTRFS_BACKREF_REV_SHIFT		56
+#define BTRFS_BACKREF_REV_MASK		(((u64)BTRFS_BACKREF_REV_MAX - 1) << \
+					 BTRFS_BACKREF_REV_SHIFT)
+
+#define BTRFS_OLD_BACKREF_REV		0
+#define BTRFS_MIXED_BACKREF_REV		1
+
 /*
  * this flag is only used internally by scrub and may be changed at any time
  * it is only declared here to avoid collisions
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (3 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:10   ` Qu Wenruo
  2022-09-15 14:14   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h Josef Bacik
                   ` (12 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This inline helper calls btrfs_fs_compat_ro(), which is defined in
another header.  To avoid weird header dependency problems move this
helper into disk-io.c with the rest of the global root helpers.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/disk-io.c | 7 +++++++
 fs/btrfs/disk-io.h | 8 +-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a887fe67a2a0..d32aa67f962b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1169,6 +1169,13 @@ struct btrfs_root *btrfs_extent_root(struct btrfs_fs_info *fs_info, u64 bytenr)
 	return btrfs_global_root(fs_info, &key);
 }
 
+struct btrfs_root *btrfs_block_group_root(struct btrfs_fs_info *fs_info)
+{
+	if (btrfs_fs_compat_ro(fs_info, BLOCK_GROUP_TREE))
+		return fs_info->block_group_root;
+	return btrfs_extent_root(fs_info, 0);
+}
+
 struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 				     u64 objectid)
 {
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index 7e545ec09a10..084fbe5768e1 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -72,6 +72,7 @@ struct btrfs_root *btrfs_global_root(struct btrfs_fs_info *fs_info,
 				     struct btrfs_key *key);
 struct btrfs_root *btrfs_csum_root(struct btrfs_fs_info *fs_info, u64 bytenr);
 struct btrfs_root *btrfs_extent_root(struct btrfs_fs_info *fs_info, u64 bytenr);
+struct btrfs_root *btrfs_block_group_root(struct btrfs_fs_info *fs_info);
 
 void btrfs_free_fs_info(struct btrfs_fs_info *fs_info);
 int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info);
@@ -103,13 +104,6 @@ static inline struct btrfs_root *btrfs_grab_root(struct btrfs_root *root)
 	return NULL;
 }
 
-static inline struct btrfs_root *btrfs_block_group_root(struct btrfs_fs_info *fs_info)
-{
-	if (btrfs_fs_compat_ro(fs_info, BLOCK_GROUP_TREE))
-		return fs_info->block_group_root;
-	return btrfs_extent_root(fs_info, 0);
-}
-
 void btrfs_put_root(struct btrfs_root *root);
 void btrfs_mark_buffer_dirty(struct extent_buffer *buf);
 int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (4 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:10   ` Qu Wenruo
  2022-09-15 14:15   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c Josef Bacik
                   ` (11 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

We have maximum link and name length limits, move these to btrfs_tree.h
as they're on disk limitations.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h                | 13 -------------
 include/uapi/linux/btrfs_tree.h | 13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index c3a8440d3223..5e6b025c0870 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -63,19 +63,6 @@ struct btrfs_ioctl_encoded_io_args;
 
 #define BTRFS_OLDEST_GENERATION	0ULL
 
-/*
- * we can actually store much bigger names, but lets not confuse the rest
- * of linux
- */
-#define BTRFS_NAME_LEN 255
-
-/*
- * Theoretical limit is larger, but we keep this down to a sane
- * value. That should limit greatly the possibility of collisions on
- * inode ref items.
- */
-#define BTRFS_LINK_MAX 65535U
-
 #define BTRFS_EMPTY_DIR_SIZE 0
 
 #define BTRFS_DIRTY_METADATA_THRESH	SZ_32M
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index e6bf902b9c92..c85e8c44ab92 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -14,6 +14,19 @@
 
 #define BTRFS_MAX_LEVEL 8
 
+/*
+ * we can actually store much bigger names, but lets not confuse the rest
+ * of linux
+ */
+#define BTRFS_NAME_LEN 255
+
+/*
+ * Theoretical limit is larger, but we keep this down to a sane
+ * value. That should limit greatly the possibility of collisions on
+ * inode ref items.
+ */
+#define BTRFS_LINK_MAX 65535U
+
 /*
  * This header contains the structure definitions and constants used
  * by file system objects that can be retrieved using
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (5 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:11   ` Qu Wenruo
  2022-09-15 14:16   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h Josef Bacik
                   ` (10 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This is only used locally in scrub.c, move it out of ctree.h into
scrub.c.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h | 11 -----------
 fs/btrfs/scrub.c | 11 +++++++++++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5e6b025c0870..e1ec047deff6 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -50,17 +50,6 @@ struct btrfs_ref;
 struct btrfs_bio;
 struct btrfs_ioctl_encoded_io_args;
 
-/*
- * Maximum number of mirrors that can be available for all profiles counting
- * the target device of dev-replace as one. During an active device replace
- * procedure, the target device of the copy operation is a mirror for the
- * filesystem data as well that can be used to read data in order to repair
- * read errors on other disks.
- *
- * Current value is derived from RAID1C4 with 4 copies.
- */
-#define BTRFS_MAX_MIRRORS (4 + 1)
-
 #define BTRFS_OLDEST_GENERATION	0ULL
 
 #define BTRFS_EMPTY_DIR_SIZE 0
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 9b6a0adccc7b..35fca65f0f2a 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -56,6 +56,17 @@ struct scrub_ctx;
 
 #define SCRUB_MAX_PAGES			(DIV_ROUND_UP(BTRFS_MAX_METADATA_BLOCKSIZE, PAGE_SIZE))
 
+/*
+ * Maximum number of mirrors that can be available for all profiles counting
+ * the target device of dev-replace as one. During an active device replace
+ * procedure, the target device of the copy operation is a mirror for the
+ * filesystem data as well that can be used to read data in order to repair
+ * read errors on other disks.
+ *
+ * Current value is derived from RAID1C4 with 4 copies.
+ */
+#define BTRFS_MAX_MIRRORS (4 + 1)
+
 struct scrub_recover {
 	refcount_t		refs;
 	struct btrfs_io_context	*bioc;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (6 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:13   ` Qu Wenruo
                     ` (2 more replies)
  2022-09-14 15:06 ` [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h Josef Bacik
                   ` (9 subsequent siblings)
  17 siblings, 3 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

These definitions are used for discard statistics, move them out of
ctree.h and put them in free-space-cache.h.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h            | 9 ---------
 fs/btrfs/free-space-cache.h | 9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e1ec047deff6..2e6a947a48de 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -58,15 +58,6 @@ struct btrfs_ioctl_encoded_io_args;
 
 #define BTRFS_MAX_EXTENT_SIZE SZ_128M
 
-/*
- * Deltas are an effective way to populate global statistics.  Give macro names
- * to make it clear what we're doing.  An example is discard_extents in
- * btrfs_free_space_ctl.
- */
-#define BTRFS_STAT_NR_ENTRIES	2
-#define BTRFS_STAT_CURR		0
-#define BTRFS_STAT_PREV		1
-
 static inline unsigned long btrfs_chunk_item_size(int num_stripes)
 {
 	BUG_ON(num_stripes == 0);
diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index 6d419ba53e95..eaf30f6444dd 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -43,6 +43,15 @@ static inline bool btrfs_free_space_trimming_bitmap(
 	return (info->trim_state == BTRFS_TRIM_STATE_TRIMMING);
 }
 
+/*
+ * Deltas are an effective way to populate global statistics.  Give macro names
+ * to make it clear what we're doing.  An example is discard_extents in
+ * btrfs_free_space_ctl.
+ */
+#define BTRFS_STAT_NR_ENTRIES	2
+#define BTRFS_STAT_CURR		0
+#define BTRFS_STAT_PREV		1
+
 struct btrfs_free_space_ctl {
 	spinlock_t tree_lock;
 	struct rb_root free_space_offset;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (7 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:14   ` Qu Wenruo
  2022-09-15 14:19   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c Josef Bacik
                   ` (8 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This is more of a volumes related helper, additionally it has a BUG_ON()
which isn't defined in the related header.  Move the code to volumes.c,
change the BUG_ON() to an ASSERT(), and move the prototype to volumes.h.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h   | 7 -------
 fs/btrfs/volumes.c | 7 +++++++
 fs/btrfs/volumes.h | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 2e6a947a48de..60f8817f5b7c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -58,13 +58,6 @@ struct btrfs_ioctl_encoded_io_args;
 
 #define BTRFS_MAX_EXTENT_SIZE SZ_128M
 
-static inline unsigned long btrfs_chunk_item_size(int num_stripes)
-{
-	BUG_ON(num_stripes == 0);
-	return sizeof(struct btrfs_chunk) +
-		sizeof(struct btrfs_stripe) * (num_stripes - 1);
-}
-
 /*
  * Runtime (in-memory) states of filesystem
  */
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 94ba46d57920..b4de4d5ed69f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -160,6 +160,13 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 	},
 };
 
+unsigned long btrfs_chunk_item_size(int num_stripes)
+{
+	ASSERT(num_stripes);
+	return sizeof(struct btrfs_chunk) +
+		sizeof(struct btrfs_stripe) * (num_stripes - 1);
+}
+
 /*
  * Convert block group flags (BTRFS_BLOCK_GROUP_*) to btrfs_raid_types, which
  * can be used as index to access btrfs_raid_array[].
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index f19a1cd1bfcf..96a7b437ff20 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -730,5 +730,5 @@ int btrfs_bg_type_to_factor(u64 flags);
 const char *btrfs_bg_type_to_raid_name(u64 flags);
 int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
 bool btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical);
-
+unsigned long btrfs_chunk_item_size(int num_stripes);
 #endif
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (8 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:16   ` Qu Wenruo
  2022-09-15 14:21   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 11/17] btrfs: move flush related definitions to space-info.h Josef Bacik
                   ` (7 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This function uses functions that are not defined in block-group.h, move
it into block-group.c in order to keep the header clean.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/block-group.c | 12 ++++++++++++
 fs/btrfs/block-group.h | 11 +----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index c52b6e245b9a..c91f47a45b06 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -18,6 +18,18 @@
 #include "raid56.h"
 #include "zoned.h"
 
+#ifdef CONFIG_BTRFS_DEBUG
+int btrfs_should_fragment_free_space(struct btrfs_block_group *block_group)
+{
+	struct btrfs_fs_info *fs_info = block_group->fs_info;
+
+	return (btrfs_test_opt(fs_info, FRAGMENT_METADATA) &&
+		block_group->flags & BTRFS_BLOCK_GROUP_METADATA) ||
+	       (btrfs_test_opt(fs_info, FRAGMENT_DATA) &&
+		block_group->flags &  BTRFS_BLOCK_GROUP_DATA);
+}
+#endif
+
 /*
  * Return target flags in extended format or 0 if restripe for this chunk_type
  * is not in progress
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index 4d4d2e1f137b..e34cb80ffb25 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -242,16 +242,7 @@ static inline bool btrfs_is_block_group_data_only(
 }
 
 #ifdef CONFIG_BTRFS_DEBUG
-static inline int btrfs_should_fragment_free_space(
-		struct btrfs_block_group *block_group)
-{
-	struct btrfs_fs_info *fs_info = block_group->fs_info;
-
-	return (btrfs_test_opt(fs_info, FRAGMENT_METADATA) &&
-		block_group->flags & BTRFS_BLOCK_GROUP_METADATA) ||
-	       (btrfs_test_opt(fs_info, FRAGMENT_DATA) &&
-		block_group->flags &  BTRFS_BLOCK_GROUP_DATA);
-}
+int btrfs_should_fragment_free_space(struct btrfs_block_group *block_group);
 #endif
 
 struct btrfs_block_group *btrfs_lookup_first_block_group(
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 11/17] btrfs: move flush related definitions to space-info.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (9 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:21   ` Qu Wenruo
  2022-09-15 14:21   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c Josef Bacik
                   ` (6 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This code is used in space-info.c, move the definitions to space-info.h.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h         | 59 ----------------------------------------
 fs/btrfs/delayed-inode.c |  1 +
 fs/btrfs/inode-item.c    |  1 +
 fs/btrfs/props.c         |  1 +
 fs/btrfs/relocation.c    |  1 +
 fs/btrfs/space-info.h    | 59 ++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/super.c         |  1 +
 7 files changed, 64 insertions(+), 59 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 60f8817f5b7c..d99720cf4835 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2654,65 +2654,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
 
 void btrfs_clear_space_info_full(struct btrfs_fs_info *info);
 
-/*
- * Different levels for to flush space when doing space reservations.
- *
- * The higher the level, the more methods we try to reclaim space.
- */
-enum btrfs_reserve_flush_enum {
-	/* If we are in the transaction, we can't flush anything.*/
-	BTRFS_RESERVE_NO_FLUSH,
-
-	/*
-	 * Flush space by:
-	 * - Running delayed inode items
-	 * - Allocating a new chunk
-	 */
-	BTRFS_RESERVE_FLUSH_LIMIT,
-
-	/*
-	 * Flush space by:
-	 * - Running delayed inode items
-	 * - Running delayed refs
-	 * - Running delalloc and waiting for ordered extents
-	 * - Allocating a new chunk
-	 */
-	BTRFS_RESERVE_FLUSH_EVICT,
-
-	/*
-	 * Flush space by above mentioned methods and by:
-	 * - Running delayed iputs
-	 * - Committing transaction
-	 *
-	 * Can be interrupted by a fatal signal.
-	 */
-	BTRFS_RESERVE_FLUSH_DATA,
-	BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE,
-	BTRFS_RESERVE_FLUSH_ALL,
-
-	/*
-	 * Pretty much the same as FLUSH_ALL, but can also steal space from
-	 * global rsv.
-	 *
-	 * Can be interrupted by a fatal signal.
-	 */
-	BTRFS_RESERVE_FLUSH_ALL_STEAL,
-};
-
-enum btrfs_flush_state {
-	FLUSH_DELAYED_ITEMS_NR	=	1,
-	FLUSH_DELAYED_ITEMS	=	2,
-	FLUSH_DELAYED_REFS_NR	=	3,
-	FLUSH_DELAYED_REFS	=	4,
-	FLUSH_DELALLOC		=	5,
-	FLUSH_DELALLOC_WAIT	=	6,
-	FLUSH_DELALLOC_FULL	=	7,
-	ALLOC_CHUNK		=	8,
-	ALLOC_CHUNK_FORCE	=	9,
-	RUN_DELAYED_IPUTS	=	10,
-	COMMIT_TRANS		=	11,
-};
-
 int btrfs_subvolume_reserve_metadata(struct btrfs_root *root,
 				     struct btrfs_block_rsv *rsv,
 				     int nitems, bool use_global_rsv);
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index cac5169eaf8d..a411f04a7b97 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -14,6 +14,7 @@
 #include "qgroup.h"
 #include "locking.h"
 #include "inode-item.h"
+#include "space-info.h"
 
 #define BTRFS_DELAYED_WRITEBACK		512
 #define BTRFS_DELAYED_BACKGROUND	128
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index 0eeb5ea87894..366f3a788c6a 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -8,6 +8,7 @@
 #include "disk-io.h"
 #include "transaction.h"
 #include "print-tree.h"
+#include "space-info.h"
 
 struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
 						   int slot, const char *name,
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index 055a631276ce..07f62e3ba6a5 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -10,6 +10,7 @@
 #include "ctree.h"
 #include "xattr.h"
 #include "compression.h"
+#include "space-info.h"
 
 #define BTRFS_PROP_HANDLERS_HT_BITS 8
 static DEFINE_HASHTABLE(prop_handlers_ht, BTRFS_PROP_HANDLERS_HT_BITS);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d87020ae5810..41adbfa3a5f6 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -27,6 +27,7 @@
 #include "subpage.h"
 #include "zoned.h"
 #include "inode-item.h"
+#include "space-info.h"
 
 /*
  * Relocation overview
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index 8f5948740941..729820582fa7 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -5,6 +5,65 @@
 
 #include "volumes.h"
 
+/*
+ * Different levels for to flush space when doing space reservations.
+ *
+ * The higher the level, the more methods we try to reclaim space.
+ */
+enum btrfs_reserve_flush_enum {
+	/* If we are in the transaction, we can't flush anything.*/
+	BTRFS_RESERVE_NO_FLUSH,
+
+	/*
+	 * Flush space by:
+	 * - Running delayed inode items
+	 * - Allocating a new chunk
+	 */
+	BTRFS_RESERVE_FLUSH_LIMIT,
+
+	/*
+	 * Flush space by:
+	 * - Running delayed inode items
+	 * - Running delayed refs
+	 * - Running delalloc and waiting for ordered extents
+	 * - Allocating a new chunk
+	 */
+	BTRFS_RESERVE_FLUSH_EVICT,
+
+	/*
+	 * Flush space by above mentioned methods and by:
+	 * - Running delayed iputs
+	 * - Committing transaction
+	 *
+	 * Can be interrupted by a fatal signal.
+	 */
+	BTRFS_RESERVE_FLUSH_DATA,
+	BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE,
+	BTRFS_RESERVE_FLUSH_ALL,
+
+	/*
+	 * Pretty much the same as FLUSH_ALL, but can also steal space from
+	 * global rsv.
+	 *
+	 * Can be interrupted by a fatal signal.
+	 */
+	BTRFS_RESERVE_FLUSH_ALL_STEAL,
+};
+
+enum btrfs_flush_state {
+	FLUSH_DELAYED_ITEMS_NR	=	1,
+	FLUSH_DELAYED_ITEMS	=	2,
+	FLUSH_DELAYED_REFS_NR	=	3,
+	FLUSH_DELAYED_REFS	=	4,
+	FLUSH_DELALLOC		=	5,
+	FLUSH_DELALLOC_WAIT	=	6,
+	FLUSH_DELALLOC_FULL	=	7,
+	ALLOC_CHUNK		=	8,
+	ALLOC_CHUNK_FORCE	=	9,
+	RUN_DELAYED_IPUTS	=	10,
+	COMMIT_TRANS		=	11,
+};
+
 struct btrfs_space_info {
 	spinlock_t lock;
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index be7df8d1d5b8..2add5b23c476 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -49,6 +49,7 @@
 #include "discard.h"
 #include "qgroup.h"
 #include "raid56.h"
+#include "space-info.h"
 #define CREATE_TRACE_POINTS
 #include <trace/events/btrfs.h>
 
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (10 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 11/17] btrfs: move flush related definitions to space-info.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:22   ` Qu Wenruo
  2022-09-15 14:23   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h Josef Bacik
                   ` (5 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This isn't used outside of inode.c, there's no reason to define it in
btrfs_inode.h.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/btrfs_inode.h | 26 --------------------------
 fs/btrfs/inode.c       | 25 +++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 108af52ba870..890c9f979a3d 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -426,30 +426,4 @@ static inline void btrfs_inode_split_flags(u64 inode_item_flags,
 /* Array of bytes with variable length, hexadecimal format 0x1234 */
 #define CSUM_FMT				"0x%*phN"
 #define CSUM_FMT_VALUE(size, bytes)		size, bytes
-
-static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
-		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
-{
-	struct btrfs_root *root = inode->root;
-	const u32 csum_size = root->fs_info->csum_size;
-
-	/* Output minus objectid, which is more meaningful */
-	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
-		btrfs_warn_rl(root->fs_info,
-"csum failed root %lld ino %lld off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d",
-			root->root_key.objectid, btrfs_ino(inode),
-			logical_start,
-			CSUM_FMT_VALUE(csum_size, csum),
-			CSUM_FMT_VALUE(csum_size, csum_expected),
-			mirror_num);
-	else
-		btrfs_warn_rl(root->fs_info,
-"csum failed root %llu ino %llu off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d",
-			root->root_key.objectid, btrfs_ino(inode),
-			logical_start,
-			CSUM_FMT_VALUE(csum_size, csum),
-			CSUM_FMT_VALUE(csum_size, csum_expected),
-			mirror_num);
-}
-
 #endif
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 6fde13f62c1d..998d1c7134ff 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -125,6 +125,31 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start,
 				       u64 ram_bytes, int compress_type,
 				       int type);
 
+static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
+		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
+{
+	struct btrfs_root *root = inode->root;
+	const u32 csum_size = root->fs_info->csum_size;
+
+	/* Output minus objectid, which is more meaningful */
+	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
+		btrfs_warn_rl(root->fs_info,
+"csum failed root %lld ino %lld off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d",
+			root->root_key.objectid, btrfs_ino(inode),
+			logical_start,
+			CSUM_FMT_VALUE(csum_size, csum),
+			CSUM_FMT_VALUE(csum_size, csum_expected),
+			mirror_num);
+	else
+		btrfs_warn_rl(root->fs_info,
+"csum failed root %llu ino %llu off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d",
+			root->root_key.objectid, btrfs_ino(inode),
+			logical_start,
+			CSUM_FMT_VALUE(csum_size, csum),
+			CSUM_FMT_VALUE(csum_size, csum_expected),
+			mirror_num);
+}
+
 /*
  * btrfs_inode_lock - lock inode i_rwsem based on arguments passed
  *
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (11 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:23   ` Qu Wenruo
  2022-09-15 14:24   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 14/17] btrfs: move btrfs_path_cachep " Josef Bacik
                   ` (4 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This is local to the transaction code, remove it from ctree.h and
inode.c, create new helpers in the transaction to handle the init work
and move the cachep locally to transaction.c.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h       |  1 -
 fs/btrfs/inode.c       |  8 --------
 fs/btrfs/super.c       |  9 ++++++++-
 fs/btrfs/transaction.c | 17 +++++++++++++++++
 fs/btrfs/transaction.h |  2 ++
 5 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d99720cf4835..439b205f4207 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -41,7 +41,6 @@ struct btrfs_pending_snapshot;
 struct btrfs_delayed_ref_root;
 struct btrfs_space_info;
 struct btrfs_block_group;
-extern struct kmem_cache *btrfs_trans_handle_cachep;
 extern struct kmem_cache *btrfs_path_cachep;
 extern struct kmem_cache *btrfs_free_space_cachep;
 extern struct kmem_cache *btrfs_free_space_bitmap_cachep;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 998d1c7134ff..78e7f5397d58 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -107,7 +107,6 @@ static const struct address_space_operations btrfs_aops;
 static const struct file_operations btrfs_dir_file_operations;
 
 static struct kmem_cache *btrfs_inode_cachep;
-struct kmem_cache *btrfs_trans_handle_cachep;
 struct kmem_cache *btrfs_path_cachep;
 struct kmem_cache *btrfs_free_space_cachep;
 struct kmem_cache *btrfs_free_space_bitmap_cachep;
@@ -8938,7 +8937,6 @@ void __cold btrfs_destroy_cachep(void)
 	rcu_barrier();
 	bioset_exit(&btrfs_dio_bioset);
 	kmem_cache_destroy(btrfs_inode_cachep);
-	kmem_cache_destroy(btrfs_trans_handle_cachep);
 	kmem_cache_destroy(btrfs_path_cachep);
 	kmem_cache_destroy(btrfs_free_space_cachep);
 	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
@@ -8953,12 +8951,6 @@ int __init btrfs_init_cachep(void)
 	if (!btrfs_inode_cachep)
 		goto fail;
 
-	btrfs_trans_handle_cachep = kmem_cache_create("btrfs_trans_handle",
-			sizeof(struct btrfs_trans_handle), 0,
-			SLAB_TEMPORARY | SLAB_MEM_SPREAD, NULL);
-	if (!btrfs_trans_handle_cachep)
-		goto fail;
-
 	btrfs_path_cachep = kmem_cache_create("btrfs_path",
 			sizeof(struct btrfs_path), 0,
 			SLAB_MEM_SPREAD, NULL);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 2add5b23c476..9f7fc1c71148 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2737,10 +2737,14 @@ static int __init init_btrfs_fs(void)
 	if (err)
 		goto free_compress;
 
-	err = extent_state_init_cachep();
+	err = btrfs_transaction_init();
 	if (err)
 		goto free_cachep;
 
+	err = extent_state_init_cachep();
+	if (err)
+		goto free_transaction;
+
 	err = extent_buffer_init_cachep();
 	if (err)
 		goto free_extent_cachep;
@@ -2809,6 +2813,8 @@ static int __init init_btrfs_fs(void)
 	extent_buffer_free_cachep();
 free_extent_cachep:
 	extent_state_free_cachep();
+free_transaction:
+	btrfs_transaction_exit();
 free_cachep:
 	btrfs_destroy_cachep();
 free_compress:
@@ -2820,6 +2826,7 @@ static int __init init_btrfs_fs(void)
 
 static void __exit exit_btrfs_fs(void)
 {
+	btrfs_transaction_exit();
 	btrfs_destroy_cachep();
 	btrfs_delayed_ref_exit();
 	btrfs_auto_defrag_exit();
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index d1f1da6820fb..ae7d4aca771d 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -24,6 +24,8 @@
 #include "space-info.h"
 #include "zoned.h"
 
+static struct kmem_cache *btrfs_trans_handle_cachep;
+
 #define BTRFS_ROOT_TRANS_TAG 0
 
 /*
@@ -2600,3 +2602,18 @@ void btrfs_apply_pending_changes(struct btrfs_fs_info *fs_info)
 		btrfs_warn(fs_info,
 			"unknown pending changes left 0x%lx, ignoring", prev);
 }
+
+int __init btrfs_transaction_init(void)
+{
+	btrfs_trans_handle_cachep = kmem_cache_create("btrfs_trans_handle",
+			sizeof(struct btrfs_trans_handle), 0,
+			SLAB_TEMPORARY | SLAB_MEM_SPREAD, NULL);
+	if (!btrfs_trans_handle_cachep)
+		return -ENOMEM;
+	return 0;
+}
+
+void __cold btrfs_transaction_exit(void)
+{
+	kmem_cache_destroy(btrfs_trans_handle_cachep);
+}
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 970ff316069d..b5651c372946 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -236,4 +236,6 @@ void btrfs_add_dropped_root(struct btrfs_trans_handle *trans,
 			    struct btrfs_root *root);
 void btrfs_trans_release_chunk_metadata(struct btrfs_trans_handle *trans);
 
+int __init btrfs_transaction_init(void);
+void __cold btrfs_transaction_exit(void);
 #endif
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 14/17] btrfs: move btrfs_path_cachep out of ctree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (12 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:27   ` Qu Wenruo
  2022-09-15 14:27   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 15/17] btrfs: move free space cachep's " Josef Bacik
                   ` (3 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This is local to the ctree code, remove it from ctree.h and inode.c,
create new init/exit functions for the cachep, and move it locally to
ctree.c.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.c | 17 +++++++++++++++++
 fs/btrfs/ctree.h |  3 ++-
 fs/btrfs/inode.c |  8 --------
 fs/btrfs/super.c |  9 ++++++++-
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index ebfa35fe1c38..1f0355c74fe6 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -18,6 +18,8 @@
 #include "tree-mod-log.h"
 #include "tree-checker.h"
 
+static struct kmem_cache *btrfs_path_cachep;
+
 static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
 		      *root, struct btrfs_path *path, int level);
 static int split_leaf(struct btrfs_trans_handle *trans, struct btrfs_root *root,
@@ -4856,3 +4858,18 @@ int btrfs_previous_extent_item(struct btrfs_root *root,
 	}
 	return 1;
 }
+
+int __init btrfs_ctree_init(void)
+{
+	btrfs_path_cachep = kmem_cache_create("btrfs_path",
+			sizeof(struct btrfs_path), 0,
+			SLAB_MEM_SPREAD, NULL);
+	if (!btrfs_path_cachep)
+		return -ENOMEM;
+	return 0;
+}
+
+void __cold btrfs_ctree_exit(void)
+{
+	kmem_cache_destroy(btrfs_path_cachep);
+}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 439b205f4207..3a61f5c0ab5f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -41,7 +41,6 @@ struct btrfs_pending_snapshot;
 struct btrfs_delayed_ref_root;
 struct btrfs_space_info;
 struct btrfs_block_group;
-extern struct kmem_cache *btrfs_path_cachep;
 extern struct kmem_cache *btrfs_free_space_cachep;
 extern struct kmem_cache *btrfs_free_space_bitmap_cachep;
 struct btrfs_ordered_sum;
@@ -2677,6 +2676,8 @@ void btrfs_end_write_no_snapshotting(struct btrfs_root *root);
 void btrfs_wait_for_snapshot_creation(struct btrfs_root *root);
 
 /* ctree.c */
+int __init btrfs_ctree_init(void);
+void __cold btrfs_ctree_exit(void);
 int btrfs_bin_search(struct extent_buffer *eb, const struct btrfs_key *key,
 		     int *slot);
 int __pure btrfs_comp_cpu_keys(const struct btrfs_key *k1, const struct btrfs_key *k2);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 78e7f5397d58..1401e2da9284 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -107,7 +107,6 @@ static const struct address_space_operations btrfs_aops;
 static const struct file_operations btrfs_dir_file_operations;
 
 static struct kmem_cache *btrfs_inode_cachep;
-struct kmem_cache *btrfs_path_cachep;
 struct kmem_cache *btrfs_free_space_cachep;
 struct kmem_cache *btrfs_free_space_bitmap_cachep;
 
@@ -8937,7 +8936,6 @@ void __cold btrfs_destroy_cachep(void)
 	rcu_barrier();
 	bioset_exit(&btrfs_dio_bioset);
 	kmem_cache_destroy(btrfs_inode_cachep);
-	kmem_cache_destroy(btrfs_path_cachep);
 	kmem_cache_destroy(btrfs_free_space_cachep);
 	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
 }
@@ -8951,12 +8949,6 @@ int __init btrfs_init_cachep(void)
 	if (!btrfs_inode_cachep)
 		goto fail;
 
-	btrfs_path_cachep = kmem_cache_create("btrfs_path",
-			sizeof(struct btrfs_path), 0,
-			SLAB_MEM_SPREAD, NULL);
-	if (!btrfs_path_cachep)
-		goto fail;
-
 	btrfs_free_space_cachep = kmem_cache_create("btrfs_free_space",
 			sizeof(struct btrfs_free_space), 0,
 			SLAB_MEM_SPREAD, NULL);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 9f7fc1c71148..acd590bed579 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2741,10 +2741,14 @@ static int __init init_btrfs_fs(void)
 	if (err)
 		goto free_cachep;
 
-	err = extent_state_init_cachep();
+	err = btrfs_ctree_init();
 	if (err)
 		goto free_transaction;
 
+	err = extent_state_init_cachep();
+	if (err)
+		goto free_ctree;
+
 	err = extent_buffer_init_cachep();
 	if (err)
 		goto free_extent_cachep;
@@ -2813,6 +2817,8 @@ static int __init init_btrfs_fs(void)
 	extent_buffer_free_cachep();
 free_extent_cachep:
 	extent_state_free_cachep();
+free_ctree:
+	btrfs_ctree_exit();
 free_transaction:
 	btrfs_transaction_exit();
 free_cachep:
@@ -2826,6 +2832,7 @@ static int __init init_btrfs_fs(void)
 
 static void __exit exit_btrfs_fs(void)
 {
+	btrfs_ctree_exit();
 	btrfs_transaction_exit();
 	btrfs_destroy_cachep();
 	btrfs_delayed_ref_exit();
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 15/17] btrfs: move free space cachep's out of ctree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (13 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 14/17] btrfs: move btrfs_path_cachep " Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:27   ` Qu Wenruo
  2022-09-15 14:27   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c Josef Bacik
                   ` (2 subsequent siblings)
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This is local to the free-space-cache.c code, remove it from ctree.h and
inode.c, create new init/exit functions for the cachep, and move it
locally to free-space-cache.c.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h            |  2 --
 fs/btrfs/free-space-cache.c | 28 ++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |  2 ++
 fs/btrfs/inode.c            | 16 ----------------
 fs/btrfs/super.c            |  9 ++++++++-
 5 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3a61f5c0ab5f..af6f6764d9a4 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -41,8 +41,6 @@ struct btrfs_pending_snapshot;
 struct btrfs_delayed_ref_root;
 struct btrfs_space_info;
 struct btrfs_block_group;
-extern struct kmem_cache *btrfs_free_space_cachep;
-extern struct kmem_cache *btrfs_free_space_bitmap_cachep;
 struct btrfs_ordered_sum;
 struct btrfs_ref;
 struct btrfs_bio;
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 7859eeca484c..ee03c5e6db4c 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -29,6 +29,9 @@
 #define MAX_CACHE_BYTES_PER_GIG	SZ_64K
 #define FORCE_EXTENT_THRESHOLD	SZ_1M
 
+static struct kmem_cache *btrfs_free_space_cachep;
+static struct kmem_cache *btrfs_free_space_bitmap_cachep;
+
 struct btrfs_trim_range {
 	u64 start;
 	u64 bytes;
@@ -4132,6 +4135,31 @@ int btrfs_set_free_space_cache_v1_active(struct btrfs_fs_info *fs_info, bool act
 	return ret;
 }
 
+int __init btrfs_free_space_init(void)
+{
+	btrfs_free_space_cachep = kmem_cache_create("btrfs_free_space",
+			sizeof(struct btrfs_free_space), 0,
+			SLAB_MEM_SPREAD, NULL);
+	if (!btrfs_free_space_cachep)
+		return -ENOMEM;
+
+	btrfs_free_space_bitmap_cachep = kmem_cache_create("btrfs_free_space_bitmap",
+							PAGE_SIZE, PAGE_SIZE,
+							SLAB_MEM_SPREAD, NULL);
+	if (!btrfs_free_space_bitmap_cachep) {
+		kmem_cache_destroy(btrfs_free_space_cachep);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void __cold btrfs_free_space_exit(void)
+{
+	kmem_cache_destroy(btrfs_free_space_cachep);
+	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
+}
+
 #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
 /*
  * Use this if you need to make a bitmap or extent entry specifically, it
diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index eaf30f6444dd..cab954a9d97b 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -88,6 +88,8 @@ struct btrfs_io_ctl {
 	int bitmaps;
 };
 
+int __init btrfs_free_space_init(void);
+void __cold btrfs_free_space_exit(void);
 struct inode *lookup_free_space_inode(struct btrfs_block_group *block_group,
 		struct btrfs_path *path);
 int create_free_space_inode(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1401e2da9284..da5be8f23f68 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -107,8 +107,6 @@ static const struct address_space_operations btrfs_aops;
 static const struct file_operations btrfs_dir_file_operations;
 
 static struct kmem_cache *btrfs_inode_cachep;
-struct kmem_cache *btrfs_free_space_cachep;
-struct kmem_cache *btrfs_free_space_bitmap_cachep;
 
 static int btrfs_setsize(struct inode *inode, struct iattr *attr);
 static int btrfs_truncate(struct inode *inode, bool skip_writeback);
@@ -8936,8 +8934,6 @@ void __cold btrfs_destroy_cachep(void)
 	rcu_barrier();
 	bioset_exit(&btrfs_dio_bioset);
 	kmem_cache_destroy(btrfs_inode_cachep);
-	kmem_cache_destroy(btrfs_free_space_cachep);
-	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
 }
 
 int __init btrfs_init_cachep(void)
@@ -8949,18 +8945,6 @@ int __init btrfs_init_cachep(void)
 	if (!btrfs_inode_cachep)
 		goto fail;
 
-	btrfs_free_space_cachep = kmem_cache_create("btrfs_free_space",
-			sizeof(struct btrfs_free_space), 0,
-			SLAB_MEM_SPREAD, NULL);
-	if (!btrfs_free_space_cachep)
-		goto fail;
-
-	btrfs_free_space_bitmap_cachep = kmem_cache_create("btrfs_free_space_bitmap",
-							PAGE_SIZE, PAGE_SIZE,
-							SLAB_MEM_SPREAD, NULL);
-	if (!btrfs_free_space_bitmap_cachep)
-		goto fail;
-
 	if (bioset_init(&btrfs_dio_bioset, BIO_POOL_SIZE,
 			offsetof(struct btrfs_dio_private, bio),
 			BIOSET_NEED_BVECS))
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index acd590bed579..c2e634de01e4 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2745,10 +2745,14 @@ static int __init init_btrfs_fs(void)
 	if (err)
 		goto free_transaction;
 
-	err = extent_state_init_cachep();
+	err = btrfs_free_space_init();
 	if (err)
 		goto free_ctree;
 
+	err = extent_state_init_cachep();
+	if (err)
+		goto free_free_space;
+
 	err = extent_buffer_init_cachep();
 	if (err)
 		goto free_extent_cachep;
@@ -2817,6 +2821,8 @@ static int __init init_btrfs_fs(void)
 	extent_buffer_free_cachep();
 free_extent_cachep:
 	extent_state_free_cachep();
+free_free_space:
+	btrfs_free_space_exit();
 free_ctree:
 	btrfs_ctree_exit();
 free_transaction:
@@ -2832,6 +2838,7 @@ static int __init init_btrfs_fs(void)
 
 static void __exit exit_btrfs_fs(void)
 {
+	btrfs_free_space_exit();
 	btrfs_ctree_exit();
 	btrfs_transaction_exit();
 	btrfs_destroy_cachep();
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (14 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 15/17] btrfs: move free space cachep's " Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:29   ` Qu Wenruo
  2022-09-15 14:29   ` Johannes Thumshirn
  2022-09-14 15:06 ` [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h Josef Bacik
  2022-09-15  9:47 ` [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Qu Wenruo
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

This uses btrfs_header_nritems, which I will be moving out of ctree.h.
In order to avoid needing to include the relevant header in ctree.h,
simply move this helper function into ctree.c.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.c |  9 +++++++++
 fs/btrfs/ctree.h | 10 ++--------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 1f0355c74fe6..0f7f93bc2582 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -4775,6 +4775,15 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
 	return ret;
 }
 
+int btrfs_next_old_item(struct btrfs_root *root, struct btrfs_path *p,
+			u64 time_seq)
+{
+	++p->slots[0];
+	if (p->slots[0] >= btrfs_header_nritems(p->nodes[0]))
+		return btrfs_next_old_leaf(root, p, time_seq);
+	return 0;
+}
+
 /*
  * this uses btrfs_prev_leaf to walk backwards in the tree, and keeps
  * searching until it gets past min_objectid or finds an item of 'type'
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index af6f6764d9a4..42dec21b3517 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2836,14 +2836,8 @@ int btrfs_get_next_valid_item(struct btrfs_root *root, struct btrfs_key *key,
 		(path)->slots[0]++						\
 	)
 
-static inline int btrfs_next_old_item(struct btrfs_root *root,
-				      struct btrfs_path *p, u64 time_seq)
-{
-	++p->slots[0];
-	if (p->slots[0] >= btrfs_header_nritems(p->nodes[0]))
-		return btrfs_next_old_leaf(root, p, time_seq);
-	return 0;
-}
+int btrfs_next_old_item(struct btrfs_root *root, struct btrfs_path *p,
+			u64 time_seq);
 
 /*
  * Search the tree again to find a leaf with greater keys.
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (15 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c Josef Bacik
@ 2022-09-14 15:06 ` Josef Bacik
  2022-09-15  9:30   ` Qu Wenruo
  2022-09-15 14:30   ` Johannes Thumshirn
  2022-09-15  9:47 ` [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Qu Wenruo
  17 siblings, 2 replies; 61+ messages in thread
From: Josef Bacik @ 2022-09-14 15:06 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

These are wrapped in CONFIG_FS_VERITY, but we can have the definitions
without verity enabled.  Move these definitions up with the other
accessor helpers.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 42dec21b3517..cb1ae35c1095 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2499,6 +2499,16 @@ BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_left,
 BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_right,
 			 struct btrfs_dev_replace_item, cursor_right, 64);
 
+/* btrfs_verity_descriptor_item */
+BTRFS_SETGET_FUNCS(verity_descriptor_encryption, struct btrfs_verity_descriptor_item,
+		   encryption, 8);
+BTRFS_SETGET_FUNCS(verity_descriptor_size, struct btrfs_verity_descriptor_item,
+		   size, 64);
+BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_encryption,
+			 struct btrfs_verity_descriptor_item, encryption, 8);
+BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_size,
+			 struct btrfs_verity_descriptor_item, size, 64);
+
 /* helper function to cast into the data area of the leaf. */
 #define btrfs_item_ptr(leaf, slot, type) \
 	((type *)(BTRFS_LEAF_DATA_OFFSET + \
@@ -3742,22 +3752,10 @@ static inline int btrfs_defrag_cancelled(struct btrfs_fs_info *fs_info)
 
 /* verity.c */
 #ifdef CONFIG_FS_VERITY
-
 extern const struct fsverity_operations btrfs_verityops;
 int btrfs_drop_verity_items(struct btrfs_inode *inode);
 int btrfs_get_verity_descriptor(struct inode *inode, void *buf, size_t buf_size);
-
-BTRFS_SETGET_FUNCS(verity_descriptor_encryption, struct btrfs_verity_descriptor_item,
-		   encryption, 8);
-BTRFS_SETGET_FUNCS(verity_descriptor_size, struct btrfs_verity_descriptor_item,
-		   size, 64);
-BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_encryption,
-			 struct btrfs_verity_descriptor_item, encryption, 8);
-BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_size,
-			 struct btrfs_verity_descriptor_item, size, 64);
-
 #else
-
 static inline int btrfs_drop_verity_items(struct btrfs_inode *inode)
 {
 	return 0;
@@ -3768,7 +3766,6 @@ static inline int btrfs_get_verity_descriptor(struct inode *inode, void *buf,
 {
 	return -EPERM;
 }
-
 #endif
 
 /* Sanity test specific functions */
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* Re: [PATCH 01/17] btrfs: remove set/clear_pending_info helpers
  2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
@ 2022-09-15  9:03   ` Qu Wenruo
  2022-09-15 14:11   ` Johannes Thumshirn
  2022-10-07 16:51   ` David Sterba
  2 siblings, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:03 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> The last users of these helpers were removed in
> 
> 5297199a8bca ("btrfs: remove inode number cache feature")
> 
> so delete these helpers.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/ctree.h | 24 ------------------------
>   1 file changed, 24 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 8b7b7a212da0..0003ba925d93 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1644,30 +1644,6 @@ do {									\
>   #define btrfs_clear_pending(info, opt)	\
>   	clear_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)
>   
> -/*
> - * Helpers for setting pending mount option changes.
> - *
> - * Expects corresponding macros
> - * BTRFS_PENDING_SET_ and CLEAR_ + short mount option name
> - */
> -#define btrfs_set_pending_and_info(info, opt, fmt, args...)            \
> -do {                                                                   \
> -       if (!btrfs_raw_test_opt((info)->mount_opt, opt)) {              \
> -               btrfs_info((info), fmt, ##args);                        \
> -               btrfs_set_pending((info), SET_##opt);                   \
> -               btrfs_clear_pending((info), CLEAR_##opt);               \
> -       }                                                               \
> -} while(0)
> -
> -#define btrfs_clear_pending_and_info(info, opt, fmt, args...)          \
> -do {                                                                   \
> -       if (btrfs_raw_test_opt((info)->mount_opt, opt)) {               \
> -               btrfs_info((info), fmt, ##args);                        \
> -               btrfs_set_pending((info), CLEAR_##opt);                 \
> -               btrfs_clear_pending((info), SET_##opt);                 \
> -       }                                                               \
> -} while(0)
> -
>   /*
>    * Inode flags
>    */

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH
  2022-09-14 15:06 ` [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH Josef Bacik
@ 2022-09-15  9:03   ` Qu Wenruo
  2022-09-15 14:11   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:03 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This hasn't been used since
> 
> 138a12d86574 ("btrfs: rip out btrfs_space_info::total_bytes_pinned")
> 
> so it is safe to remove.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>


Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.h | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 0003ba925d93..3936bb95331d 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -87,14 +87,6 @@ struct btrfs_ioctl_encoded_io_args;
>   
>   #define BTRFS_DIRTY_METADATA_THRESH	SZ_32M
>   
> -/*
> - * Use large batch size to reduce overhead of metadata updates.  On the reader
> - * side, we only read it when we are close to ENOSPC and the read overhead is
> - * mostly related to the number of CPUs, so it is OK to use arbitrary large
> - * value here.
> - */
> -#define BTRFS_TOTAL_BYTES_PINNED_BATCH	SZ_128M
> -
>   #define BTRFS_MAX_EXTENT_SIZE SZ_128M
>   
>   /*

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA
  2022-09-14 15:06 ` [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA Josef Bacik
@ 2022-09-15  9:03   ` Qu Wenruo
  2022-09-15 14:12   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:03 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> The last user of this definition was removed in patch
> 
> f26c92386028 ("btrfs: remove reada infrastructure")
> 
> so we can remove this definition.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 3936bb95331d..5cf18a120dff 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -82,9 +82,6 @@ struct btrfs_ioctl_encoded_io_args;
>   
>   #define BTRFS_EMPTY_DIR_SIZE 0
>   
> -/* ioprio of readahead is set to idle */
> -#define BTRFS_IOPRIO_READA (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_IDLE, 0))
> -
>   #define BTRFS_DIRTY_METADATA_THRESH	SZ_32M
>   
>   #define BTRFS_MAX_EXTENT_SIZE SZ_128M

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h
  2022-09-14 15:06 ` [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h Josef Bacik
@ 2022-09-15  9:07   ` Qu Wenruo
  2022-10-07 17:07     ` David Sterba
  0 siblings, 1 reply; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:07 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> The bulk of our on-disk definitions exist in btrfs_tree.h, which user
> space can use.

Previously I tried to move some members to btrfs_tree.h, but didn't get 
approved, mostly due to the fact that, we have those members exposed 
through uapi is for TREE_SEARCH ioctl.

But I'm not buying that reason at all.

To me, all on-disk format, no matter if it's exposed through tree-search 
should be in btrfs_tree.h.

Although I'd prefer to rename btrfs_tree.h to btrfs_ondisk_format.h.

Thus to David:

Can we make it clear that, btrfs_tree.h is not only for tree search 
ioctl, but also all the on-disk format thing?

Reject once that's fine, but reject twice from two different guys, I 
think it's not correct.

>  Keep things consistent and move the rest of the on disk
> definitions out of ctree.h into btrfs_tree.h.  Note I did have to update
> all u8's to __u8, but otherwise this is a strict copy and paste.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu


> ---
>   fs/btrfs/ctree.h                | 215 +-------------------------------
>   include/uapi/linux/btrfs_tree.h | 213 +++++++++++++++++++++++++++++++
>   2 files changed, 214 insertions(+), 214 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 5cf18a120dff..c3a8440d3223 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -50,8 +50,6 @@ struct btrfs_ref;
>   struct btrfs_bio;
>   struct btrfs_ioctl_encoded_io_args;
>   
> -#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
> -
>   /*
>    * Maximum number of mirrors that can be available for all profiles counting
>    * the target device of dev-replace as one. During an active device replace
> @@ -63,8 +61,6 @@ struct btrfs_ioctl_encoded_io_args;
>    */
>   #define BTRFS_MAX_MIRRORS (4 + 1)
>   
> -#define BTRFS_MAX_LEVEL 8
> -
>   #define BTRFS_OLDEST_GENERATION	0ULL
>   
>   /*
> @@ -133,81 +129,9 @@ enum {
>   	BTRFS_FS_STATE_COUNT
>   };
>   
> -#define BTRFS_BACKREF_REV_MAX		256
> -#define BTRFS_BACKREF_REV_SHIFT		56
> -#define BTRFS_BACKREF_REV_MASK		(((u64)BTRFS_BACKREF_REV_MAX - 1) << \
> -					 BTRFS_BACKREF_REV_SHIFT)
> -
> -#define BTRFS_OLD_BACKREF_REV		0
> -#define BTRFS_MIXED_BACKREF_REV		1
> -
> -/*
> - * every tree block (leaf or node) starts with this header.
> - */
> -struct btrfs_header {
> -	/* these first four must match the super block */
> -	u8 csum[BTRFS_CSUM_SIZE];
> -	u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
> -	__le64 bytenr; /* which block this node is supposed to live in */
> -	__le64 flags;
> -
> -	/* allowed to be different from the super from here on down */
> -	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
> -	__le64 generation;
> -	__le64 owner;
> -	__le32 nritems;
> -	u8 level;
> -} __attribute__ ((__packed__));
> -
> -/*
> - * this is a very generous portion of the super block, giving us
> - * room to translate 14 chunks with 3 stripes each.
> - */
> -#define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
> -
> -/*
> - * just in case we somehow lose the roots and are not able to mount,
> - * we store an array of the roots from previous transactions
> - * in the super.
> - */
> -#define BTRFS_NUM_BACKUP_ROOTS 4
> -struct btrfs_root_backup {
> -	__le64 tree_root;
> -	__le64 tree_root_gen;
> -
> -	__le64 chunk_root;
> -	__le64 chunk_root_gen;
> -
> -	__le64 extent_root;
> -	__le64 extent_root_gen;
> -
> -	__le64 fs_root;
> -	__le64 fs_root_gen;
> -
> -	__le64 dev_root;
> -	__le64 dev_root_gen;
> -
> -	__le64 csum_root;
> -	__le64 csum_root_gen;
> -
> -	__le64 total_bytes;
> -	__le64 bytes_used;
> -	__le64 num_devices;
> -	/* future */
> -	__le64 unused_64[4];
> -
> -	u8 tree_root_level;
> -	u8 chunk_root_level;
> -	u8 extent_root_level;
> -	u8 fs_root_level;
> -	u8 dev_root_level;
> -	u8 csum_root_level;
> -	/* future and to align */
> -	u8 unused_8[10];
> -} __attribute__ ((__packed__));
> -
>   #define BTRFS_SUPER_INFO_OFFSET			SZ_64K
>   #define BTRFS_SUPER_INFO_SIZE			4096
> +static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
>   
>   /*
>    * The reserved space at the beginning of each device.
> @@ -216,69 +140,6 @@ struct btrfs_root_backup {
>    */
>   #define BTRFS_DEVICE_RANGE_RESERVED			(SZ_1M)
>   
> -/*
> - * the super block basically lists the main trees of the FS
> - * it currently lacks any block count etc etc
> - */
> -struct btrfs_super_block {
> -	/* the first 4 fields must match struct btrfs_header */
> -	u8 csum[BTRFS_CSUM_SIZE];
> -	/* FS specific UUID, visible to user */
> -	u8 fsid[BTRFS_FSID_SIZE];
> -	__le64 bytenr; /* this block number */
> -	__le64 flags;
> -
> -	/* allowed to be different from the btrfs_header from here own down */
> -	__le64 magic;
> -	__le64 generation;
> -	__le64 root;
> -	__le64 chunk_root;
> -	__le64 log_root;
> -
> -	/*
> -	 * This member has never been utilized since the very beginning, thus
> -	 * it's always 0 regardless of kernel version.  We always use
> -	 * generation + 1 to read log tree root.  So here we mark it deprecated.
> -	 */
> -	__le64 __unused_log_root_transid;
> -	__le64 total_bytes;
> -	__le64 bytes_used;
> -	__le64 root_dir_objectid;
> -	__le64 num_devices;
> -	__le32 sectorsize;
> -	__le32 nodesize;
> -	__le32 __unused_leafsize;
> -	__le32 stripesize;
> -	__le32 sys_chunk_array_size;
> -	__le64 chunk_root_generation;
> -	__le64 compat_flags;
> -	__le64 compat_ro_flags;
> -	__le64 incompat_flags;
> -	__le16 csum_type;
> -	u8 root_level;
> -	u8 chunk_root_level;
> -	u8 log_root_level;
> -	struct btrfs_dev_item dev_item;
> -
> -	char label[BTRFS_LABEL_SIZE];
> -
> -	__le64 cache_generation;
> -	__le64 uuid_tree_generation;
> -
> -	/* the UUID written into btree blocks */
> -	u8 metadata_uuid[BTRFS_FSID_SIZE];
> -
> -	/* future expansion */
> -	u8 reserved8[8];
> -	__le64 reserved[27];
> -	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
> -	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
> -
> -	/* Padded to 4096 bytes */
> -	u8 padding[565];
> -} __attribute__ ((__packed__));
> -static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
> -
>   /*
>    * Compat flags that we support.  If any incompat flags are set other than the
>    * ones specified below then we will fail to mount
> @@ -336,43 +197,6 @@ static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
>   	(BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF)
>   #define BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR		0ULL
>   
> -/*
> - * A leaf is full of items. offset and size tell us where to find
> - * the item in the leaf (relative to the start of the data area)
> - */
> -struct btrfs_item {
> -	struct btrfs_disk_key key;
> -	__le32 offset;
> -	__le32 size;
> -} __attribute__ ((__packed__));
> -
> -/*
> - * leaves have an item area and a data area:
> - * [item0, item1....itemN] [free space] [dataN...data1, data0]
> - *
> - * The data is separate from the items to get the keys closer together
> - * during searches.
> - */
> -struct btrfs_leaf {
> -	struct btrfs_header header;
> -	struct btrfs_item items[];
> -} __attribute__ ((__packed__));
> -
> -/*
> - * all non-leaf blocks are nodes, they hold only keys and pointers to
> - * other blocks
> - */
> -struct btrfs_key_ptr {
> -	struct btrfs_disk_key key;
> -	__le64 blockptr;
> -	__le64 generation;
> -} __attribute__ ((__packed__));
> -
> -struct btrfs_node {
> -	struct btrfs_header header;
> -	struct btrfs_key_ptr ptrs[];
> -} __attribute__ ((__packed__));
> -
>   /* Read ahead values for struct btrfs_path.reada */
>   enum {
>   	READA_NONE,
> @@ -1633,43 +1457,6 @@ do {									\
>   #define btrfs_clear_pending(info, opt)	\
>   	clear_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)
>   
> -/*
> - * Inode flags
> - */
> -#define BTRFS_INODE_NODATASUM		(1U << 0)
> -#define BTRFS_INODE_NODATACOW		(1U << 1)
> -#define BTRFS_INODE_READONLY		(1U << 2)
> -#define BTRFS_INODE_NOCOMPRESS		(1U << 3)
> -#define BTRFS_INODE_PREALLOC		(1U << 4)
> -#define BTRFS_INODE_SYNC		(1U << 5)
> -#define BTRFS_INODE_IMMUTABLE		(1U << 6)
> -#define BTRFS_INODE_APPEND		(1U << 7)
> -#define BTRFS_INODE_NODUMP		(1U << 8)
> -#define BTRFS_INODE_NOATIME		(1U << 9)
> -#define BTRFS_INODE_DIRSYNC		(1U << 10)
> -#define BTRFS_INODE_COMPRESS		(1U << 11)
> -
> -#define BTRFS_INODE_ROOT_ITEM_INIT	(1U << 31)
> -
> -#define BTRFS_INODE_FLAG_MASK						\
> -	(BTRFS_INODE_NODATASUM |					\
> -	 BTRFS_INODE_NODATACOW |					\
> -	 BTRFS_INODE_READONLY |						\
> -	 BTRFS_INODE_NOCOMPRESS |					\
> -	 BTRFS_INODE_PREALLOC |						\
> -	 BTRFS_INODE_SYNC |						\
> -	 BTRFS_INODE_IMMUTABLE |					\
> -	 BTRFS_INODE_APPEND |						\
> -	 BTRFS_INODE_NODUMP |						\
> -	 BTRFS_INODE_NOATIME |						\
> -	 BTRFS_INODE_DIRSYNC |						\
> -	 BTRFS_INODE_COMPRESS |						\
> -	 BTRFS_INODE_ROOT_ITEM_INIT)
> -
> -#define BTRFS_INODE_RO_VERITY		(1U << 0)
> -
> -#define BTRFS_INODE_RO_FLAG_MASK	(BTRFS_INODE_RO_VERITY)
> -
>   struct btrfs_map_token {
>   	struct extent_buffer *eb;
>   	char *kaddr;
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index 1f7a38ec6ac3..e6bf902b9c92 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -10,6 +10,10 @@
>   #include <stddef.h>
>   #endif
>   
> +#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
> +
> +#define BTRFS_MAX_LEVEL 8
> +
>   /*
>    * This header contains the structure definitions and constants used
>    * by file system objects that can be retrieved using
> @@ -360,6 +364,43 @@ enum btrfs_csum_type {
>   #define BTRFS_FT_XATTR		8
>   #define BTRFS_FT_MAX		9
>   
> +/*
> + * Inode flags
> + */
> +#define BTRFS_INODE_NODATASUM		(1U << 0)
> +#define BTRFS_INODE_NODATACOW		(1U << 1)
> +#define BTRFS_INODE_READONLY		(1U << 2)
> +#define BTRFS_INODE_NOCOMPRESS		(1U << 3)
> +#define BTRFS_INODE_PREALLOC		(1U << 4)
> +#define BTRFS_INODE_SYNC		(1U << 5)
> +#define BTRFS_INODE_IMMUTABLE		(1U << 6)
> +#define BTRFS_INODE_APPEND		(1U << 7)
> +#define BTRFS_INODE_NODUMP		(1U << 8)
> +#define BTRFS_INODE_NOATIME		(1U << 9)
> +#define BTRFS_INODE_DIRSYNC		(1U << 10)
> +#define BTRFS_INODE_COMPRESS		(1U << 11)
> +
> +#define BTRFS_INODE_ROOT_ITEM_INIT	(1U << 31)
> +
> +#define BTRFS_INODE_FLAG_MASK						\
> +	(BTRFS_INODE_NODATASUM |					\
> +	 BTRFS_INODE_NODATACOW |					\
> +	 BTRFS_INODE_READONLY |						\
> +	 BTRFS_INODE_NOCOMPRESS |					\
> +	 BTRFS_INODE_PREALLOC |						\
> +	 BTRFS_INODE_SYNC |						\
> +	 BTRFS_INODE_IMMUTABLE |					\
> +	 BTRFS_INODE_APPEND |						\
> +	 BTRFS_INODE_NODUMP |						\
> +	 BTRFS_INODE_NOATIME |						\
> +	 BTRFS_INODE_DIRSYNC |						\
> +	 BTRFS_INODE_COMPRESS |						\
> +	 BTRFS_INODE_ROOT_ITEM_INIT)
> +
> +#define BTRFS_INODE_RO_VERITY		(1U << 0)
> +
> +#define BTRFS_INODE_RO_FLAG_MASK	(BTRFS_INODE_RO_VERITY)
> +
>   /*
>    * The key defines the order in the tree, and so it also defines (optimal)
>    * block layout.
> @@ -389,6 +430,108 @@ struct btrfs_key {
>   	__u64 offset;
>   } __attribute__ ((__packed__));
>   
> +/*
> + * every tree block (leaf or node) starts with this header.
> + */
> +struct btrfs_header {
> +	/* these first four must match the super block */
> +	__u8 csum[BTRFS_CSUM_SIZE];
> +	__u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
> +	__le64 bytenr; /* which block this node is supposed to live in */
> +	__le64 flags;
> +
> +	/* allowed to be different from the super from here on down */
> +	__u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
> +	__le64 generation;
> +	__le64 owner;
> +	__le32 nritems;
> +	__u8 level;
> +} __attribute__ ((__packed__));
> +
> +/*
> + * this is a very generous portion of the super block, giving us
> + * room to translate 14 chunks with 3 stripes each.
> + */
> +#define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
> +
> +/*
> + * just in case we somehow lose the roots and are not able to mount,
> + * we store an array of the roots from previous transactions
> + * in the super.
> + */
> +#define BTRFS_NUM_BACKUP_ROOTS 4
> +struct btrfs_root_backup {
> +	__le64 tree_root;
> +	__le64 tree_root_gen;
> +
> +	__le64 chunk_root;
> +	__le64 chunk_root_gen;
> +
> +	__le64 extent_root;
> +	__le64 extent_root_gen;
> +
> +	__le64 fs_root;
> +	__le64 fs_root_gen;
> +
> +	__le64 dev_root;
> +	__le64 dev_root_gen;
> +
> +	__le64 csum_root;
> +	__le64 csum_root_gen;
> +
> +	__le64 total_bytes;
> +	__le64 bytes_used;
> +	__le64 num_devices;
> +	/* future */
> +	__le64 unused_64[4];
> +
> +	__u8 tree_root_level;
> +	__u8 chunk_root_level;
> +	__u8 extent_root_level;
> +	__u8 fs_root_level;
> +	__u8 dev_root_level;
> +	__u8 csum_root_level;
> +	/* future and to align */
> +	__u8 unused_8[10];
> +} __attribute__ ((__packed__));
> +
> +/*
> + * A leaf is full of items. offset and size tell us where to find
> + * the item in the leaf (relative to the start of the data area)
> + */
> +struct btrfs_item {
> +	struct btrfs_disk_key key;
> +	__le32 offset;
> +	__le32 size;
> +} __attribute__ ((__packed__));
> +
> +/*
> + * leaves have an item area and a data area:
> + * [item0, item1....itemN] [free space] [dataN...data1, data0]
> + *
> + * The data is separate from the items to get the keys closer together
> + * during searches.
> + */
> +struct btrfs_leaf {
> +	struct btrfs_header header;
> +	struct btrfs_item items[];
> +} __attribute__ ((__packed__));
> +
> +/*
> + * all non-leaf blocks are nodes, they hold only keys and pointers to
> + * other blocks
> + */
> +struct btrfs_key_ptr {
> +	struct btrfs_disk_key key;
> +	__le64 blockptr;
> +	__le64 generation;
> +} __attribute__ ((__packed__));
> +
> +struct btrfs_node {
> +	struct btrfs_header header;
> +	struct btrfs_key_ptr ptrs[];
> +} __attribute__ ((__packed__));
> +
>   struct btrfs_dev_item {
>   	/* the internal btrfs device id */
>   	__le64 devid;
> @@ -472,6 +615,68 @@ struct btrfs_chunk {
>   	/* additional stripes go here */
>   } __attribute__ ((__packed__));
>   
> +/*
> + * the super block basically lists the main trees of the FS
> + * it currently lacks any block count etc etc
> + */
> +struct btrfs_super_block {
> +	/* the first 4 fields must match struct btrfs_header */
> +	__u8 csum[BTRFS_CSUM_SIZE];
> +	/* FS specific UUID, visible to user */
> +	__u8 fsid[BTRFS_FSID_SIZE];
> +	__le64 bytenr; /* this block number */
> +	__le64 flags;
> +
> +	/* allowed to be different from the btrfs_header from here own down */
> +	__le64 magic;
> +	__le64 generation;
> +	__le64 root;
> +	__le64 chunk_root;
> +	__le64 log_root;
> +
> +	/*
> +	 * This member has never been utilized since the very beginning, thus
> +	 * it's always 0 regardless of kernel version.  We always use
> +	 * generation + 1 to read log tree root.  So here we mark it deprecated.
> +	 */
> +	__le64 __unused_log_root_transid;
> +	__le64 total_bytes;
> +	__le64 bytes_used;
> +	__le64 root_dir_objectid;
> +	__le64 num_devices;
> +	__le32 sectorsize;
> +	__le32 nodesize;
> +	__le32 __unused_leafsize;
> +	__le32 stripesize;
> +	__le32 sys_chunk_array_size;
> +	__le64 chunk_root_generation;
> +	__le64 compat_flags;
> +	__le64 compat_ro_flags;
> +	__le64 incompat_flags;
> +	__le16 csum_type;
> +	__u8 root_level;
> +	__u8 chunk_root_level;
> +	__u8 log_root_level;
> +	struct btrfs_dev_item dev_item;
> +
> +	char label[BTRFS_LABEL_SIZE];
> +
> +	__le64 cache_generation;
> +	__le64 uuid_tree_generation;
> +
> +	/* the UUID written into btree blocks */
> +	__u8 metadata_uuid[BTRFS_FSID_SIZE];
> +
> +	/* future expansion */
> +	__u8 reserved8[8];
> +	__le64 reserved[27];
> +	__u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
> +	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
> +
> +	/* Padded to 4096 bytes */
> +	__u8 padding[565];
> +} __attribute__ ((__packed__));
> +
>   #define BTRFS_FREE_SPACE_EXTENT	1
>   #define BTRFS_FREE_SPACE_BITMAP	2
>   
> @@ -526,6 +731,14 @@ struct btrfs_extent_item_v0 {
>   /* use full backrefs for extent pointers in the block */
>   #define BTRFS_BLOCK_FLAG_FULL_BACKREF	(1ULL << 8)
>   
> +#define BTRFS_BACKREF_REV_MAX		256
> +#define BTRFS_BACKREF_REV_SHIFT		56
> +#define BTRFS_BACKREF_REV_MASK		(((u64)BTRFS_BACKREF_REV_MAX - 1) << \
> +					 BTRFS_BACKREF_REV_SHIFT)
> +
> +#define BTRFS_OLD_BACKREF_REV		0
> +#define BTRFS_MIXED_BACKREF_REV		1
> +
>   /*
>    * this flag is only used internally by scrub and may be changed at any time
>    * it is only declared here to avoid collisions

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h
  2022-09-14 15:06 ` [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h Josef Bacik
@ 2022-09-15  9:10   ` Qu Wenruo
  2022-09-15 14:14   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:10 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This inline helper calls btrfs_fs_compat_ro(), which is defined in
> another header.  To avoid weird header dependency problems move this
> helper into disk-io.c with the rest of the global root helpers.

I kinda like this small function to be inlined.
As the function call cost is not that small compared to the tiny 
function body.

Mind to give some example of the problem you hit?
Maybe we can find some other solution.

Thanks,
Qu

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   fs/btrfs/disk-io.c | 7 +++++++
>   fs/btrfs/disk-io.h | 8 +-------
>   2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a887fe67a2a0..d32aa67f962b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1169,6 +1169,13 @@ struct btrfs_root *btrfs_extent_root(struct btrfs_fs_info *fs_info, u64 bytenr)
>   	return btrfs_global_root(fs_info, &key);
>   }
>   
> +struct btrfs_root *btrfs_block_group_root(struct btrfs_fs_info *fs_info)
> +{
> +	if (btrfs_fs_compat_ro(fs_info, BLOCK_GROUP_TREE))
> +		return fs_info->block_group_root;
> +	return btrfs_extent_root(fs_info, 0);
> +}
> +
>   struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
>   				     u64 objectid)
>   {
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 7e545ec09a10..084fbe5768e1 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -72,6 +72,7 @@ struct btrfs_root *btrfs_global_root(struct btrfs_fs_info *fs_info,
>   				     struct btrfs_key *key);
>   struct btrfs_root *btrfs_csum_root(struct btrfs_fs_info *fs_info, u64 bytenr);
>   struct btrfs_root *btrfs_extent_root(struct btrfs_fs_info *fs_info, u64 bytenr);
> +struct btrfs_root *btrfs_block_group_root(struct btrfs_fs_info *fs_info);
>   
>   void btrfs_free_fs_info(struct btrfs_fs_info *fs_info);
>   int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info);
> @@ -103,13 +104,6 @@ static inline struct btrfs_root *btrfs_grab_root(struct btrfs_root *root)
>   	return NULL;
>   }
>   
> -static inline struct btrfs_root *btrfs_block_group_root(struct btrfs_fs_info *fs_info)
> -{
> -	if (btrfs_fs_compat_ro(fs_info, BLOCK_GROUP_TREE))
> -		return fs_info->block_group_root;
> -	return btrfs_extent_root(fs_info, 0);
> -}
> -
>   void btrfs_put_root(struct btrfs_root *root);
>   void btrfs_mark_buffer_dirty(struct extent_buffer *buf);
>   int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h
  2022-09-14 15:06 ` [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h Josef Bacik
@ 2022-09-15  9:10   ` Qu Wenruo
  2022-09-15 14:15   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:10 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> We have maximum link and name length limits, move these to btrfs_tree.h
> as they're on disk limitations.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.h                | 13 -------------
>   include/uapi/linux/btrfs_tree.h | 13 +++++++++++++
>   2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index c3a8440d3223..5e6b025c0870 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -63,19 +63,6 @@ struct btrfs_ioctl_encoded_io_args;
>   
>   #define BTRFS_OLDEST_GENERATION	0ULL
>   
> -/*
> - * we can actually store much bigger names, but lets not confuse the rest
> - * of linux
> - */
> -#define BTRFS_NAME_LEN 255
> -
> -/*
> - * Theoretical limit is larger, but we keep this down to a sane
> - * value. That should limit greatly the possibility of collisions on
> - * inode ref items.
> - */
> -#define BTRFS_LINK_MAX 65535U
> -
>   #define BTRFS_EMPTY_DIR_SIZE 0
>   
>   #define BTRFS_DIRTY_METADATA_THRESH	SZ_32M
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index e6bf902b9c92..c85e8c44ab92 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -14,6 +14,19 @@
>   
>   #define BTRFS_MAX_LEVEL 8
>   
> +/*
> + * we can actually store much bigger names, but lets not confuse the rest
> + * of linux
> + */
> +#define BTRFS_NAME_LEN 255
> +
> +/*
> + * Theoretical limit is larger, but we keep this down to a sane
> + * value. That should limit greatly the possibility of collisions on
> + * inode ref items.
> + */
> +#define BTRFS_LINK_MAX 65535U
> +
>   /*
>    * This header contains the structure definitions and constants used
>    * by file system objects that can be retrieved using

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c
  2022-09-14 15:06 ` [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c Josef Bacik
@ 2022-09-15  9:11   ` Qu Wenruo
  2022-09-15 14:16   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:11 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This is only used locally in scrub.c, move it out of ctree.h into
> scrub.c.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/ctree.h | 11 -----------
>   fs/btrfs/scrub.c | 11 +++++++++++
>   2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 5e6b025c0870..e1ec047deff6 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -50,17 +50,6 @@ struct btrfs_ref;
>   struct btrfs_bio;
>   struct btrfs_ioctl_encoded_io_args;
>   
> -/*
> - * Maximum number of mirrors that can be available for all profiles counting
> - * the target device of dev-replace as one. During an active device replace
> - * procedure, the target device of the copy operation is a mirror for the
> - * filesystem data as well that can be used to read data in order to repair
> - * read errors on other disks.
> - *
> - * Current value is derived from RAID1C4 with 4 copies.
> - */
> -#define BTRFS_MAX_MIRRORS (4 + 1)
> -
>   #define BTRFS_OLDEST_GENERATION	0ULL
>   
>   #define BTRFS_EMPTY_DIR_SIZE 0
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 9b6a0adccc7b..35fca65f0f2a 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -56,6 +56,17 @@ struct scrub_ctx;
>   
>   #define SCRUB_MAX_PAGES			(DIV_ROUND_UP(BTRFS_MAX_METADATA_BLOCKSIZE, PAGE_SIZE))
>   
> +/*
> + * Maximum number of mirrors that can be available for all profiles counting
> + * the target device of dev-replace as one. During an active device replace
> + * procedure, the target device of the copy operation is a mirror for the
> + * filesystem data as well that can be used to read data in order to repair
> + * read errors on other disks.
> + *
> + * Current value is derived from RAID1C4 with 4 copies.
> + */
> +#define BTRFS_MAX_MIRRORS (4 + 1)
> +
>   struct scrub_recover {
>   	refcount_t		refs;
>   	struct btrfs_io_context	*bioc;

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h
  2022-09-14 15:06 ` [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h Josef Bacik
@ 2022-09-15  9:13   ` Qu Wenruo
  2022-09-15 14:18   ` Johannes Thumshirn
  2022-10-07 17:16   ` David Sterba
  2 siblings, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:13 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> These definitions are used for discard statistics, move them out of
> ctree.h and put them in free-space-cache.h.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.h            | 9 ---------
>   fs/btrfs/free-space-cache.h | 9 +++++++++
>   2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index e1ec047deff6..2e6a947a48de 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -58,15 +58,6 @@ struct btrfs_ioctl_encoded_io_args;
>   
>   #define BTRFS_MAX_EXTENT_SIZE SZ_128M
>   
> -/*
> - * Deltas are an effective way to populate global statistics.  Give macro names
> - * to make it clear what we're doing.  An example is discard_extents in
> - * btrfs_free_space_ctl.
> - */
> -#define BTRFS_STAT_NR_ENTRIES	2
> -#define BTRFS_STAT_CURR		0
> -#define BTRFS_STAT_PREV		1
> -
>   static inline unsigned long btrfs_chunk_item_size(int num_stripes)
>   {
>   	BUG_ON(num_stripes == 0);
> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> index 6d419ba53e95..eaf30f6444dd 100644
> --- a/fs/btrfs/free-space-cache.h
> +++ b/fs/btrfs/free-space-cache.h
> @@ -43,6 +43,15 @@ static inline bool btrfs_free_space_trimming_bitmap(
>   	return (info->trim_state == BTRFS_TRIM_STATE_TRIMMING);
>   }
>   
> +/*
> + * Deltas are an effective way to populate global statistics.  Give macro names
> + * to make it clear what we're doing.  An example is discard_extents in
> + * btrfs_free_space_ctl.
> + */
> +#define BTRFS_STAT_NR_ENTRIES	2
> +#define BTRFS_STAT_CURR		0
> +#define BTRFS_STAT_PREV		1
> +
>   struct btrfs_free_space_ctl {
>   	spinlock_t tree_lock;
>   	struct rb_root free_space_offset;

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h
  2022-09-14 15:06 ` [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h Josef Bacik
@ 2022-09-15  9:14   ` Qu Wenruo
  2022-10-07 17:23     ` David Sterba
  2022-09-15 14:19   ` Johannes Thumshirn
  1 sibling, 1 reply; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:14 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This is more of a volumes related helper, additionally it has a BUG_ON()
> which isn't defined in the related header.  Move the code to volumes.c,
> change the BUG_ON() to an ASSERT(), and move the prototype to volumes.h.

Again a very small function, can it be inlined instead?

Thanks,
Qu
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   fs/btrfs/ctree.h   | 7 -------
>   fs/btrfs/volumes.c | 7 +++++++
>   fs/btrfs/volumes.h | 2 +-
>   3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 2e6a947a48de..60f8817f5b7c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -58,13 +58,6 @@ struct btrfs_ioctl_encoded_io_args;
>   
>   #define BTRFS_MAX_EXTENT_SIZE SZ_128M
>   
> -static inline unsigned long btrfs_chunk_item_size(int num_stripes)
> -{
> -	BUG_ON(num_stripes == 0);
> -	return sizeof(struct btrfs_chunk) +
> -		sizeof(struct btrfs_stripe) * (num_stripes - 1);
> -}
> -
>   /*
>    * Runtime (in-memory) states of filesystem
>    */
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 94ba46d57920..b4de4d5ed69f 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -160,6 +160,13 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>   	},
>   };
>   
> +unsigned long btrfs_chunk_item_size(int num_stripes)
> +{
> +	ASSERT(num_stripes);
> +	return sizeof(struct btrfs_chunk) +
> +		sizeof(struct btrfs_stripe) * (num_stripes - 1);
> +}
> +
>   /*
>    * Convert block group flags (BTRFS_BLOCK_GROUP_*) to btrfs_raid_types, which
>    * can be used as index to access btrfs_raid_array[].
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index f19a1cd1bfcf..96a7b437ff20 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -730,5 +730,5 @@ int btrfs_bg_type_to_factor(u64 flags);
>   const char *btrfs_bg_type_to_raid_name(u64 flags);
>   int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
>   bool btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical);
> -
> +unsigned long btrfs_chunk_item_size(int num_stripes);
>   #endif

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c
  2022-09-14 15:06 ` [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c Josef Bacik
@ 2022-09-15  9:16   ` Qu Wenruo
  2022-09-15 14:21   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:16 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This function uses functions that are not defined in block-group.h, move
> it into block-group.c in order to keep the header clean.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>


Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/block-group.c | 12 ++++++++++++
>   fs/btrfs/block-group.h | 11 +----------
>   2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index c52b6e245b9a..c91f47a45b06 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -18,6 +18,18 @@
>   #include "raid56.h"
>   #include "zoned.h"
>   
> +#ifdef CONFIG_BTRFS_DEBUG
> +int btrfs_should_fragment_free_space(struct btrfs_block_group *block_group)
> +{
> +	struct btrfs_fs_info *fs_info = block_group->fs_info;
> +
> +	return (btrfs_test_opt(fs_info, FRAGMENT_METADATA) &&
> +		block_group->flags & BTRFS_BLOCK_GROUP_METADATA) ||
> +	       (btrfs_test_opt(fs_info, FRAGMENT_DATA) &&
> +		block_group->flags &  BTRFS_BLOCK_GROUP_DATA);
> +}
> +#endif
> +
>   /*
>    * Return target flags in extended format or 0 if restripe for this chunk_type
>    * is not in progress
> diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
> index 4d4d2e1f137b..e34cb80ffb25 100644
> --- a/fs/btrfs/block-group.h
> +++ b/fs/btrfs/block-group.h
> @@ -242,16 +242,7 @@ static inline bool btrfs_is_block_group_data_only(
>   }
>   
>   #ifdef CONFIG_BTRFS_DEBUG
> -static inline int btrfs_should_fragment_free_space(
> -		struct btrfs_block_group *block_group)
> -{
> -	struct btrfs_fs_info *fs_info = block_group->fs_info;
> -
> -	return (btrfs_test_opt(fs_info, FRAGMENT_METADATA) &&
> -		block_group->flags & BTRFS_BLOCK_GROUP_METADATA) ||
> -	       (btrfs_test_opt(fs_info, FRAGMENT_DATA) &&
> -		block_group->flags &  BTRFS_BLOCK_GROUP_DATA);
> -}
> +int btrfs_should_fragment_free_space(struct btrfs_block_group *block_group);
>   #endif
>   
>   struct btrfs_block_group *btrfs_lookup_first_block_group(

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 11/17] btrfs: move flush related definitions to space-info.h
  2022-09-14 15:06 ` [PATCH 11/17] btrfs: move flush related definitions to space-info.h Josef Bacik
@ 2022-09-15  9:21   ` Qu Wenruo
  2022-10-07 17:28     ` David Sterba
  2022-09-15 14:21   ` Johannes Thumshirn
  1 sibling, 1 reply; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:21 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This code is used in space-info.c, move the definitions to space-info.h.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Looks good overall, but one new header makes me wonder:

> ---
>   fs/btrfs/ctree.h         | 59 ----------------------------------------
>   fs/btrfs/delayed-inode.c |  1 +
>   fs/btrfs/inode-item.c    |  1 +
>   fs/btrfs/props.c         |  1 +
>   fs/btrfs/relocation.c    |  1 +
>   fs/btrfs/space-info.h    | 59 ++++++++++++++++++++++++++++++++++++++++
>   fs/btrfs/super.c         |  1 +
>   7 files changed, 64 insertions(+), 59 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 60f8817f5b7c..d99720cf4835 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2654,65 +2654,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>   
>   void btrfs_clear_space_info_full(struct btrfs_fs_info *info);
>   
> -/*
> - * Different levels for to flush space when doing space reservations.
> - *
> - * The higher the level, the more methods we try to reclaim space.
> - */
> -enum btrfs_reserve_flush_enum {
> -	/* If we are in the transaction, we can't flush anything.*/
> -	BTRFS_RESERVE_NO_FLUSH,
> -
> -	/*
> -	 * Flush space by:
> -	 * - Running delayed inode items
> -	 * - Allocating a new chunk
> -	 */
> -	BTRFS_RESERVE_FLUSH_LIMIT,
> -
> -	/*
> -	 * Flush space by:
> -	 * - Running delayed inode items
> -	 * - Running delayed refs
> -	 * - Running delalloc and waiting for ordered extents
> -	 * - Allocating a new chunk
> -	 */
> -	BTRFS_RESERVE_FLUSH_EVICT,
> -
> -	/*
> -	 * Flush space by above mentioned methods and by:
> -	 * - Running delayed iputs
> -	 * - Committing transaction
> -	 *
> -	 * Can be interrupted by a fatal signal.
> -	 */
> -	BTRFS_RESERVE_FLUSH_DATA,
> -	BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE,
> -	BTRFS_RESERVE_FLUSH_ALL,
> -
> -	/*
> -	 * Pretty much the same as FLUSH_ALL, but can also steal space from
> -	 * global rsv.
> -	 *
> -	 * Can be interrupted by a fatal signal.
> -	 */
> -	BTRFS_RESERVE_FLUSH_ALL_STEAL,
> -};
> -
> -enum btrfs_flush_state {
> -	FLUSH_DELAYED_ITEMS_NR	=	1,
> -	FLUSH_DELAYED_ITEMS	=	2,
> -	FLUSH_DELAYED_REFS_NR	=	3,
> -	FLUSH_DELAYED_REFS	=	4,
> -	FLUSH_DELALLOC		=	5,
> -	FLUSH_DELALLOC_WAIT	=	6,
> -	FLUSH_DELALLOC_FULL	=	7,
> -	ALLOC_CHUNK		=	8,
> -	ALLOC_CHUNK_FORCE	=	9,
> -	RUN_DELAYED_IPUTS	=	10,
> -	COMMIT_TRANS		=	11,
> -};
> -
>   int btrfs_subvolume_reserve_metadata(struct btrfs_root *root,
>   				     struct btrfs_block_rsv *rsv,
>   				     int nitems, bool use_global_rsv);
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index cac5169eaf8d..a411f04a7b97 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -14,6 +14,7 @@
>   #include "qgroup.h"
>   #include "locking.h"
>   #include "inode-item.h"
> +#include "space-info.h"
>   
>   #define BTRFS_DELAYED_WRITEBACK		512
>   #define BTRFS_DELAYED_BACKGROUND	128
> diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
> index 0eeb5ea87894..366f3a788c6a 100644
> --- a/fs/btrfs/inode-item.c
> +++ b/fs/btrfs/inode-item.c
> @@ -8,6 +8,7 @@
>   #include "disk-io.h"
>   #include "transaction.h"
>   #include "print-tree.h"
> +#include "space-info.h"
>   
>   struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
>   						   int slot, const char *name,
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index 055a631276ce..07f62e3ba6a5 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -10,6 +10,7 @@
>   #include "ctree.h"
>   #include "xattr.h"
>   #include "compression.h"
> +#include "space-info.h"
>   
>   #define BTRFS_PROP_HANDLERS_HT_BITS 8
>   static DEFINE_HASHTABLE(prop_handlers_ht, BTRFS_PROP_HANDLERS_HT_BITS);

>   struct btrfs_space_info {
>   	spinlock_t lock;
>   
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index be7df8d1d5b8..2add5b23c476 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -49,6 +49,7 @@
>   #include "discard.h"
>   #include "qgroup.h"
>   #include "raid56.h"
> +#include "space-info.h"

Why super.c needs this header?

The moved code is definition for btrfs_reserve_flush_enum and 
btrfs_flush_state, but I didn't see any direct usage inside super.c.

Is it for trace event?

Or you just did the same search using "FLUSH" as keyword and got hit on 
FLUSHONCOMMIT?

Thanks,
Qu

>   #define CREATE_TRACE_POINTS
>   #include <trace/events/btrfs.h>
>   

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c
  2022-09-14 15:06 ` [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c Josef Bacik
@ 2022-09-15  9:22   ` Qu Wenruo
  2022-10-07 17:31     ` David Sterba
  2022-09-15 14:23   ` Johannes Thumshirn
  1 sibling, 1 reply; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:22 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This isn't used outside of inode.c, there's no reason to define it in
> btrfs_inode.h.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just a small nitpick below.

> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 6fde13f62c1d..998d1c7134ff 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -125,6 +125,31 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start,
>   				       u64 ram_bytes, int compress_type,
>   				       int type);
>   
> +static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,

IIRC for static function there is no need for explicit inline keyword.

Under most cases the compiler should be more clever than us.

Thanks,
Qu

> +		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
> +{
> +	struct btrfs_root *root = inode->root;
> +	const u32 csum_size = root->fs_info->csum_size;
> +
> +	/* Output minus objectid, which is more meaningful */
> +	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
> +		btrfs_warn_rl(root->fs_info,
> +"csum failed root %lld ino %lld off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d",
> +			root->root_key.objectid, btrfs_ino(inode),
> +			logical_start,
> +			CSUM_FMT_VALUE(csum_size, csum),
> +			CSUM_FMT_VALUE(csum_size, csum_expected),
> +			mirror_num);
> +	else
> +		btrfs_warn_rl(root->fs_info,
> +"csum failed root %llu ino %llu off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d",
> +			root->root_key.objectid, btrfs_ino(inode),
> +			logical_start,
> +			CSUM_FMT_VALUE(csum_size, csum),
> +			CSUM_FMT_VALUE(csum_size, csum_expected),
> +			mirror_num);
> +}
> +
>   /*
>    * btrfs_inode_lock - lock inode i_rwsem based on arguments passed
>    *

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h
  2022-09-14 15:06 ` [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h Josef Bacik
@ 2022-09-15  9:23   ` Qu Wenruo
  2022-09-15 14:24   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:23 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This is local to the transaction code, remove it from ctree.h and
> inode.c, create new helpers in the transaction to handle the init work
> and move the cachep locally to transaction.c.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.h       |  1 -
>   fs/btrfs/inode.c       |  8 --------
>   fs/btrfs/super.c       |  9 ++++++++-
>   fs/btrfs/transaction.c | 17 +++++++++++++++++
>   fs/btrfs/transaction.h |  2 ++
>   5 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index d99720cf4835..439b205f4207 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -41,7 +41,6 @@ struct btrfs_pending_snapshot;
>   struct btrfs_delayed_ref_root;
>   struct btrfs_space_info;
>   struct btrfs_block_group;
> -extern struct kmem_cache *btrfs_trans_handle_cachep;
>   extern struct kmem_cache *btrfs_path_cachep;
>   extern struct kmem_cache *btrfs_free_space_cachep;
>   extern struct kmem_cache *btrfs_free_space_bitmap_cachep;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 998d1c7134ff..78e7f5397d58 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -107,7 +107,6 @@ static const struct address_space_operations btrfs_aops;
>   static const struct file_operations btrfs_dir_file_operations;
>   
>   static struct kmem_cache *btrfs_inode_cachep;
> -struct kmem_cache *btrfs_trans_handle_cachep;
>   struct kmem_cache *btrfs_path_cachep;
>   struct kmem_cache *btrfs_free_space_cachep;
>   struct kmem_cache *btrfs_free_space_bitmap_cachep;
> @@ -8938,7 +8937,6 @@ void __cold btrfs_destroy_cachep(void)
>   	rcu_barrier();
>   	bioset_exit(&btrfs_dio_bioset);
>   	kmem_cache_destroy(btrfs_inode_cachep);
> -	kmem_cache_destroy(btrfs_trans_handle_cachep);
>   	kmem_cache_destroy(btrfs_path_cachep);
>   	kmem_cache_destroy(btrfs_free_space_cachep);
>   	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
> @@ -8953,12 +8951,6 @@ int __init btrfs_init_cachep(void)
>   	if (!btrfs_inode_cachep)
>   		goto fail;
>   
> -	btrfs_trans_handle_cachep = kmem_cache_create("btrfs_trans_handle",
> -			sizeof(struct btrfs_trans_handle), 0,
> -			SLAB_TEMPORARY | SLAB_MEM_SPREAD, NULL);
> -	if (!btrfs_trans_handle_cachep)
> -		goto fail;
> -
>   	btrfs_path_cachep = kmem_cache_create("btrfs_path",
>   			sizeof(struct btrfs_path), 0,
>   			SLAB_MEM_SPREAD, NULL);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 2add5b23c476..9f7fc1c71148 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2737,10 +2737,14 @@ static int __init init_btrfs_fs(void)
>   	if (err)
>   		goto free_compress;
>   
> -	err = extent_state_init_cachep();
> +	err = btrfs_transaction_init();
>   	if (err)
>   		goto free_cachep;
>   
> +	err = extent_state_init_cachep();
> +	if (err)
> +		goto free_transaction;
> +
>   	err = extent_buffer_init_cachep();
>   	if (err)
>   		goto free_extent_cachep;
> @@ -2809,6 +2813,8 @@ static int __init init_btrfs_fs(void)
>   	extent_buffer_free_cachep();
>   free_extent_cachep:
>   	extent_state_free_cachep();
> +free_transaction:
> +	btrfs_transaction_exit();
>   free_cachep:
>   	btrfs_destroy_cachep();
>   free_compress:
> @@ -2820,6 +2826,7 @@ static int __init init_btrfs_fs(void)
>   
>   static void __exit exit_btrfs_fs(void)
>   {
> +	btrfs_transaction_exit();
>   	btrfs_destroy_cachep();
>   	btrfs_delayed_ref_exit();
>   	btrfs_auto_defrag_exit();
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index d1f1da6820fb..ae7d4aca771d 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -24,6 +24,8 @@
>   #include "space-info.h"
>   #include "zoned.h"
>   
> +static struct kmem_cache *btrfs_trans_handle_cachep;
> +
>   #define BTRFS_ROOT_TRANS_TAG 0
>   
>   /*
> @@ -2600,3 +2602,18 @@ void btrfs_apply_pending_changes(struct btrfs_fs_info *fs_info)
>   		btrfs_warn(fs_info,
>   			"unknown pending changes left 0x%lx, ignoring", prev);
>   }
> +
> +int __init btrfs_transaction_init(void)
> +{
> +	btrfs_trans_handle_cachep = kmem_cache_create("btrfs_trans_handle",
> +			sizeof(struct btrfs_trans_handle), 0,
> +			SLAB_TEMPORARY | SLAB_MEM_SPREAD, NULL);
> +	if (!btrfs_trans_handle_cachep)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +void __cold btrfs_transaction_exit(void)
> +{
> +	kmem_cache_destroy(btrfs_trans_handle_cachep);
> +}
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 970ff316069d..b5651c372946 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -236,4 +236,6 @@ void btrfs_add_dropped_root(struct btrfs_trans_handle *trans,
>   			    struct btrfs_root *root);
>   void btrfs_trans_release_chunk_metadata(struct btrfs_trans_handle *trans);
>   
> +int __init btrfs_transaction_init(void);
> +void __cold btrfs_transaction_exit(void);
>   #endif

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 14/17] btrfs: move btrfs_path_cachep out of ctree.h
  2022-09-14 15:06 ` [PATCH 14/17] btrfs: move btrfs_path_cachep " Josef Bacik
@ 2022-09-15  9:27   ` Qu Wenruo
  2022-09-15 14:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:27 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This is local to the ctree code, remove it from ctree.h and inode.c,
> create new init/exit functions for the cachep, and move it locally to
> ctree.c.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

But one thing mentioned below.

[...]
> @@ -2813,6 +2817,8 @@ static int __init init_btrfs_fs(void)
>   	extent_buffer_free_cachep();
>   free_extent_cachep:
>   	extent_state_free_cachep();
> +free_ctree:
> +	btrfs_ctree_exit();

The tags will grow larger and larger.

Can we make a new cleanup to make all those _exit() function to be 
unconditional? (e.g. check the cachep and only free them if initialized)

That already happened for open_ctree() which has some incorrect error 
handling.

Thus a new small series cleaning up those tags would be a good idea.

Thanks,
Qu

>   free_transaction:
>   	btrfs_transaction_exit();
>   free_cachep:
> @@ -2826,6 +2832,7 @@ static int __init init_btrfs_fs(void)
>   
>   static void __exit exit_btrfs_fs(void)
>   {
> +	btrfs_ctree_exit();
>   	btrfs_transaction_exit();
>   	btrfs_destroy_cachep();
>   	btrfs_delayed_ref_exit();

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 15/17] btrfs: move free space cachep's out of ctree.h
  2022-09-14 15:06 ` [PATCH 15/17] btrfs: move free space cachep's " Josef Bacik
@ 2022-09-15  9:27   ` Qu Wenruo
  2022-09-15 14:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:27 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This is local to the free-space-cache.c code, remove it from ctree.h and
> inode.c, create new init/exit functions for the cachep, and move it
> locally to free-space-cache.c.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/ctree.h            |  2 --
>   fs/btrfs/free-space-cache.c | 28 ++++++++++++++++++++++++++++
>   fs/btrfs/free-space-cache.h |  2 ++
>   fs/btrfs/inode.c            | 16 ----------------
>   fs/btrfs/super.c            |  9 ++++++++-
>   5 files changed, 38 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 3a61f5c0ab5f..af6f6764d9a4 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -41,8 +41,6 @@ struct btrfs_pending_snapshot;
>   struct btrfs_delayed_ref_root;
>   struct btrfs_space_info;
>   struct btrfs_block_group;
> -extern struct kmem_cache *btrfs_free_space_cachep;
> -extern struct kmem_cache *btrfs_free_space_bitmap_cachep;
>   struct btrfs_ordered_sum;
>   struct btrfs_ref;
>   struct btrfs_bio;
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 7859eeca484c..ee03c5e6db4c 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -29,6 +29,9 @@
>   #define MAX_CACHE_BYTES_PER_GIG	SZ_64K
>   #define FORCE_EXTENT_THRESHOLD	SZ_1M
>   
> +static struct kmem_cache *btrfs_free_space_cachep;
> +static struct kmem_cache *btrfs_free_space_bitmap_cachep;
> +
>   struct btrfs_trim_range {
>   	u64 start;
>   	u64 bytes;
> @@ -4132,6 +4135,31 @@ int btrfs_set_free_space_cache_v1_active(struct btrfs_fs_info *fs_info, bool act
>   	return ret;
>   }
>   
> +int __init btrfs_free_space_init(void)
> +{
> +	btrfs_free_space_cachep = kmem_cache_create("btrfs_free_space",
> +			sizeof(struct btrfs_free_space), 0,
> +			SLAB_MEM_SPREAD, NULL);
> +	if (!btrfs_free_space_cachep)
> +		return -ENOMEM;
> +
> +	btrfs_free_space_bitmap_cachep = kmem_cache_create("btrfs_free_space_bitmap",
> +							PAGE_SIZE, PAGE_SIZE,
> +							SLAB_MEM_SPREAD, NULL);
> +	if (!btrfs_free_space_bitmap_cachep) {
> +		kmem_cache_destroy(btrfs_free_space_cachep);
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +void __cold btrfs_free_space_exit(void)
> +{
> +	kmem_cache_destroy(btrfs_free_space_cachep);
> +	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
> +}
> +
>   #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>   /*
>    * Use this if you need to make a bitmap or extent entry specifically, it
> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> index eaf30f6444dd..cab954a9d97b 100644
> --- a/fs/btrfs/free-space-cache.h
> +++ b/fs/btrfs/free-space-cache.h
> @@ -88,6 +88,8 @@ struct btrfs_io_ctl {
>   	int bitmaps;
>   };
>   
> +int __init btrfs_free_space_init(void);
> +void __cold btrfs_free_space_exit(void);
>   struct inode *lookup_free_space_inode(struct btrfs_block_group *block_group,
>   		struct btrfs_path *path);
>   int create_free_space_inode(struct btrfs_trans_handle *trans,
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1401e2da9284..da5be8f23f68 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -107,8 +107,6 @@ static const struct address_space_operations btrfs_aops;
>   static const struct file_operations btrfs_dir_file_operations;
>   
>   static struct kmem_cache *btrfs_inode_cachep;
> -struct kmem_cache *btrfs_free_space_cachep;
> -struct kmem_cache *btrfs_free_space_bitmap_cachep;
>   
>   static int btrfs_setsize(struct inode *inode, struct iattr *attr);
>   static int btrfs_truncate(struct inode *inode, bool skip_writeback);
> @@ -8936,8 +8934,6 @@ void __cold btrfs_destroy_cachep(void)
>   	rcu_barrier();
>   	bioset_exit(&btrfs_dio_bioset);
>   	kmem_cache_destroy(btrfs_inode_cachep);
> -	kmem_cache_destroy(btrfs_free_space_cachep);
> -	kmem_cache_destroy(btrfs_free_space_bitmap_cachep);
>   }
>   
>   int __init btrfs_init_cachep(void)
> @@ -8949,18 +8945,6 @@ int __init btrfs_init_cachep(void)
>   	if (!btrfs_inode_cachep)
>   		goto fail;
>   
> -	btrfs_free_space_cachep = kmem_cache_create("btrfs_free_space",
> -			sizeof(struct btrfs_free_space), 0,
> -			SLAB_MEM_SPREAD, NULL);
> -	if (!btrfs_free_space_cachep)
> -		goto fail;
> -
> -	btrfs_free_space_bitmap_cachep = kmem_cache_create("btrfs_free_space_bitmap",
> -							PAGE_SIZE, PAGE_SIZE,
> -							SLAB_MEM_SPREAD, NULL);
> -	if (!btrfs_free_space_bitmap_cachep)
> -		goto fail;
> -
>   	if (bioset_init(&btrfs_dio_bioset, BIO_POOL_SIZE,
>   			offsetof(struct btrfs_dio_private, bio),
>   			BIOSET_NEED_BVECS))
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index acd590bed579..c2e634de01e4 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2745,10 +2745,14 @@ static int __init init_btrfs_fs(void)
>   	if (err)
>   		goto free_transaction;
>   
> -	err = extent_state_init_cachep();
> +	err = btrfs_free_space_init();
>   	if (err)
>   		goto free_ctree;
>   
> +	err = extent_state_init_cachep();
> +	if (err)
> +		goto free_free_space;
> +
>   	err = extent_buffer_init_cachep();
>   	if (err)
>   		goto free_extent_cachep;
> @@ -2817,6 +2821,8 @@ static int __init init_btrfs_fs(void)
>   	extent_buffer_free_cachep();
>   free_extent_cachep:
>   	extent_state_free_cachep();
> +free_free_space:
> +	btrfs_free_space_exit();
>   free_ctree:
>   	btrfs_ctree_exit();
>   free_transaction:
> @@ -2832,6 +2838,7 @@ static int __init init_btrfs_fs(void)
>   
>   static void __exit exit_btrfs_fs(void)
>   {
> +	btrfs_free_space_exit();
>   	btrfs_ctree_exit();
>   	btrfs_transaction_exit();
>   	btrfs_destroy_cachep();

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c
  2022-09-14 15:06 ` [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c Josef Bacik
@ 2022-09-15  9:29   ` Qu Wenruo
  2022-09-15 14:29   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:29 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> This uses btrfs_header_nritems, which I will be moving out of ctree.h.
> In order to avoid needing to include the relevant header in ctree.h,
> simply move this helper function into ctree.c.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Since it's only used by backref, uninlining it looks fine.


Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.c |  9 +++++++++
>   fs/btrfs/ctree.h | 10 ++--------
>   2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 1f0355c74fe6..0f7f93bc2582 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -4775,6 +4775,15 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>   	return ret;
>   }
>   
> +int btrfs_next_old_item(struct btrfs_root *root, struct btrfs_path *p,
> +			u64 time_seq)
> +{
> +	++p->slots[0];
> +	if (p->slots[0] >= btrfs_header_nritems(p->nodes[0]))
> +		return btrfs_next_old_leaf(root, p, time_seq);
> +	return 0;
> +}
> +
>   /*
>    * this uses btrfs_prev_leaf to walk backwards in the tree, and keeps
>    * searching until it gets past min_objectid or finds an item of 'type'
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index af6f6764d9a4..42dec21b3517 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2836,14 +2836,8 @@ int btrfs_get_next_valid_item(struct btrfs_root *root, struct btrfs_key *key,
>   		(path)->slots[0]++						\
>   	)
>   
> -static inline int btrfs_next_old_item(struct btrfs_root *root,
> -				      struct btrfs_path *p, u64 time_seq)
> -{
> -	++p->slots[0];
> -	if (p->slots[0] >= btrfs_header_nritems(p->nodes[0]))
> -		return btrfs_next_old_leaf(root, p, time_seq);
> -	return 0;
> -}
> +int btrfs_next_old_item(struct btrfs_root *root, struct btrfs_path *p,
> +			u64 time_seq);
>   
>   /*
>    * Search the tree again to find a leaf with greater keys.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h
  2022-09-14 15:06 ` [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h Josef Bacik
@ 2022-09-15  9:30   ` Qu Wenruo
  2022-09-15 14:30   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:30 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> These are wrapped in CONFIG_FS_VERITY, but we can have the definitions
> without verity enabled.  Move these definitions up with the other
> accessor helpers.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>


Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/ctree.h | 23 ++++++++++-------------
>   1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 42dec21b3517..cb1ae35c1095 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2499,6 +2499,16 @@ BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_left,
>   BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_right,
>   			 struct btrfs_dev_replace_item, cursor_right, 64);
>   
> +/* btrfs_verity_descriptor_item */
> +BTRFS_SETGET_FUNCS(verity_descriptor_encryption, struct btrfs_verity_descriptor_item,
> +		   encryption, 8);
> +BTRFS_SETGET_FUNCS(verity_descriptor_size, struct btrfs_verity_descriptor_item,
> +		   size, 64);
> +BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_encryption,
> +			 struct btrfs_verity_descriptor_item, encryption, 8);
> +BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_size,
> +			 struct btrfs_verity_descriptor_item, size, 64);
> +
>   /* helper function to cast into the data area of the leaf. */
>   #define btrfs_item_ptr(leaf, slot, type) \
>   	((type *)(BTRFS_LEAF_DATA_OFFSET + \
> @@ -3742,22 +3752,10 @@ static inline int btrfs_defrag_cancelled(struct btrfs_fs_info *fs_info)
>   
>   /* verity.c */
>   #ifdef CONFIG_FS_VERITY
> -
>   extern const struct fsverity_operations btrfs_verityops;
>   int btrfs_drop_verity_items(struct btrfs_inode *inode);
>   int btrfs_get_verity_descriptor(struct inode *inode, void *buf, size_t buf_size);
> -
> -BTRFS_SETGET_FUNCS(verity_descriptor_encryption, struct btrfs_verity_descriptor_item,
> -		   encryption, 8);
> -BTRFS_SETGET_FUNCS(verity_descriptor_size, struct btrfs_verity_descriptor_item,
> -		   size, 64);
> -BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_encryption,
> -			 struct btrfs_verity_descriptor_item, encryption, 8);
> -BTRFS_SETGET_STACK_FUNCS(stack_verity_descriptor_size,
> -			 struct btrfs_verity_descriptor_item, size, 64);
> -
>   #else
> -
>   static inline int btrfs_drop_verity_items(struct btrfs_inode *inode)
>   {
>   	return 0;
> @@ -3768,7 +3766,6 @@ static inline int btrfs_get_verity_descriptor(struct inode *inode, void *buf,
>   {
>   	return -EPERM;
>   }
> -
>   #endif
>   
>   /* Sanity test specific functions */

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff
  2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
                   ` (16 preceding siblings ...)
  2022-09-14 15:06 ` [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h Josef Bacik
@ 2022-09-15  9:47 ` Qu Wenruo
  2022-10-07 17:51   ` David Sterba
  17 siblings, 1 reply; 61+ messages in thread
From: Qu Wenruo @ 2022-09-15  9:47 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team



On 2022/9/14 23:06, Josef Bacik wrote:
> Hello,
> 
> This is the first part in what will probably be very many parts in my work to
> cleanup ctree.h.  These are the smaller changes, mostly removing code that's not
> used anymore, moving some stuff that's local to C files that don't need to be in
> the header at all, and moving the rest of the on-disk definition stuff to
> btrfs_tree.h.  There's a lot of patche here, but they're all relatively small,
> the largest being the patch to move the on-disk definitions to btrfs_tree.h,
> which is not very large compared to patches in the next several series.  This
> has been built and tested, it's relatively low risk.  Thanks,

Looks good overall to me.

Just 3 small points of concern:

- About btrfs_tree.h usage.

   Really hope David can make it clear that, that UAPI header is for ALL
   on-disk format definition.

   I'm not buying the old reason that the UAPI header is only for tree
   search ioctl, and really want to move the whole super block definition
   into UAPI header.

   (And I also think all upstream fses should have a concentrated UAPI
    header too, just to make new comers easier to hack)

- Some tiny inline functions got unexported

   May not be a big thing though, just want to make sure we have no other
   choice but make them uninlined.

- Extra error tags in init_btrfs_fs()

   Just like open_ctree(), it's when but not whether to have wrong jump.
   Thus a new series to make those cachep related exit function
   conditional. (aka, we can call them unconditionally at error path)

Thanks,
Qu
> 
> Josef
> 
> Josef Bacik (17):
>    btrfs: remove set/clear_pending_info helpers
>    btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH
>    btrfs: remove BTRFS_IOPRIO_READA
>    btrfs: move btrfs on disk definitions out of ctree.h
>    btrfs: move btrfs_get_block_group helper out of disk-io.h
>    btrfs: move maximum limits to btrfs_tree.h
>    btrfs: move BTRFS_MAX_MIRRORS into scrub.c
>    btrfs: move discard stat defs to free-space-cache.h
>    btrfs: move btrfs_chunk_item_size out of ctree.h
>    btrfs: move btrfs_should_fragment_free_space into block-group.c
>    btrfs: move flush related definitions to space-info.h
>    btrfs: move btrfs_print_data_csum_error into inode.c
>    btrfs: move trans_handle_cachep out of ctree.h
>    btrfs: move btrfs_path_cachep out of ctree.h
>    btrfs: move free space cachep's out of ctree.h
>    btrfs: move btrfs_next_old_item into ctree.c
>    btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h
> 
>   fs/btrfs/block-group.c          |  12 +
>   fs/btrfs/block-group.h          |  11 +-
>   fs/btrfs/btrfs_inode.h          |  26 ---
>   fs/btrfs/ctree.c                |  26 +++
>   fs/btrfs/ctree.h                | 388 ++------------------------------
>   fs/btrfs/delayed-inode.c        |   1 +
>   fs/btrfs/disk-io.c              |   7 +
>   fs/btrfs/disk-io.h              |   8 +-
>   fs/btrfs/free-space-cache.c     |  28 +++
>   fs/btrfs/free-space-cache.h     |  11 +
>   fs/btrfs/inode-item.c           |   1 +
>   fs/btrfs/inode.c                |  57 ++---
>   fs/btrfs/props.c                |   1 +
>   fs/btrfs/relocation.c           |   1 +
>   fs/btrfs/scrub.c                |  11 +
>   fs/btrfs/space-info.h           |  59 +++++
>   fs/btrfs/super.c                |  24 +-
>   fs/btrfs/transaction.c          |  17 ++
>   fs/btrfs/transaction.h          |   2 +
>   fs/btrfs/volumes.c              |   7 +
>   fs/btrfs/volumes.h              |   2 +-
>   include/uapi/linux/btrfs_tree.h | 226 +++++++++++++++++++
>   22 files changed, 476 insertions(+), 450 deletions(-)
> 

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 01/17] btrfs: remove set/clear_pending_info helpers
  2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
@ 2022-09-15 14:11   ` Johannes Thumshirn
  2022-10-07 16:51   ` David Sterba
  2 siblings, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:11 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH
  2022-09-14 15:06 ` [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
@ 2022-09-15 14:11   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:11 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA
  2022-09-14 15:06 ` [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
@ 2022-09-15 14:12   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:12 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h
  2022-09-14 15:06 ` [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h Josef Bacik
  2022-09-15  9:10   ` Qu Wenruo
@ 2022-09-15 14:14   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:14 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h
  2022-09-14 15:06 ` [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h Josef Bacik
  2022-09-15  9:10   ` Qu Wenruo
@ 2022-09-15 14:15   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:15 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
In case the David approves the other 'move to btrfs_tree.h' patch.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c
  2022-09-14 15:06 ` [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c Josef Bacik
  2022-09-15  9:11   ` Qu Wenruo
@ 2022-09-15 14:16   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:16 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h
  2022-09-14 15:06 ` [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h Josef Bacik
  2022-09-15  9:13   ` Qu Wenruo
@ 2022-09-15 14:18   ` Johannes Thumshirn
  2022-10-07 17:17     ` David Sterba
  2022-10-07 17:16   ` David Sterba
  2 siblings, 1 reply; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:18 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

On 14.09.22 17:07, Josef Bacik wrote:
>  
> +/*
> + * Deltas are an effective way to populate global statistics.  Give macro names
> + * to make it clear what we're doing.  An example is discard_extents in
> + * btrfs_free_space_ctl.
> + */
> +#define BTRFS_STAT_NR_ENTRIES	2
> +#define BTRFS_STAT_CURR		0
> +#define BTRFS_STAT_PREV		1

I get this is a plain code movement patch, but can we please get
enum {
	BTRFS_STAT_CURR,
	BTRFS_STAT_PREV,
	BTRFS_STAT_NR_ENTRIES
};

Other wise looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h
  2022-09-14 15:06 ` [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h Josef Bacik
  2022-09-15  9:14   ` Qu Wenruo
@ 2022-09-15 14:19   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:19 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c
  2022-09-14 15:06 ` [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c Josef Bacik
  2022-09-15  9:16   ` Qu Wenruo
@ 2022-09-15 14:21   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:21 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 11/17] btrfs: move flush related definitions to space-info.h
  2022-09-14 15:06 ` [PATCH 11/17] btrfs: move flush related definitions to space-info.h Josef Bacik
  2022-09-15  9:21   ` Qu Wenruo
@ 2022-09-15 14:21   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:21 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c
  2022-09-14 15:06 ` [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c Josef Bacik
  2022-09-15  9:22   ` Qu Wenruo
@ 2022-09-15 14:23   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:23 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h
  2022-09-14 15:06 ` [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h Josef Bacik
  2022-09-15  9:23   ` Qu Wenruo
@ 2022-09-15 14:24   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:24 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 14/17] btrfs: move btrfs_path_cachep out of ctree.h
  2022-09-14 15:06 ` [PATCH 14/17] btrfs: move btrfs_path_cachep " Josef Bacik
  2022-09-15  9:27   ` Qu Wenruo
@ 2022-09-15 14:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:27 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 15/17] btrfs: move free space cachep's out of ctree.h
  2022-09-14 15:06 ` [PATCH 15/17] btrfs: move free space cachep's " Josef Bacik
  2022-09-15  9:27   ` Qu Wenruo
@ 2022-09-15 14:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:27 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c
  2022-09-14 15:06 ` [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c Josef Bacik
  2022-09-15  9:29   ` Qu Wenruo
@ 2022-09-15 14:29   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:29 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h
  2022-09-14 15:06 ` [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h Josef Bacik
  2022-09-15  9:30   ` Qu Wenruo
@ 2022-09-15 14:30   ` Johannes Thumshirn
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Thumshirn @ 2022-09-15 14:30 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs, kernel-team

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

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 01/17] btrfs: remove set/clear_pending_info helpers
  2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
  2022-09-15  9:03   ` Qu Wenruo
  2022-09-15 14:11   ` Johannes Thumshirn
@ 2022-10-07 16:51   ` David Sterba
  2 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 16:51 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs, kernel-team

On Wed, Sep 14, 2022 at 11:06:25AM -0400, Josef Bacik wrote:
> The last users of these helpers were removed in
> 
> 5297199a8bca ("btrfs: remove inode number cache feature")
> 
> so delete these helpers.

I've added a comment what was the purpose, basically for mount options
that must be applied during transaction commit, but we don't have any.
Patch can be reverted if needed.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h
  2022-09-15  9:07   ` Qu Wenruo
@ 2022-10-07 17:07     ` David Sterba
  2022-10-07 23:50       ` Qu Wenruo
  0 siblings, 1 reply; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:07 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Josef Bacik, linux-btrfs, kernel-team

On Thu, Sep 15, 2022 at 05:07:42PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/9/14 23:06, Josef Bacik wrote:
> > The bulk of our on-disk definitions exist in btrfs_tree.h, which user
> > space can use.
> 
> Previously I tried to move some members to btrfs_tree.h, but didn't get 
> approved, mostly due to the fact that, we have those members exposed 
> through uapi is for TREE_SEARCH ioctl.
> 
> But I'm not buying that reason at all.
> 
> To me, all on-disk format, no matter if it's exposed through tree-search 
> should be in btrfs_tree.h.

All the structures are internal and not a guaranteed public API, we may
want to change them any time. So if we move the definitions to UAPI then
with a disclaimer that it's not a stable api and any compilation
failures outside of kernel are up to the users to fix.

Which does not work in practice as easy as said and we have reverted
some changes. See 34c51814b2b8 ("btrfs: re-instantiate the removed
BTRFS_SUBVOL_CREATE_ASYNC definition").

> Although I'd prefer to rename btrfs_tree.h to btrfs_ondisk_format.h.
> 
> Thus to David:
> 
> Can we make it clear that, btrfs_tree.h is not only for tree search 
> ioctl, but also all the on-disk format thing?

It is for on-disk format defitions, but that's a different problem than
the internal/external API.

> Reject once that's fine, but reject twice from two different guys, I 
> think it's not correct.
> 
> >  Keep things consistent and move the rest of the on disk
> > definitions out of ctree.h into btrfs_tree.h.  Note I did have to update
> > all u8's to __u8, but otherwise this is a strict copy and paste.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Reviewed-by: Qu Wenruo <wqu@suse.com>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h
  2022-09-14 15:06 ` [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h Josef Bacik
  2022-09-15  9:13   ` Qu Wenruo
  2022-09-15 14:18   ` Johannes Thumshirn
@ 2022-10-07 17:16   ` David Sterba
  2 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:16 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs, kernel-team

On Wed, Sep 14, 2022 at 11:06:32AM -0400, Josef Bacik wrote:
> These definitions are used for discard statistics, move them out of
> ctree.h and put them in free-space-cache.h.

Maybe it's in another patchset, but I'll note it here, there are some
discard related defitions in ctree.h so the BTRFS_STAT_* should go there
once we have such header.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h
  2022-09-15 14:18   ` Johannes Thumshirn
@ 2022-10-07 17:17     ` David Sterba
  0 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:17 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Josef Bacik, linux-btrfs, kernel-team

On Thu, Sep 15, 2022 at 02:18:40PM +0000, Johannes Thumshirn wrote:
> On 14.09.22 17:07, Josef Bacik wrote:
> >  
> > +/*
> > + * Deltas are an effective way to populate global statistics.  Give macro names
> > + * to make it clear what we're doing.  An example is discard_extents in
> > + * btrfs_free_space_ctl.
> > + */
> > +#define BTRFS_STAT_NR_ENTRIES	2
> > +#define BTRFS_STAT_CURR		0
> > +#define BTRFS_STAT_PREV		1
> 
> I get this is a plain code movement patch, but can we please get
> enum {
> 	BTRFS_STAT_CURR,
> 	BTRFS_STAT_PREV,
> 	BTRFS_STAT_NR_ENTRIES
> };

There are still some defines that could be enums so let's do that
separately and in one go.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h
  2022-09-15  9:14   ` Qu Wenruo
@ 2022-10-07 17:23     ` David Sterba
  0 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:23 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Josef Bacik, linux-btrfs, kernel-team

On Thu, Sep 15, 2022 at 05:14:14PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/9/14 23:06, Josef Bacik wrote:
> > This is more of a volumes related helper, additionally it has a BUG_ON()
> > which isn't defined in the related header.  Move the code to volumes.c,
> > change the BUG_ON() to an ASSERT(), and move the prototype to volumes.h.
> 
> Again a very small function, can it be inlined instead?

I agree, such simple helpers it makes sense. The reason here is the
BUG_ON/ASSERT that's now definied in ctree.h so that would require some
more shuffling.

The assert makes some sense but in many cases the value is validated
before use so we might safely drop it.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 11/17] btrfs: move flush related definitions to space-info.h
  2022-09-15  9:21   ` Qu Wenruo
@ 2022-10-07 17:28     ` David Sterba
  0 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:28 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Josef Bacik, linux-btrfs, kernel-team

On Thu, Sep 15, 2022 at 05:21:16PM +0800, Qu Wenruo wrote:
> >   #include "compression.h"
> > +#include "space-info.h"
> >   
> >   #define BTRFS_PROP_HANDLERS_HT_BITS 8
> >   static DEFINE_HASHTABLE(prop_handlers_ht, BTRFS_PROP_HANDLERS_HT_BITS);
> 
> >   struct btrfs_space_info {
> >   	spinlock_t lock;
> >   
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index be7df8d1d5b8..2add5b23c476 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -49,6 +49,7 @@
> >   #include "discard.h"
> >   #include "qgroup.h"
> >   #include "raid56.h"
> > +#include "space-info.h"
> 
> Why super.c needs this header?
> 
> The moved code is definition for btrfs_reserve_flush_enum and 
> btrfs_flush_state, but I didn't see any direct usage inside super.c.

It still compiles without the header and I don't think it should be
included, I don't see any usage of the space-info.h definitions.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c
  2022-09-15  9:22   ` Qu Wenruo
@ 2022-10-07 17:31     ` David Sterba
  0 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:31 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Josef Bacik, linux-btrfs, kernel-team

On Thu, Sep 15, 2022 at 05:22:48PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/9/14 23:06, Josef Bacik wrote:
> > This isn't used outside of inode.c, there's no reason to define it in
> > btrfs_inode.h.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> Just a small nitpick below.
> 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 6fde13f62c1d..998d1c7134ff 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -125,6 +125,31 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start,
> >   				       u64 ram_bytes, int compress_type,
> >   				       int type);
> >   
> > +static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
> 
> IIRC for static function there is no need for explicit inline keyword.
> 
> Under most cases the compiler should be more clever than us.

For error printing function we don't need inlining at all and if we'd
want to micro optimize the function can be put to __cold section.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff
  2022-09-15  9:47 ` [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Qu Wenruo
@ 2022-10-07 17:51   ` David Sterba
  0 siblings, 0 replies; 61+ messages in thread
From: David Sterba @ 2022-10-07 17:51 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Josef Bacik, linux-btrfs, kernel-team

On Thu, Sep 15, 2022 at 05:47:59PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/9/14 23:06, Josef Bacik wrote:
> > Hello,
> > 
> > This is the first part in what will probably be very many parts in my work to
> > cleanup ctree.h.  These are the smaller changes, mostly removing code that's not
> > used anymore, moving some stuff that's local to C files that don't need to be in
> > the header at all, and moving the rest of the on-disk definition stuff to
> > btrfs_tree.h.  There's a lot of patche here, but they're all relatively small,
> > the largest being the patch to move the on-disk definitions to btrfs_tree.h,
> > which is not very large compared to patches in the next several series.  This
> > has been built and tested, it's relatively low risk.  Thanks,
> 
> Looks good overall to me.
> 
> Just 3 small points of concern:
> 
> - About btrfs_tree.h usage.
> 
>    Really hope David can make it clear that, that UAPI header is for ALL
>    on-disk format definition.
> 
>    I'm not buying the old reason that the UAPI header is only for tree
>    search ioctl, and really want to move the whole super block definition
>    into UAPI header.

I have merged the patch but am expecting problems once people start
using the headers. I'll be addressed case by case.

>    (And I also think all upstream fses should have a concentrated UAPI
>     header too, just to make new comers easier to hack)
> 
> - Some tiny inline functions got unexported
> 
>    May not be a big thing though, just want to make sure we have no other
>    choice but make them uninlined.

This depends, trivial functions or where it's on a hot path it's better
to keep inline.

> 
> - Extra error tags in init_btrfs_fs()
                ^^^^
                labels

>    Just like open_ctree(), it's when but not whether to have wrong jump.
>    Thus a new series to make those cachep related exit function
>    conditional. (aka, we can call them unconditionally at error path)

You've sent the series that converts it to the array and we're going to
it that way. Please refresh the series as there were some minor changes
to the init/exit functions and resend. Thanks.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h
  2022-10-07 17:07     ` David Sterba
@ 2022-10-07 23:50       ` Qu Wenruo
  0 siblings, 0 replies; 61+ messages in thread
From: Qu Wenruo @ 2022-10-07 23:50 UTC (permalink / raw)
  To: dsterba; +Cc: Josef Bacik, linux-btrfs, kernel-team



On 2022/10/8 01:07, David Sterba wrote:
> On Thu, Sep 15, 2022 at 05:07:42PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2022/9/14 23:06, Josef Bacik wrote:
>>> The bulk of our on-disk definitions exist in btrfs_tree.h, which user
>>> space can use.
>>
>> Previously I tried to move some members to btrfs_tree.h, but didn't get
>> approved, mostly due to the fact that, we have those members exposed
>> through uapi is for TREE_SEARCH ioctl.
>>
>> But I'm not buying that reason at all.
>>
>> To me, all on-disk format, no matter if it's exposed through tree-search
>> should be in btrfs_tree.h.
> 
> All the structures are internal and not a guaranteed public API, we may
> want to change them any time. So if we move the definitions to UAPI then
> with a disclaimer that it's not a stable api and any compilation
> failures outside of kernel are up to the users to fix.

The point is, if we're changing the super block format, it's no 
difference than changing a on-disk structure.

It's still the same incompat/compat_ro/compat change depending on the 
member we change.

> 
> Which does not work in practice as easy as said and we have reverted
> some changes. See 34c51814b2b8 ("btrfs: re-instantiate the removed
> BTRFS_SUBVOL_CREATE_ASYNC definition").

That commit indeed teaches us something, even if we deprecated some 
features, it still has to be kept in the UAPI.

But that argument doesn't really affect the super block move AFAIK.

Since it's not even part of an ioctl, thus I don't think user-space tool 
would really both that.

> 
>> Although I'd prefer to rename btrfs_tree.h to btrfs_ondisk_format.h.
>>
>> Thus to David:
>>
>> Can we make it clear that, btrfs_tree.h is not only for tree search
>> ioctl, but also all the on-disk format thing?
> 
> It is for on-disk format defitions, but that's a different problem than
> the internal/external API.

Then, what's the proper way to export btrfs on-disk format?

 From an ioctl point of view, all those on-disk format things don't even 
need to be exported.

TREE_SEARCH ioctl is just returning a certain TLV formatted memory.
Yes, the basic things like btrfs_ioctl_search_header, but the content is 
still internal, not directly user-facing.

All the returned TLV members are really implementation related details, 
and even the TREE_SEARCH ioctl itself is more like a debug tool than 
proper interface.
A lot of things are done using TREE_SEARCH ioctl because we don't have 
better more defined interface.

Thanks,
Qu

> 
>> Reject once that's fine, but reject twice from two different guys, I
>> think it's not correct.
>>
>>>   Keep things consistent and move the rest of the on disk
>>> definitions out of ctree.h into btrfs_tree.h.  Note I did have to update
>>> all u8's to __u8, but otherwise this is a strict copy and paste.
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> Reviewed-by: Qu Wenruo <wqu@suse.com>

^ permalink raw reply	[flat|nested] 61+ messages in thread

end of thread, other threads:[~2022-10-07 23:51 UTC | newest]

Thread overview: 61+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-14 15:06 [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Josef Bacik
2022-09-14 15:06 ` [PATCH 01/17] btrfs: remove set/clear_pending_info helpers Josef Bacik
2022-09-15  9:03   ` Qu Wenruo
2022-09-15 14:11   ` Johannes Thumshirn
2022-10-07 16:51   ` David Sterba
2022-09-14 15:06 ` [PATCH 02/17] btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH Josef Bacik
2022-09-15  9:03   ` Qu Wenruo
2022-09-15 14:11   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 03/17] btrfs: remove BTRFS_IOPRIO_READA Josef Bacik
2022-09-15  9:03   ` Qu Wenruo
2022-09-15 14:12   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 04/17] btrfs: move btrfs on disk definitions out of ctree.h Josef Bacik
2022-09-15  9:07   ` Qu Wenruo
2022-10-07 17:07     ` David Sterba
2022-10-07 23:50       ` Qu Wenruo
2022-09-14 15:06 ` [PATCH 05/17] btrfs: move btrfs_get_block_group helper out of disk-io.h Josef Bacik
2022-09-15  9:10   ` Qu Wenruo
2022-09-15 14:14   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 06/17] btrfs: move maximum limits to btrfs_tree.h Josef Bacik
2022-09-15  9:10   ` Qu Wenruo
2022-09-15 14:15   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 07/17] btrfs: move BTRFS_MAX_MIRRORS into scrub.c Josef Bacik
2022-09-15  9:11   ` Qu Wenruo
2022-09-15 14:16   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 08/17] btrfs: move discard stat defs to free-space-cache.h Josef Bacik
2022-09-15  9:13   ` Qu Wenruo
2022-09-15 14:18   ` Johannes Thumshirn
2022-10-07 17:17     ` David Sterba
2022-10-07 17:16   ` David Sterba
2022-09-14 15:06 ` [PATCH 09/17] btrfs: move btrfs_chunk_item_size out of ctree.h Josef Bacik
2022-09-15  9:14   ` Qu Wenruo
2022-10-07 17:23     ` David Sterba
2022-09-15 14:19   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 10/17] btrfs: move btrfs_should_fragment_free_space into block-group.c Josef Bacik
2022-09-15  9:16   ` Qu Wenruo
2022-09-15 14:21   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 11/17] btrfs: move flush related definitions to space-info.h Josef Bacik
2022-09-15  9:21   ` Qu Wenruo
2022-10-07 17:28     ` David Sterba
2022-09-15 14:21   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 12/17] btrfs: move btrfs_print_data_csum_error into inode.c Josef Bacik
2022-09-15  9:22   ` Qu Wenruo
2022-10-07 17:31     ` David Sterba
2022-09-15 14:23   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 13/17] btrfs: move trans_handle_cachep out of ctree.h Josef Bacik
2022-09-15  9:23   ` Qu Wenruo
2022-09-15 14:24   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 14/17] btrfs: move btrfs_path_cachep " Josef Bacik
2022-09-15  9:27   ` Qu Wenruo
2022-09-15 14:27   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 15/17] btrfs: move free space cachep's " Josef Bacik
2022-09-15  9:27   ` Qu Wenruo
2022-09-15 14:27   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 16/17] btrfs: move btrfs_next_old_item into ctree.c Josef Bacik
2022-09-15  9:29   ` Qu Wenruo
2022-09-15 14:29   ` Johannes Thumshirn
2022-09-14 15:06 ` [PATCH 17/17] btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h Josef Bacik
2022-09-15  9:30   ` Qu Wenruo
2022-09-15 14:30   ` Johannes Thumshirn
2022-09-15  9:47 ` [PATCH 00/17] btrfs: initial ctree.h cleanups, simple stuff Qu Wenruo
2022-10-07 17:51   ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.