All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] btrfs: Reformat and make btrfs_tree.h self-contained
@ 2020-04-15  8:41 Qu Wenruo
  2020-04-15  8:41 ` [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h Qu Wenruo
  2020-04-15  8:41 ` [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments Qu Wenruo
  0 siblings, 2 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-04-15  8:41 UTC (permalink / raw)
  To: linux-btrfs

Before this patch, although btrfs_tree.h contains most of needed
structures for one to read on-disk btrfs data, it still has some missing
parts:
- Missing some structures and flags
  Like btrfs_super_block, and BTRFS_INODE_* flags

- Not self-contained
  It still needs to include <linux/btrfs.h>

- Uses old/deprecated comment style

This patchset will first move on-disk structure definition to
btrfs_tree.h first, making it self-contained.

Then reformat btrfs_tree.h to make it follow current comment style.

This patchset is mostly designed for incoming projects like U-boot to
share the kernel definitions more easily.

Changelog:
v2:
- Add the reason why we move the code

v3:
- Move more flags/enum shared with ioctl to btrfs_btree.h
  This makes ioctl header to rely on btree_btree.h.
  But this makes btrfs_tree.h completely self-consistent.
  This problem is mostly exposed when syncing the header to btrfs-progs.

v4:
- Update btrfs_tree.h comment style
- Update btrfs_tree.h header define line
- Move enum btrfs_compression_type to btrfs_tree.h

Qu Wenruo (2):
  btrfs: Move on-disk structure definition to btrfs_tree.h
  btrfs: Reformat btrfs_tree.h comments

 fs/btrfs/compression.h          |   9 +-
 fs/btrfs/ctree.h                | 246 -----------
 include/uapi/linux/btrfs.h      |  74 +---
 include/uapi/linux/btrfs_tree.h | 721 +++++++++++++++++++++++---------
 4 files changed, 532 insertions(+), 518 deletions(-)

-- 
2.26.0


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

* [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-15  8:41 [PATCH v4 0/2] btrfs: Reformat and make btrfs_tree.h self-contained Qu Wenruo
@ 2020-04-15  8:41 ` Qu Wenruo
  2020-04-21 11:31   ` Christoph Hellwig
  2020-04-23 11:32     ` kbuild test robot
  2020-04-15  8:41 ` [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments Qu Wenruo
  1 sibling, 2 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-04-15  8:41 UTC (permalink / raw)
  To: linux-btrfs

These structures all are on-disk format. Move them to btrfs_tree.h

This allows us to sync the header to different projects, who need to
read btrfs filesystem, like U-boot, grub.

With this modification, all on-disk format is definite in btrfs_tree.h,
and can be easily synced to other projects.

This move includes:
- btrfs magic
  It's a surprise that it's not even definied in btrfs_tree.h

- tree block max level
  Move it before btrfs_header definition.

- tree block backref revision
- btrfs_header structure
- btrfs_root_backup structure
- btrfs_super_block structure
- BTRFS_FEATURE_* flags
- BTRFS_(FSID|UUID|LABEL)_SIZE macros
- BTRFS_MAX_METADATA_BLOCKSIZE macro
- BTRFS_NAME_LEN macro

- btrfs_item structure
- btrfs_leaf structure
- btrfs_key_ptr structure
- btrfs_node structure

- btrfs_dev_stat_values
  Since on-disk format btrfs_dev_stats_item needs it.

- BTRFS_INODE_* flags
- BTRFS_QGROUP_LIMIT_* flags

- enum btrfs_compression_type
  With this movement, btrfs_tree is completely self-contained.
  It only needs <linux/types.h> header.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/compression.h          |   9 +-
 fs/btrfs/ctree.h                | 246 -----------------------
 include/uapi/linux/btrfs.h      |  74 +------
 include/uapi/linux/btrfs_tree.h | 338 +++++++++++++++++++++++++++++++-
 4 files changed, 336 insertions(+), 331 deletions(-)

diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index d253f7aa8ed5..65f6f31a7ec4 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -7,6 +7,7 @@
 #define BTRFS_COMPRESSION_H
 
 #include <linux/sizes.h>
+#include <linux/btrfs_tree.h>
 
 /*
  * We want to make sure that amount of RAM required to uncompress an extent is
@@ -100,14 +101,6 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 
 unsigned int btrfs_compress_str2level(unsigned int type, const char *str);
 
-enum btrfs_compression_type {
-	BTRFS_COMPRESS_NONE  = 0,
-	BTRFS_COMPRESS_ZLIB  = 1,
-	BTRFS_COMPRESS_LZO   = 2,
-	BTRFS_COMPRESS_ZSTD  = 3,
-	BTRFS_NR_COMPRESS_TYPES = 4,
-};
-
 struct workspace_manager {
 	struct list_head idle_ws;
 	spinlock_t ws_lock;
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8aa7b9dac405..4d787d749315 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -49,8 +49,6 @@ extern struct kmem_cache *btrfs_free_space_bitmap_cachep;
 struct btrfs_ordered_sum;
 struct btrfs_ref;
 
-#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
@@ -62,22 +60,8 @@ struct btrfs_ref;
  */
 #define BTRFS_MAX_MIRRORS (4 + 1)
 
-#define BTRFS_MAX_LEVEL 8
-
 #define BTRFS_OLDEST_GENERATION	0ULL
 
-/*
- * the max metadata block size.  This limit is somewhat artificial,
- * but the memmove costs go through the roof for larger blocks.
- */
-#define BTRFS_MAX_METADATA_BLOCKSIZE 65536
-
-/*
- * 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
@@ -148,203 +132,6 @@ enum {
 	BTRFS_FS_STATE_DUMMY_FS_INFO,
 };
 
-#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__));
-
-/*
- * 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 will help find the new super based on the log root */
-	__le64 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 */
-	__le64 reserved[28];
-	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
-	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
-} __attribute__ ((__packed__));
-
-/*
- * Compat flags that we support.  If any incompat flags are set other than the
- * ones specified below then we will fail to mount
- */
-#define BTRFS_FEATURE_COMPAT_SUPP		0ULL
-#define BTRFS_FEATURE_COMPAT_SAFE_SET		0ULL
-#define BTRFS_FEATURE_COMPAT_SAFE_CLEAR		0ULL
-
-#define BTRFS_FEATURE_COMPAT_RO_SUPP			\
-	(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |	\
-	 BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID)
-
-#define BTRFS_FEATURE_COMPAT_RO_SAFE_SET	0ULL
-#define BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR	0ULL
-
-#define BTRFS_FEATURE_INCOMPAT_SUPP			\
-	(BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF |		\
-	 BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL |	\
-	 BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS |		\
-	 BTRFS_FEATURE_INCOMPAT_BIG_METADATA |		\
-	 BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO |		\
-	 BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD |		\
-	 BTRFS_FEATURE_INCOMPAT_RAID56 |		\
-	 BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF |		\
-	 BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA |	\
-	 BTRFS_FEATURE_INCOMPAT_NO_HOLES	|	\
-	 BTRFS_FEATURE_INCOMPAT_METADATA_UUID	|	\
-	 BTRFS_FEATURE_INCOMPAT_RAID1C34)
-
-#define BTRFS_FEATURE_INCOMPAT_SAFE_SET			\
-	(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__));
-
 /*
  * btrfs_paths remember the path taken from the root down to the leaf.
  * level 0 is always the leaf, and nodes[1...BTRFS_MAX_LEVEL] will point
@@ -1307,39 +1094,6 @@ do {                                                                   \
        }                                                               \
 } while(0)
 
-/*
- * Inode flags
- */
-#define BTRFS_INODE_NODATASUM		(1 << 0)
-#define BTRFS_INODE_NODATACOW		(1 << 1)
-#define BTRFS_INODE_READONLY		(1 << 2)
-#define BTRFS_INODE_NOCOMPRESS		(1 << 3)
-#define BTRFS_INODE_PREALLOC		(1 << 4)
-#define BTRFS_INODE_SYNC		(1 << 5)
-#define BTRFS_INODE_IMMUTABLE		(1 << 6)
-#define BTRFS_INODE_APPEND		(1 << 7)
-#define BTRFS_INODE_NODUMP		(1 << 8)
-#define BTRFS_INODE_NOATIME		(1 << 9)
-#define BTRFS_INODE_DIRSYNC		(1 << 10)
-#define BTRFS_INODE_COMPRESS		(1 << 11)
-
-#define BTRFS_INODE_ROOT_ITEM_INIT	(1 << 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)
-
 struct btrfs_map_token {
 	const struct extent_buffer *eb;
 	char *kaddr;
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 8134924cfc17..69c2c21e7f7b 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -21,10 +21,10 @@
 #define _UAPI_LINUX_BTRFS_H
 #include <linux/types.h>
 #include <linux/ioctl.h>
+#include <linux/btrfs_tree.h>
 
 #define BTRFS_IOCTL_MAGIC 0x94
 #define BTRFS_VOL_NAME_MAX 255
-#define BTRFS_LABEL_SIZE 256
 
 /* this should be 4k */
 #define BTRFS_PATH_NAME_MAX 4087
@@ -55,24 +55,8 @@ struct btrfs_ioctl_vol_args {
 			BTRFS_DEVICE_SPEC_BY_ID |	\
 			BTRFS_SUBVOL_SPEC_BY_ID)
 
-#define BTRFS_FSID_SIZE 16
-#define BTRFS_UUID_SIZE 16
 #define BTRFS_UUID_UNPARSED_SIZE	37
 
-/*
- * flags definition for qgroup limits
- *
- * Used by:
- * struct btrfs_qgroup_limit.flags
- * struct btrfs_qgroup_limit_item.flags
- */
-#define BTRFS_QGROUP_LIMIT_MAX_RFER	(1ULL << 0)
-#define BTRFS_QGROUP_LIMIT_MAX_EXCL	(1ULL << 1)
-#define BTRFS_QGROUP_LIMIT_RSV_RFER	(1ULL << 2)
-#define BTRFS_QGROUP_LIMIT_RSV_EXCL	(1ULL << 3)
-#define BTRFS_QGROUP_LIMIT_RFER_CMPR	(1ULL << 4)
-#define BTRFS_QGROUP_LIMIT_EXCL_CMPR	(1ULL << 5)
-
 struct btrfs_qgroup_limit {
 	__u64	flags;
 	__u64	max_rfer;
@@ -256,43 +240,6 @@ struct btrfs_ioctl_fs_info_args {
 	__u64 reserved[122];			/* pad to 1k */
 };
 
-/*
- * feature flags
- *
- * Used by:
- * struct btrfs_ioctl_feature_flags
- */
-#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE		(1ULL << 0)
-/*
- * Older kernels (< 4.9) on big-endian systems produced broken free space tree
- * bitmaps, and btrfs-progs also used to corrupt the free space tree (versions
- * < 4.7.3).  If this bit is clear, then the free space tree cannot be trusted.
- * btrfs-progs can also intentionally clear this bit to ask the kernel to
- * rebuild the free space tree, however this might not work on older kernels
- * that do not know about this bit. If not sure, clear the cache manually on
- * first mount when booting older kernel versions.
- */
-#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID	(1ULL << 1)
-
-#define BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF	(1ULL << 0)
-#define BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL	(1ULL << 1)
-#define BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS	(1ULL << 2)
-#define BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO	(1ULL << 3)
-#define BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD	(1ULL << 4)
-
-/*
- * older kernels tried to do bigger metadata blocks, but the
- * code was pretty buggy.  Lets not let them try anymore.
- */
-#define BTRFS_FEATURE_INCOMPAT_BIG_METADATA	(1ULL << 5)
-
-#define BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF	(1ULL << 6)
-#define BTRFS_FEATURE_INCOMPAT_RAID56		(1ULL << 7)
-#define BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA	(1ULL << 8)
-#define BTRFS_FEATURE_INCOMPAT_NO_HOLES		(1ULL << 9)
-#define BTRFS_FEATURE_INCOMPAT_METADATA_UUID	(1ULL << 10)
-#define BTRFS_FEATURE_INCOMPAT_RAID1C34		(1ULL << 11)
-
 struct btrfs_ioctl_feature_flags {
 	__u64 compat_flags;
 	__u64 compat_ro_flags;
@@ -657,25 +604,6 @@ struct btrfs_ioctl_logical_ino_args {
  * Requires logical == extent bytenr. */
 #define BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET	(1ULL << 0)
 
-enum btrfs_dev_stat_values {
-	/* disk I/O failure stats */
-	BTRFS_DEV_STAT_WRITE_ERRS, /* EIO or EREMOTEIO from lower layers */
-	BTRFS_DEV_STAT_READ_ERRS, /* EIO or EREMOTEIO from lower layers */
-	BTRFS_DEV_STAT_FLUSH_ERRS, /* EIO or EREMOTEIO from lower layers */
-
-	/* stats for indirect indications for I/O failures */
-	BTRFS_DEV_STAT_CORRUPTION_ERRS, /* checksum error, bytenr error or
-					 * contents is illegal: this is an
-					 * indication that the block was damaged
-					 * during read or write, or written to
-					 * wrong location or read from wrong
-					 * location */
-	BTRFS_DEV_STAT_GENERATION_ERRS, /* an indication that blocks have not
-					 * been written */
-
-	BTRFS_DEV_STAT_VALUES_MAX
-};
-
 /* Reset statistics after reading; needs SYS_ADMIN capability */
 #define	BTRFS_DEV_STATS_RESET		(1ULL << 0)
 
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 8e322e2c7e78..1170be498c43 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -2,9 +2,26 @@
 #ifndef _BTRFS_CTREE_H_
 #define _BTRFS_CTREE_H_
 
-#include <linux/btrfs.h>
 #include <linux/types.h>
 
+#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
+
+/*
+ * The max metadata block size (node size).
+ *
+ * This limit is somewhat artificial. The memmove and tree block locking cost
+ * go up with larger node size.
+ */
+#define BTRFS_MAX_METADATA_BLOCKSIZE 65536
+
+/*
+ * We can actually store much bigger names, but lets not confuse the rest
+ * of linux.
+ *
+ * btrfs_dir_item::name_len follows this limitation.
+ */
+#define BTRFS_NAME_LEN 255
+
 /*
  * This header contains the structure definitions and constants used
  * by file system objects that can be retrieved using
@@ -281,9 +298,6 @@
  * The key is built like this:
  * (UUID_upper_64_bits, BTRFS_UUID_KEY*, UUID_lower_64_bits).
  */
-#if BTRFS_UUID_SIZE != 16
-#error "UUID items require BTRFS_UUID_SIZE == 16!"
-#endif
 #define BTRFS_UUID_KEY_SUBVOL	251	/* for UUIDs assigned to subvols */
 #define BTRFS_UUID_KEY_RECEIVED_SUBVOL	252	/* for UUIDs assigned to
 						 * received subvols */
@@ -326,6 +340,9 @@ enum btrfs_csum_type {
 #define BTRFS_FT_XATTR		8
 #define BTRFS_FT_MAX		9
 
+#define BTRFS_FSID_SIZE 16
+#define BTRFS_UUID_SIZE 16
+
 /*
  * The key defines the order in the tree, and so it also defines (optimal)
  * block layout.
@@ -559,6 +576,39 @@ struct btrfs_timespec {
 	__le32 nsec;
 } __attribute__ ((__packed__));
 
+/*
+ * Inode flags
+ */
+#define BTRFS_INODE_NODATASUM		(1 << 0)
+#define BTRFS_INODE_NODATACOW		(1 << 1)
+#define BTRFS_INODE_READONLY		(1 << 2)
+#define BTRFS_INODE_NOCOMPRESS		(1 << 3)
+#define BTRFS_INODE_PREALLOC		(1 << 4)
+#define BTRFS_INODE_SYNC		(1 << 5)
+#define BTRFS_INODE_IMMUTABLE		(1 << 6)
+#define BTRFS_INODE_APPEND		(1 << 7)
+#define BTRFS_INODE_NODUMP		(1 << 8)
+#define BTRFS_INODE_NOATIME		(1 << 9)
+#define BTRFS_INODE_DIRSYNC		(1 << 10)
+#define BTRFS_INODE_COMPRESS		(1 << 11)
+
+#define BTRFS_INODE_ROOT_ITEM_INIT	(1 << 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)
+
 struct btrfs_inode_item {
 	/* nfs style generation number */
 	__le64 generation;
@@ -747,6 +797,14 @@ enum {
 	BTRFS_NR_FILE_EXTENT_TYPES = 3,
 };
 
+enum btrfs_compression_type {
+	BTRFS_COMPRESS_NONE  = 0,
+	BTRFS_COMPRESS_ZLIB  = 1,
+	BTRFS_COMPRESS_LZO   = 2,
+	BTRFS_COMPRESS_ZSTD  = 3,
+	BTRFS_NR_COMPRESS_TYPES = 4,
+};
+
 struct btrfs_file_extent_item {
 	/*
 	 * transaction id that created this extent
@@ -803,6 +861,25 @@ struct btrfs_csum_item {
 	__u8 csum;
 } __attribute__ ((__packed__));
 
+enum btrfs_dev_stat_values {
+	/* disk I/O failure stats */
+	BTRFS_DEV_STAT_WRITE_ERRS, /* EIO or EREMOTEIO from lower layers */
+	BTRFS_DEV_STAT_READ_ERRS, /* EIO or EREMOTEIO from lower layers */
+	BTRFS_DEV_STAT_FLUSH_ERRS, /* EIO or EREMOTEIO from lower layers */
+
+	/* stats for indirect indications for I/O failures */
+	BTRFS_DEV_STAT_CORRUPTION_ERRS, /* checksum error, bytenr error or
+					 * contents is illegal: this is an
+					 * indication that the block was damaged
+					 * during read or write, or written to
+					 * wrong location or read from wrong
+					 * location */
+	BTRFS_DEV_STAT_GENERATION_ERRS, /* an indication that blocks have not
+					 * been written */
+
+	BTRFS_DEV_STAT_VALUES_MAX
+};
+
 struct btrfs_dev_stats_item {
 	/*
 	 * grow this item struct at the end for future enhancements and keep
@@ -974,6 +1051,20 @@ struct btrfs_qgroup_info_item {
 	__le64 excl_cmpr;
 } __attribute__ ((__packed__));
 
+/*
+ * flags definition for qgroup limits
+ *
+ * Used by:
+ * struct btrfs_qgroup_limit.flags
+ * struct btrfs_qgroup_limit_item.flags
+ */
+#define BTRFS_QGROUP_LIMIT_MAX_RFER	(1ULL << 0)
+#define BTRFS_QGROUP_LIMIT_MAX_EXCL	(1ULL << 1)
+#define BTRFS_QGROUP_LIMIT_RSV_RFER	(1ULL << 2)
+#define BTRFS_QGROUP_LIMIT_RSV_EXCL	(1ULL << 3)
+#define BTRFS_QGROUP_LIMIT_RFER_CMPR	(1ULL << 4)
+#define BTRFS_QGROUP_LIMIT_EXCL_CMPR	(1ULL << 5)
+
 struct btrfs_qgroup_limit_item {
 	/*
 	 * only updated when any of the other values change
@@ -985,4 +1076,243 @@ struct btrfs_qgroup_limit_item {
 	__le64 rsv_excl;
 } __attribute__ ((__packed__));
 
+/*
+ * 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__));
+
+/*
+ * 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
+
+#define BTRFS_LABEL_SIZE 256
+/*
+ * 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 will help find the new super based on the log root */
+	__le64 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 */
+	__le64 reserved[28];
+	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
+	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
+} __attribute__ ((__packed__));
+
+/*
+ * feature flags
+ *
+ * Used by:
+ * struct btrfs_super_block::(compat|compat_ro|incompat)_flags
+ * struct btrfs_ioctl_feature_flags
+ */
+#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE		(1ULL << 0)
+
+/*
+ * Older kernels (< 4.9) on big-endian systems produced broken free space tree
+ * bitmaps, and btrfs-progs also used to corrupt the free space tree (versions
+ * < 4.7.3).  If this bit is clear, then the free space tree cannot be trusted.
+ * btrfs-progs can also intentionally clear this bit to ask the kernel to
+ * rebuild the free space tree, however this might not work on older kernels
+ * that do not know about this bit. If not sure, clear the cache manually on
+ * first mount when booting older kernel versions.
+ */
+#define BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID	(1ULL << 1)
+
+#define BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF	(1ULL << 0)
+#define BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL	(1ULL << 1)
+#define BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS	(1ULL << 2)
+#define BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO	(1ULL << 3)
+#define BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD	(1ULL << 4)
+
+/*
+ * older kernels tried to do bigger metadata blocks, but the
+ * code was pretty buggy.  Lets not let them try anymore.
+ */
+#define BTRFS_FEATURE_INCOMPAT_BIG_METADATA	(1ULL << 5)
+
+#define BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF	(1ULL << 6)
+#define BTRFS_FEATURE_INCOMPAT_RAID56		(1ULL << 7)
+#define BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA	(1ULL << 8)
+#define BTRFS_FEATURE_INCOMPAT_NO_HOLES		(1ULL << 9)
+#define BTRFS_FEATURE_INCOMPAT_METADATA_UUID	(1ULL << 10)
+#define BTRFS_FEATURE_INCOMPAT_RAID1C34		(1ULL << 11)
+
+/*
+ * Compat flags that we support.  If any incompat flags are set other than the
+ * ones specified below then we will fail to mount
+ */
+#define BTRFS_FEATURE_COMPAT_SUPP		0ULL
+#define BTRFS_FEATURE_COMPAT_SAFE_SET		0ULL
+#define BTRFS_FEATURE_COMPAT_SAFE_CLEAR		0ULL
+
+#define BTRFS_FEATURE_COMPAT_RO_SUPP			\
+	(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |	\
+	 BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID)
+
+#define BTRFS_FEATURE_COMPAT_RO_SAFE_SET	0ULL
+#define BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR	0ULL
+
+#define BTRFS_FEATURE_INCOMPAT_SUPP			\
+	(BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF |		\
+	 BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL |	\
+	 BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS |		\
+	 BTRFS_FEATURE_INCOMPAT_BIG_METADATA |		\
+	 BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO |		\
+	 BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD |		\
+	 BTRFS_FEATURE_INCOMPAT_RAID56 |		\
+	 BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF |		\
+	 BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA |	\
+	 BTRFS_FEATURE_INCOMPAT_NO_HOLES	|	\
+	 BTRFS_FEATURE_INCOMPAT_METADATA_UUID	|	\
+	 BTRFS_FEATURE_INCOMPAT_RAID1C34)
+
+#define BTRFS_FEATURE_INCOMPAT_SAFE_SET			\
+	(BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF)
+#define BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR		0ULL
+
+#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
+
+#define BTRFS_MAX_LEVEL 8
+
+/*
+ * 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__));
+
+/*
+ * 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__));
+
 #endif /* _BTRFS_CTREE_H_ */
-- 
2.26.0


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

* [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments
  2020-04-15  8:41 [PATCH v4 0/2] btrfs: Reformat and make btrfs_tree.h self-contained Qu Wenruo
  2020-04-15  8:41 ` [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h Qu Wenruo
@ 2020-04-15  8:41 ` Qu Wenruo
  2020-04-16 10:20   ` David Sterba
  2020-04-16 12:54   ` David Sterba
  1 sibling, 2 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-04-15  8:41 UTC (permalink / raw)
  To: linux-btrfs

Since we're here, modify btrfs_tree.h to follow the latest comment
style.

This involves:
- Use upper case char for the first word
- Use one line comment if possible
- Add the ending dot if it's a sentence
- Add more comment explaining the usage of each tree
- Add key type/objectid/offset reference URL
- Remove dead comment
- Update the header define line to reflect the filename
- Add newline to seperate long comment

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 include/uapi/linux/btrfs_tree.h | 455 ++++++++++++++++----------------
 1 file changed, 232 insertions(+), 223 deletions(-)

diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 1170be498c43..d0402359300c 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _BTRFS_CTREE_H_
-#define _BTRFS_CTREE_H_
+#ifndef __BTRFS_TREE_H__
+#define __BTRFS_TREE_H__
 
 #include <linux/types.h>
 
@@ -23,95 +23,98 @@
 #define BTRFS_NAME_LEN 255
 
 /*
- * This header contains the structure definitions and constants used
- * by file system objects that can be retrieved using
- * the BTRFS_IOC_SEARCH_TREE ioctl.  That means basically anything that
- * is needed to describe a leaf node's key or item contents.
+ * Objectids start from here.
+ *
+ * Check btrfs_disk_key for the meaning of objectids.
  */
 
-/* holds pointers to all of the tree roots */
+/*
+ * Root tree holds pointers to all of the tree roots.
+ * Without special mention, the root tree contains the root bytenr of all other 
+ * trees, except the chunk tree and the log tree.
+ *
+ * The super block contains the root bytenr of this tree.
+ */
 #define BTRFS_ROOT_TREE_OBJECTID 1ULL
 
-/* stores information about which extents are in use, and reference counts */
+/*
+ * Extent tree stores information about which extents are in use, and backrefs
+ * for each extent.
+ */
 #define BTRFS_EXTENT_TREE_OBJECTID 2ULL
 
 /*
- * chunk tree stores translations from logical -> physical block numbering
- * the super block points to the chunk tree
+ * Chunk tree stores btrfs logical address -> physical address mapping.
+ *
+ * The super block contains part of chunk tree for bootstrap, and contains
+ * the root bytenr of this tree.
  */
 #define BTRFS_CHUNK_TREE_OBJECTID 3ULL
 
 /*
- * stores information about which areas of a given device are in use.
- * one per device.  The tree of tree roots points to the device tree
+ * Device tree stores info about which areas of a given device are in use,
+ * and physical address -> btrfs logical address mapping.
  */
 #define BTRFS_DEV_TREE_OBJECTID 4ULL
 
-/* one per subvolume, storing files and directories */
+/* The fs tree is the first subvolume tree, storing files and directories. */
 #define BTRFS_FS_TREE_OBJECTID 5ULL
 
-/* directory objectid inside the root tree */
+/* Shows the directory objectid inside the root tree. */
 #define BTRFS_ROOT_TREE_DIR_OBJECTID 6ULL
 
-/* holds checksums of all the data extents */
+/* Csum tree holds checksums of all the data extents. */
 #define BTRFS_CSUM_TREE_OBJECTID 7ULL
 
-/* holds quota configuration and tracking */
+/* Quota tree holds quota configuration and tracking. */
 #define BTRFS_QUOTA_TREE_OBJECTID 8ULL
 
-/* for storing items that use the BTRFS_UUID_KEY* types */
+/* UUID tree stores items that use the BTRFS_UUID_KEY* types. */
 #define BTRFS_UUID_TREE_OBJECTID 9ULL
 
-/* tracks free space in block groups. */
+/* Free space cache tree (v2 space cache) tracks free space in block groups. */
 #define BTRFS_FREE_SPACE_TREE_OBJECTID 10ULL
 
-/* device stats in the device tree */
+/* Indicates device stats in the device tree. */
 #define BTRFS_DEV_STATS_OBJECTID 0ULL
 
-/* for storing balance parameters in the root tree */
+/* For storing balance parameters in the root tree. */
 #define BTRFS_BALANCE_OBJECTID -4ULL
 
-/* orhpan objectid for tracking unlinked/truncated files */
+/* Orhpan objectid for tracking unlinked/truncated files. */
 #define BTRFS_ORPHAN_OBJECTID -5ULL
 
-/* does write ahead logging to speed up fsyncs */
+/* Does write ahead logging to speed up fsyncs. */
 #define BTRFS_TREE_LOG_OBJECTID -6ULL
 #define BTRFS_TREE_LOG_FIXUP_OBJECTID -7ULL
 
-/* for space balancing */
+/* For space balancing. */
 #define BTRFS_TREE_RELOC_OBJECTID -8ULL
 #define BTRFS_DATA_RELOC_TREE_OBJECTID -9ULL
 
-/*
- * extent checksums all have this objectid
- * this allows them to share the logging tree
- * for fsyncs
- */
+/* Extent checksums, shared between the csum tree and log trees. */
 #define BTRFS_EXTENT_CSUM_OBJECTID -10ULL
 
-/* For storing free space cache */
+/* For storing free space cache (v1 space cache). */
 #define BTRFS_FREE_SPACE_OBJECTID -11ULL
 
-/*
- * The inode number assigned to the special inode for storing
- * free ino cache
- */
+/* The inode number assigned to the special inode for storing free ino cache. */
 #define BTRFS_FREE_INO_OBJECTID -12ULL
 
-/* dummy objectid represents multiple objectids */
+/* Dummy objectid represents multiple objectids. */
 #define BTRFS_MULTIPLE_OBJECTIDS -255ULL
 
-/*
- * All files have objectids in this range.
- */
+/* All files have objectids in this range. */
 #define BTRFS_FIRST_FREE_OBJECTID 256ULL
 #define BTRFS_LAST_FREE_OBJECTID -256ULL
 #define BTRFS_FIRST_CHUNK_TREE_OBJECTID 256ULL
 
 
 /*
- * the device items go into the chunk tree.  The key is in the form
- * [ 1 BTRFS_DEV_ITEM_KEY device_id ]
+ * The device items go into the chunk tree.
+ *
+ * The key is in the form
+ * (BTRFS_DEV_ITEMS_OBJECTID, BTRFS_DEV_ITEM_KEY,  <device_id>)
  */
 #define BTRFS_DEV_ITEMS_OBJECTID 1ULL
 
@@ -122,58 +125,68 @@
 #define BTRFS_DEV_REPLACE_DEVID 0ULL
 
 /*
- * inode items have the data typically returned from stat and store other
- * info about object characteristics.  There is one for every file and dir in
- * the FS
+ * Types start from here.
+ *
+ * Check btrfs_disk_key for details about types.
+ */
+
+/*
+ * Inode items have the data typically returned from stat and store other
+ * info about object characteristics.
+ *
+ * There is one for every file and dir in the FS.
  */
 #define BTRFS_INODE_ITEM_KEY		1
+/* reserve 2-11 close to the inode for later flexibility */
 #define BTRFS_INODE_REF_KEY		12
 #define BTRFS_INODE_EXTREF_KEY		13
 #define BTRFS_XATTR_ITEM_KEY		24
 #define BTRFS_ORPHAN_ITEM_KEY		48
-/* reserve 2-15 close to the inode for later flexibility */
 
 /*
- * dir items are the name -> inode pointers in a directory.  There is one
- * for every name in a directory.
+ * Dir items are the name -> inode pointers in a directory.
+ *
+ * There is one for every name in a directory.
  */
 #define BTRFS_DIR_LOG_ITEM_KEY  60
 #define BTRFS_DIR_LOG_INDEX_KEY 72
 #define BTRFS_DIR_ITEM_KEY	84
 #define BTRFS_DIR_INDEX_KEY	96
-/*
- * extent data is for file data
- */
+
+/* Stores info (position, size ...) about a data extent of a file */
 #define BTRFS_EXTENT_DATA_KEY	108
 
 /*
- * extent csums are stored in a separate tree and hold csums for
+ * Extent csums are stored in a separate tree and hold csums for
  * an entire extent on disk.
  */
 #define BTRFS_EXTENT_CSUM_KEY	128
 
 /*
- * root items point to tree roots.  They are typically in the root
- * tree used by the super block to find all the other trees
+ * Root items point to tree roots.
+ *
+ * They are typically in the root tree used by the super block to find all the
+ * other trees.
  */
 #define BTRFS_ROOT_ITEM_KEY	132
 
 /*
- * root backrefs tie subvols and snapshots to the directory entries that
- * reference them
+ * Root backrefs tie subvols and snapshots to the directory entries that
+ * reference them.
  */
 #define BTRFS_ROOT_BACKREF_KEY	144
 
 /*
- * root refs make a fast index for listing all of the snapshots and
+ * Root refs make a fast index for listing all of the snapshots and
  * subvolumes referenced by a given root.  They point directly to the
- * directory item in the root that references the subvol
+ * directory item in the root that references the subvol.
  */
 #define BTRFS_ROOT_REF_KEY	156
 
 /*
- * extent items are in the extent map tree.  These record which blocks
- * are used, and how many references there are to each block
+ * Extent items are in the extent tree.
+ *
+ * These record which blocks are used, and how many references there are.
  */
 #define BTRFS_EXTENT_ITEM_KEY	168
 
@@ -194,8 +207,9 @@
 #define BTRFS_SHARED_DATA_REF_KEY	184
 
 /*
- * block groups give us hints into the extent allocation trees.  Which
- * blocks are free etc etc
+ * Block groups give us hints into the extent allocation trees.
+ *
+ * Stores how many free space there is in a block group.
  */
 #define BTRFS_BLOCK_GROUP_ITEM_KEY 192
 
@@ -214,9 +228,10 @@
 
 /*
  * When a block group becomes very fragmented, we convert it to use bitmaps
- * instead of extents. A free space bitmap is keyed on
- * (start, FREE_SPACE_BITMAP, length); the corresponding item is a bitmap with
- * (length / sectorsize) bits.
+ * instead of extents.
+ *
+ * A free space bitmap is keyed on (start, FREE_SPACE_BITMAP, length).
+ * The corresponding item is a bitmap with (length / sectorsize) bits.
  */
 #define BTRFS_FREE_SPACE_BITMAP_KEY 200
 
@@ -226,20 +241,25 @@
 
 /*
  * Records the overall state of the qgroups.
+ *
  * There's only one instance of this key present,
  * (0, BTRFS_QGROUP_STATUS_KEY, 0)
  */
 #define BTRFS_QGROUP_STATUS_KEY         240
 /*
  * Records the currently used space of the qgroup.
+ *
  * One key per qgroup, (0, BTRFS_QGROUP_INFO_KEY, qgroupid).
  */
 #define BTRFS_QGROUP_INFO_KEY           242
+
 /*
  * Contains the user configured limits for the qgroup.
+ *
  * One key per qgroup, (0, BTRFS_QGROUP_LIMIT_KEY, qgroupid).
  */
 #define BTRFS_QGROUP_LIMIT_KEY          244
+
 /*
  * Records the child-parent relationship of qgroups. For
  * each relation, 2 keys are present:
@@ -248,9 +268,7 @@
  */
 #define BTRFS_QGROUP_RELATION_KEY       246
 
-/*
- * Obsolete name, see BTRFS_TEMPORARY_ITEM_KEY.
- */
+/* Obsolete name, see BTRFS_TEMPORARY_ITEM_KEY. */
 #define BTRFS_BALANCE_ITEM_KEY	248
 
 /*
@@ -266,9 +284,7 @@
  */
 #define BTRFS_TEMPORARY_ITEM_KEY	248
 
-/*
- * Obsolete name, see BTRFS_PERSISTENT_ITEM_KEY
- */
+/* Obsolete name, see BTRFS_PERSISTENT_ITEM_KEY */
 #define BTRFS_DEV_STATS_KEY		249
 
 /*
@@ -287,13 +303,15 @@
 #define BTRFS_PERSISTENT_ITEM_KEY	249
 
 /*
- * Persistantly stores the device replace state in the device tree.
+ * Persistently stores the device replace state in the device tree.
+ *
  * The key is built like this: (0, BTRFS_DEV_REPLACE_KEY, 0).
  */
 #define BTRFS_DEV_REPLACE_KEY	250
 
 /*
  * Stores items that allow to quickly map UUIDs to something else.
+ *
  * These items are part of the filesystem UUID tree.
  * The key is built like this:
  * (UUID_upper_64_bits, BTRFS_UUID_KEY*, UUID_lower_64_bits).
@@ -303,8 +321,9 @@
 						 * received subvols */
 
 /*
- * string items are for debugging.  They just store a short string of
- * data in the FS
+ * String items are for debugging.
+ *
+ * They just store a short string of data in the FS.
  */
 #define BTRFS_STRING_ITEM_KEY	253
 
@@ -313,7 +332,7 @@
 /* 32 bytes in various csum fields */
 #define BTRFS_CSUM_SIZE 32
 
-/* csum types */
+/* Csum types */
 enum btrfs_csum_type {
 	BTRFS_CSUM_TYPE_CRC32	= 0,
 	BTRFS_CSUM_TYPE_XXHASH	= 1,
@@ -322,7 +341,7 @@ enum btrfs_csum_type {
 };
 
 /*
- * flags definitions for directory entry item type
+ * Flags definitions for directory entry item type.
  *
  * Used by:
  * struct btrfs_dir_item.type
@@ -347,14 +366,13 @@ enum btrfs_csum_type {
  * The key defines the order in the tree, and so it also defines (optimal)
  * block layout.
  *
- * objectid corresponds to the inode number.
+ * Objectid and offset are interpreted based on type.
+ * While normally for objectid, it either represents a root number, or an
+ * inode number.
  *
- * type tells us things about the object, and is a kind of stream selector.
- * so for a given inode, keys with type of 1 might refer to the inode data,
- * type of 2 may point to file data in the btree and type == 3 may point to
- * extents.
- *
- * offset is the starting byte offset for this key in the stream.
+ * Type tells us things about the object, and is a kind of stream selector.
+ * Check the following URL for full references about btrfs_disk_key/btrfs_key:
+ * https://btrfs.wiki.kernel.org/index.php/Btree_Items
  *
  * btrfs_disk_key is in disk byte order.  struct btrfs_key is always
  * in cpu native order.  Otherwise they are identical and their sizes
@@ -373,49 +391,49 @@ struct btrfs_key {
 } __attribute__ ((__packed__));
 
 struct btrfs_dev_item {
-	/* the internal btrfs device id */
+	/* The internal btrfs device id */
 	__le64 devid;
 
-	/* size of the device */
+	/* Size of the device */
 	__le64 total_bytes;
 
-	/* bytes used */
+	/* Bytes used */
 	__le64 bytes_used;
 
-	/* optimal io alignment for this device */
+	/* Optimal io alignment for this device */
 	__le32 io_align;
 
-	/* optimal io width for this device */
+	/* Optimal io width for this device */
 	__le32 io_width;
 
-	/* minimal io size for this device */
+	/* Minimal io size for this device */
 	__le32 sector_size;
 
-	/* type and info about this device */
+	/* Type and info about this device */
 	__le64 type;
 
-	/* expected generation for this device */
+	/* Expected generation for this device */
 	__le64 generation;
 
 	/*
-	 * starting byte of this partition on the device,
-	 * to allow for stripe alignment in the future
+	 * Starting byte of this partition on the device,
+	 * to allow for stripe alignment in the future.
 	 */
 	__le64 start_offset;
 
-	/* grouping information for allocation decisions */
+	/* Grouping information for allocation decisions */
 	__le32 dev_group;
 
-	/* seek speed 0-100 where 100 is fastest */
+	/* Optimal seek speed 0-100 where 100 is fastest */
 	__u8 seek_speed;
 
-	/* bandwidth 0-100 where 100 is fastest */
+	/* Optimal bandwidth 0-100 where 100 is fastest */
 	__u8 bandwidth;
 
-	/* btrfs generated uuid for this device */
+	/* Btrfs generated uuid for this device */
 	__u8 uuid[BTRFS_UUID_SIZE];
 
-	/* uuid of FS who owns this device */
+	/* UUID of FS who owns this device */
 	__u8 fsid[BTRFS_UUID_SIZE];
 } __attribute__ ((__packed__));
 
@@ -426,30 +444,31 @@ struct btrfs_stripe {
 } __attribute__ ((__packed__));
 
 struct btrfs_chunk {
-	/* size of this chunk in bytes */
+	/* Size of this chunk in bytes */
 	__le64 length;
 
-	/* objectid of the root referencing this chunk */
+	/* Objectid of the root referencing this chunk */
 	__le64 owner;
 
 	__le64 stripe_len;
 	__le64 type;
 
-	/* optimal io alignment for this chunk */
+	/* Optimal io alignment for this chunk */
 	__le32 io_align;
 
-	/* optimal io width for this chunk */
+	/* Optimal io width for this chunk */
 	__le32 io_width;
 
-	/* minimal io size for this chunk */
+	/* Minimal io size for this chunk */
 	__le32 sector_size;
 
-	/* 2^16 stripes is quite a lot, a second limit is the size of a single
-	 * item in the btree
+	/*
+	 * 2^16 stripes is quite a lot, a second limit is the size of a single
+	 * item in the btree.
 	 */
 	__le16 num_stripes;
 
-	/* sub stripes only matter for raid10 */
+	/* Sub stripes only matter for raid10 */
 	__le16 sub_stripes;
 	struct btrfs_stripe stripe;
 	/* additional stripes go here */
@@ -486,10 +505,9 @@ struct btrfs_free_space_header {
 
 
 /*
- * items in the extent btree are used to record the objectid of the
- * owner of the block and the number of references
+ * Items in the extent tree are used to record the objectid of the
+ * owner of the block and the number of references.
  */
-
 struct btrfs_extent_item {
 	__le64 refs;
 	__le64 generation;
@@ -504,14 +522,12 @@ struct btrfs_extent_item_v0 {
 #define BTRFS_EXTENT_FLAG_DATA		(1ULL << 0)
 #define BTRFS_EXTENT_FLAG_TREE_BLOCK	(1ULL << 1)
 
-/* following flags only apply to tree blocks */
-
-/* use full backrefs for extent pointers in the block */
+/* Use full backrefs for extent pointers in the block */
 #define BTRFS_BLOCK_FLAG_FULL_BACKREF	(1ULL << 8)
 
 /*
- * this flag is only used internally by scrub and may be changed at any time
- * it is only declared here to avoid collisions
+ * This flag is only used internally by scrub and may be changed at any time
+ * it is only declared here to avoid collisions.
  */
 #define BTRFS_EXTENT_FLAG_SUPER		(1ULL << 48)
 
@@ -536,7 +552,7 @@ struct btrfs_extent_inline_ref {
 	__le64 offset;
 } __attribute__ ((__packed__));
 
-/* old style backrefs item */
+/* Old style backrefs item */
 struct btrfs_extent_ref_v0 {
 	__le64 root;
 	__le64 generation;
@@ -545,9 +561,11 @@ struct btrfs_extent_ref_v0 {
 } __attribute__ ((__packed__));
 
 
-/* dev extents record free space on individual devices.  The owner
- * field points back to the chunk allocation mapping tree that allocated
- * the extent.  The chunk tree uuid field is a way to double check the owner
+/* Dev extents record used space on individual devices.
+ *
+ * The owner field points back to the chunk allocation mapping tree that
+ * allocated the extent.
+ * The chunk tree uuid field is a way to double check the owner.
  */
 struct btrfs_dev_extent {
 	__le64 chunk_tree;
@@ -560,7 +578,7 @@ struct btrfs_dev_extent {
 struct btrfs_inode_ref {
 	__le64 index;
 	__le16 name_len;
-	/* name goes here */
+	/* Name goes here */
 } __attribute__ ((__packed__));
 
 struct btrfs_inode_extref {
@@ -568,7 +586,7 @@ struct btrfs_inode_extref {
 	__le64 index;
 	__le16 name_len;
 	__u8   name[0];
-	/* name goes here */
+	/* Name goes here */
 } __attribute__ ((__packed__));
 
 struct btrfs_timespec {
@@ -576,9 +594,7 @@ struct btrfs_timespec {
 	__le32 nsec;
 } __attribute__ ((__packed__));
 
-/*
- * Inode flags
- */
+/* Inode flags */
 #define BTRFS_INODE_NODATASUM		(1 << 0)
 #define BTRFS_INODE_NODATACOW		(1 << 1)
 #define BTRFS_INODE_READONLY		(1 << 2)
@@ -610,9 +626,9 @@ struct btrfs_timespec {
 	 BTRFS_INODE_ROOT_ITEM_INIT)
 
 struct btrfs_inode_item {
-	/* nfs style generation number */
+	/* Nfs style generation number */
 	__le64 generation;
-	/* transid that last touched this inode */
+	/* Transid that last touched this inode */
 	__le64 transid;
 	__le64 size;
 	__le64 nbytes;
@@ -624,12 +640,12 @@ struct btrfs_inode_item {
 	__le64 rdev;
 	__le64 flags;
 
-	/* modification sequence number for NFS */
+	/* Modification sequence number for NFS */
 	__le64 sequence;
 
 	/*
-	 * a little future expansion, for more than this we can
-	 * just grow the inode item and version it
+	 * A little future expansion, for more than this we can just grow the
+	 * inode item and version it
 	 */
 	__le64 reserved[4];
 	struct btrfs_timespec atime;
@@ -685,27 +701,25 @@ struct btrfs_root_item {
 	 * mismatching generation values here and thus must invalidate the
 	 * new fields. See btrfs_update_root and btrfs_find_last_root for
 	 * details.
-	 * the offset of generation_v2 is also used as the start for the memset
+	 * The offset of generation_v2 is also used as the start for the memset
 	 * when invalidating the fields.
 	 */
 	__le64 generation_v2;
 	__u8 uuid[BTRFS_UUID_SIZE];
 	__u8 parent_uuid[BTRFS_UUID_SIZE];
 	__u8 received_uuid[BTRFS_UUID_SIZE];
-	__le64 ctransid; /* updated when an inode changes */
-	__le64 otransid; /* trans when created */
-	__le64 stransid; /* trans when sent. non-zero for received subvol */
-	__le64 rtransid; /* trans when received. non-zero for received subvol */
+	__le64 ctransid; /* Updated when an inode changes */
+	__le64 otransid; /* Trans when created */
+	__le64 stransid; /* Trans when sent. Non-zero for received subvol. */
+	__le64 rtransid; /* Trans when received. Non-zero for received subvol.*/
 	struct btrfs_timespec ctime;
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-	__le64 reserved[8]; /* for future */
+	__le64 reserved[8]; /* For future */
 } __attribute__ ((__packed__));
 
-/*
- * this is used for both forward and backward root refs
- */
+/* This is used for both forward and backward root refs */
 struct btrfs_root_ref {
 	__le64 dirid;
 	__le64 sequence;
@@ -714,13 +728,14 @@ struct btrfs_root_ref {
 
 struct btrfs_disk_balance_args {
 	/*
-	 * profiles to operate on, single is denoted by
-	 * BTRFS_AVAIL_ALLOC_BIT_SINGLE
+	 * Profiles to operate on.
+	 *
+	 * SINGLE is denoted by BTRFS_AVAIL_ALLOC_BIT_SINGLE.
 	 */
 	__le64 profiles;
 
 	/*
-	 * usage filter
+	 * Usage filter
 	 * BTRFS_BALANCE_ARGS_USAGE with a single value means '0..N'
 	 * BTRFS_BALANCE_ARGS_USAGE_RANGE - range syntax, min..max
 	 */
@@ -732,20 +747,21 @@ struct btrfs_disk_balance_args {
 		};
 	};
 
-	/* devid filter */
+	/* Devid filter */
 	__le64 devid;
 
-	/* devid subset filter [pstart..pend) */
+	/* Devid subset filter [pstart..pend) */
 	__le64 pstart;
 	__le64 pend;
 
-	/* btrfs virtual address space subset filter [vstart..vend) */
+	/* Btrfs virtual address space subset filter [vstart..vend) */
 	__le64 vstart;
 	__le64 vend;
 
 	/*
-	 * profile to convert to, single is denoted by
-	 * BTRFS_AVAIL_ALLOC_BIT_SINGLE
+	 * Profile to convert to.
+	 *
+	 * SINGLE is denoted by BTRFS_AVAIL_ALLOC_BIT_SINGLE.
 	 */
 	__le64 target;
 
@@ -753,9 +769,9 @@ struct btrfs_disk_balance_args {
 	__le64 flags;
 
 	/*
-	 * BTRFS_BALANCE_ARGS_LIMIT with value 'limit'
+	 * BTRFS_BALANCE_ARGS_LIMIT with value 'limit'.
 	 * BTRFS_BALANCE_ARGS_LIMIT_RANGE - the extend version can use minimum
-	 * and maximum
+	 * and maximum.
 	 */
 	union {
 		__le64 limit;
@@ -767,7 +783,7 @@ struct btrfs_disk_balance_args {
 
 	/*
 	 * Process chunks that cross stripes_min..stripes_max devices,
-	 * BTRFS_BALANCE_ARGS_STRIPES_RANGE
+	 * BTRFS_BALANCE_ARGS_STRIPES_RANGE.
 	 */
 	__le32 stripes_min;
 	__le32 stripes_max;
@@ -776,8 +792,8 @@ struct btrfs_disk_balance_args {
 } __attribute__ ((__packed__));
 
 /*
- * store balance parameters to disk so that balance can be properly
- * resumed after crash or unmount
+ * Stores balance parameters to disk so that balance can be properly
+ * resumed after crash or unmount.
  */
 struct btrfs_balance_item {
 	/* BTRFS_BALANCE_* */
@@ -806,16 +822,14 @@ enum btrfs_compression_type {
 };
 
 struct btrfs_file_extent_item {
-	/*
-	 * transaction id that created this extent
-	 */
+	/* Transaction id that created this extent */
 	__le64 generation;
 	/*
-	 * max number of bytes to hold this extent in ram
-	 * when we split a compressed extent we can't know how big
-	 * each of the resulting pieces will be.  So, this is
-	 * an upper limit on the size of the extent in ram instead of
-	 * an exact limit.
+	 * Max number of bytes to hold this extent in ram.
+	 *
+	 * When we split a compressed extent we can't know how big each of the
+	 * resulting pieces will be.  So, this is an upper limit on the size of
+	 * the extent in ram instead of an exact limit.
 	 */
 	__le64 ram_bytes;
 
@@ -828,30 +842,34 @@ struct btrfs_file_extent_item {
 	 */
 	__u8 compression;
 	__u8 encryption;
-	__le16 other_encoding; /* spare for later use */
+	__le16 other_encoding; /* Spare for later use */
 
-	/* are we inline data or a real extent? */
+	/* Are we inline data or a real extent? */
 	__u8 type;
 
 	/*
-	 * disk space consumed by the extent, checksum blocks are included
+	 * Disk space consumed by the extent, checksum blocks are not included
 	 * in these numbers
 	 *
 	 * At this offset in the structure, the inline extent data start.
 	 */
 	__le64 disk_bytenr;
 	__le64 disk_num_bytes;
+
 	/*
-	 * the logical offset in file blocks (no csums)
-	 * this extent record is for.  This allows a file extent to point
-	 * into the middle of an existing extent on disk, sharing it
-	 * between two snapshots (useful if some bytes in the middle of the
-	 * extent have changed
+	 * The logical offset inside the file extent.
+	 *
+	 * This allows a file extent to point into the middle of an existing
+	 * extent on disk, sharing it between two snapshots (useful if some
+	 * bytes in the middle of the extent have changed).
 	 */
 	__le64 offset;
+
 	/*
-	 * the logical number of file blocks (no csums included).  This
-	 * always reflects the size uncompressed and without encoding.
+	 * The logical number of bytes this file extent is referencing (no
+	 * csums included).
+	 *
+	 * This always reflects the size uncompressed and without encoding.
 	 */
 	__le64 num_bytes;
 
@@ -862,19 +880,19 @@ struct btrfs_csum_item {
 } __attribute__ ((__packed__));
 
 enum btrfs_dev_stat_values {
-	/* disk I/O failure stats */
+	/* Disk I/O failure stats */
 	BTRFS_DEV_STAT_WRITE_ERRS, /* EIO or EREMOTEIO from lower layers */
 	BTRFS_DEV_STAT_READ_ERRS, /* EIO or EREMOTEIO from lower layers */
 	BTRFS_DEV_STAT_FLUSH_ERRS, /* EIO or EREMOTEIO from lower layers */
 
-	/* stats for indirect indications for I/O failures */
-	BTRFS_DEV_STAT_CORRUPTION_ERRS, /* checksum error, bytenr error or
+	/* Stats for indirect indications for I/O failures */
+	BTRFS_DEV_STAT_CORRUPTION_ERRS, /* Checksum error, bytenr error or
 					 * contents is illegal: this is an
 					 * indication that the block was damaged
 					 * during read or write, or written to
 					 * wrong location or read from wrong
 					 * location */
-	BTRFS_DEV_STAT_GENERATION_ERRS, /* an indication that blocks have not
+	BTRFS_DEV_STAT_GENERATION_ERRS, /* An indication that blocks have not
 					 * been written */
 
 	BTRFS_DEV_STAT_VALUES_MAX
@@ -882,8 +900,8 @@ enum btrfs_dev_stat_values {
 
 struct btrfs_dev_stats_item {
 	/*
-	 * grow this item struct at the end for future enhancements and keep
-	 * the existing values unchanged
+	 * Grow this item struct at the end for future enhancements and keep
+	 * the existing values unchanged.
 	 */
 	__le64 values[BTRFS_DEV_STAT_VALUES_MAX];
 } __attribute__ ((__packed__));
@@ -893,8 +911,8 @@ struct btrfs_dev_stats_item {
 
 struct btrfs_dev_replace_item {
 	/*
-	 * grow this item struct at the end for future enhancements and keep
-	 * the existing values unchanged
+	 * Grow this item struct at the end for future enhancements and keep
+	 * the existing values unchanged.
 	 */
 	__le64 src_devid;
 	__le64 cursor_left;
@@ -908,7 +926,7 @@ struct btrfs_dev_replace_item {
 	__le64 num_uncorrectable_read_errors;
 } __attribute__ ((__packed__));
 
-/* different types of block groups (and chunks) */
+/* Different types of block groups (and chunks) */
 #define BTRFS_BLOCK_GROUP_DATA		(1ULL << 0)
 #define BTRFS_BLOCK_GROUP_SYSTEM	(1ULL << 1)
 #define BTRFS_BLOCK_GROUP_METADATA	(1ULL << 2)
@@ -1004,20 +1022,16 @@ static inline __u64 btrfs_qgroup_level(__u64 qgroupid)
 	return qgroupid >> BTRFS_QGROUP_LEVEL_SHIFT;
 }
 
-/*
- * is subvolume quota turned on?
- */
+/* Is subvolume quota turned on? */
 #define BTRFS_QGROUP_STATUS_FLAG_ON		(1ULL << 0)
-/*
- * RESCAN is set during the initialization phase
- */
+
+/* Is qgroup rescan running? */
 #define BTRFS_QGROUP_STATUS_FLAG_RESCAN		(1ULL << 1)
+
 /*
- * Some qgroup entries are known to be out of date,
- * either because the configuration has changed in a way that
- * makes a rescan necessary, or because the fs has been mounted
- * with a non-qgroup-aware version.
- * Turning qouta off and on again makes it inconsistent, too.
+ * Some qgroup entries are known to be out of date, either because the
+ * configuration has changed in a way that makes a rescan necessary, or
+ * because the fs has been mounted with a non-qgroup-aware version.
  */
 #define BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT	(1ULL << 2)
 
@@ -1026,19 +1040,19 @@ static inline __u64 btrfs_qgroup_level(__u64 qgroupid)
 struct btrfs_qgroup_status_item {
 	__le64 version;
 	/*
-	 * the generation is updated during every commit. As older
+	 * The generation is updated during every commit. As older
 	 * versions of btrfs are not aware of qgroups, it will be
 	 * possible to detect inconsistencies by checking the
-	 * generation on mount time
+	 * generation on mount time.
 	 */
 	__le64 generation;
 
-	/* flag definitions see above */
+	/* Flag definitions see above */
 	__le64 flags;
 
 	/*
-	 * only used during scanning to record the progress
-	 * of the scan. It contains a logical address
+	 * Only used during scanning to record the progress of the scan.
+	 * It contains a logical address.
 	 */
 	__le64 rescan;
 } __attribute__ ((__packed__));
@@ -1052,7 +1066,7 @@ struct btrfs_qgroup_info_item {
 } __attribute__ ((__packed__));
 
 /*
- * flags definition for qgroup limits
+ * Flags definition for qgroup limits
  *
  * Used by:
  * struct btrfs_qgroup_limit.flags
@@ -1066,9 +1080,7 @@ struct btrfs_qgroup_info_item {
 #define BTRFS_QGROUP_LIMIT_EXCL_CMPR	(1ULL << 5)
 
 struct btrfs_qgroup_limit_item {
-	/*
-	 * only updated when any of the other values change
-	 */
+	/* Only updated when any of the other values change. */
 	__le64 flags;
 	__le64 max_rfer;
 	__le64 max_excl;
@@ -1077,9 +1089,8 @@ struct btrfs_qgroup_limit_item {
 } __attribute__ ((__packed__));
 
 /*
- * 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.
+ * 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 {
@@ -1118,32 +1129,30 @@ struct btrfs_root_backup {
 } __attribute__ ((__packed__));
 
 /*
- * this is a very generous portion of the super block, giving us
- * room to translate 14 chunks with 3 stripes each.
+ * 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
 
 #define BTRFS_LABEL_SIZE 256
-/*
- * the super block basically lists the main trees of the FS
- * it currently lacks any block count etc etc
- */
+
+/* The super block basically lists the main trees of the FS. */
 struct btrfs_super_block {
-	/* the first 4 fields must match struct btrfs_header */
+	/* 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 */
+	/* 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 will help find the new super based on the log root */
+	/* This will help find the new super based on the log root. */
 	__le64 log_root_transid;
 	__le64 total_bytes;
 	__le64 bytes_used;
@@ -1169,17 +1178,17 @@ struct btrfs_super_block {
 	__le64 cache_generation;
 	__le64 uuid_tree_generation;
 
-	/* the UUID written into btree blocks */
+	/* The UUID written into btree blocks */
 	u8 metadata_uuid[BTRFS_FSID_SIZE];
 
-	/* future expansion */
+	/* Future expansion */
 	__le64 reserved[28];
 	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
 	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
 } __attribute__ ((__packed__));
 
 /*
- * feature flags
+ * Feature flags
  *
  * Used by:
  * struct btrfs_super_block::(compat|compat_ro|incompat)_flags
@@ -1205,7 +1214,7 @@ struct btrfs_super_block {
 #define BTRFS_FEATURE_INCOMPAT_COMPRESS_ZSTD	(1ULL << 4)
 
 /*
- * older kernels tried to do bigger metadata blocks, but the
+ * Older kernels tried to do bigger metadata blocks, but the
  * code was pretty buggy.  Lets not let them try anymore.
  */
 #define BTRFS_FEATURE_INCOMPAT_BIG_METADATA	(1ULL << 5)
@@ -1218,8 +1227,10 @@ struct btrfs_super_block {
 #define BTRFS_FEATURE_INCOMPAT_RAID1C34		(1ULL << 11)
 
 /*
- * Compat flags that we support.  If any incompat flags are set other than the
- * ones specified below then we will fail to mount
+ * Compat flags that we support.
+ *
+ * If any incompat flags are set other than the ones specified below then we
+ * will fail to mount.
  */
 #define BTRFS_FEATURE_COMPAT_SUPP		0ULL
 #define BTRFS_FEATURE_COMPAT_SAFE_SET		0ULL
@@ -1260,17 +1271,15 @@ struct btrfs_super_block {
 
 #define BTRFS_MAX_LEVEL 8
 
-/*
- * every tree block (leaf or node) starts with this header.
- */
+/* Every tree block (leaf or node) starts with this header. */
 struct btrfs_header {
-	/* these first four must match the super block */
+	/* 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 bytenr; /* Which block this node is supposed to live in */
 	__le64 flags;
 
-	/* allowed to be different from the super from here on down */
+	/* Allowed to be different from the super from here on down. */
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
 	__le64 generation;
 	__le64 owner;
@@ -1279,8 +1288,8 @@ struct btrfs_header {
 } __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)
+ * 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;
@@ -1301,8 +1310,8 @@ struct btrfs_leaf {
 } __attribute__ ((__packed__));
 
 /*
- * all non-leaf blocks are nodes, they hold only keys and pointers to
- * other blocks
+ * All non-leaf blocks are nodes, they hold only keys and pointers to children
+ * blocks.
  */
 struct btrfs_key_ptr {
 	struct btrfs_disk_key key;
@@ -1315,4 +1324,4 @@ struct btrfs_node {
 	struct btrfs_key_ptr ptrs[];
 } __attribute__ ((__packed__));
 
-#endif /* _BTRFS_CTREE_H_ */
+#endif /* __BTRFS_TREE_H__ */
-- 
2.26.0


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

* Re: [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments
  2020-04-15  8:41 ` [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments Qu Wenruo
@ 2020-04-16 10:20   ` David Sterba
  2020-04-16 12:54   ` David Sterba
  1 sibling, 0 replies; 13+ messages in thread
From: David Sterba @ 2020-04-16 10:20 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Wed, Apr 15, 2020 at 04:41:13PM +0800, Qu Wenruo wrote:
> Since we're here, modify btrfs_tree.h to follow the latest comment
> style.

Well, the point was to make the changes when moving the code so it's not
split into two patches, but this patch seems to update more code so it's
probably fine to have them separate.

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

* Re: [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments
  2020-04-15  8:41 ` [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments Qu Wenruo
  2020-04-16 10:20   ` David Sterba
@ 2020-04-16 12:54   ` David Sterba
  1 sibling, 0 replies; 13+ messages in thread
From: David Sterba @ 2020-04-16 12:54 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Wed, Apr 15, 2020 at 04:41:13PM +0800, Qu Wenruo wrote:
> Since we're here, modify btrfs_tree.h to follow the latest comment
> style.
> 
> This involves:
> - Use upper case char for the first word
> - Use one line comment if possible
> - Add the ending dot if it's a sentence
> - Add more comment explaining the usage of each tree
> - Add key type/objectid/offset reference URL
> - Remove dead comment
> - Update the header define line to reflect the filename
> - Add newline to seperate long comment

Going through the file I think we could do more changes so the format
documentation is complete. I'd also reformat the defines so the name and
value are separated by a few tabs. My changes on top of your patch will
be in misc-next but the plan is to squash them in the end. It'll
probably take a few more passes so the style is consistent.

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-15  8:41 ` [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h Qu Wenruo
@ 2020-04-21 11:31   ` Christoph Hellwig
  2020-04-21 11:41     ` Qu Wenruo
  2020-04-23 11:32     ` kbuild test robot
  1 sibling, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2020-04-21 11:31 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
> These structures all are on-disk format. Move them to btrfs_tree.h
> 
> This allows us to sync the header to different projects, who need to
> read btrfs filesystem, like U-boot, grub.

Please use a separate header for that.  btrfs_tree.h is a UAPI header
with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
can be copied into the projects where is needed.

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-21 11:31   ` Christoph Hellwig
@ 2020-04-21 11:41     ` Qu Wenruo
  2020-04-24 20:24       ` David Sterba
  0 siblings, 1 reply; 13+ messages in thread
From: Qu Wenruo @ 2020-04-21 11:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-btrfs



On 2020/4/21 下午7:31, Christoph Hellwig wrote:
> On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
>> These structures all are on-disk format. Move them to btrfs_tree.h
>>
>> This allows us to sync the header to different projects, who need to
>> read btrfs filesystem, like U-boot, grub.
> 
> Please use a separate header for that.  btrfs_tree.h is a UAPI header
> with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
> can be copied into the projects where is needed.
> 
Doesn't on-disk format itself need strict ABI guarantees?

Thus it looks like the perfect location for on-disk format definitions.

Thanks,
Qu

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-15  8:41 ` [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h Qu Wenruo
@ 2020-04-23 11:32     ` kbuild test robot
  2020-04-23 11:32     ` kbuild test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2020-04-23 11:32 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]

Hi Qu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.7-rc1]
[also build test ERROR on next-20200423]
[cannot apply to btrfs/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-Reformat-and-make-btrfs_tree-h-self-contained/20200416-023848
base:    8f3d9f354286745c751374f5f1fcafee6b3f3136
config: x86_64-randconfig-s1-20200423 (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from ./usr/include/linux/btrfs.h:24:0,
                    from <command-line>:32:
>> ./usr/include/linux/btrfs_tree.h:1110:2: error: unknown type name 'u8'
     u8 tree_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1111:2: error: unknown type name 'u8'
     u8 chunk_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1112:2: error: unknown type name 'u8'
     u8 extent_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1113:2: error: unknown type name 'u8'
     u8 fs_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1114:2: error: unknown type name 'u8'
     u8 dev_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1115:2: error: unknown type name 'u8'
     u8 csum_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1117:2: error: unknown type name 'u8'
     u8 unused_8[10];
     ^~
   ./usr/include/linux/btrfs_tree.h:1133:2: error: unknown type name 'u8'
     u8 csum[BTRFS_CSUM_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1135:2: error: unknown type name 'u8'
     u8 fsid[BTRFS_FSID_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1162:2: error: unknown type name 'u8'
     u8 root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1163:2: error: unknown type name 'u8'
     u8 chunk_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1164:2: error: unknown type name 'u8'
     u8 log_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1173:2: error: unknown type name 'u8'
     u8 metadata_uuid[BTRFS_FSID_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1177:2: error: unknown type name 'u8'
     u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1268:2: error: unknown type name 'u8'
     u8 csum[BTRFS_CSUM_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1269:2: error: unknown type name 'u8'
     u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
     ^~
   ./usr/include/linux/btrfs_tree.h:1274:2: error: unknown type name 'u8'
     u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1278:2: error: unknown type name 'u8'
     u8 level;
     ^~

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33108 bytes --]

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
@ 2020-04-23 11:32     ` kbuild test robot
  0 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2020-04-23 11:32 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3252 bytes --]

Hi Qu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.7-rc1]
[also build test ERROR on next-20200423]
[cannot apply to btrfs/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-Reformat-and-make-btrfs_tree-h-self-contained/20200416-023848
base:    8f3d9f354286745c751374f5f1fcafee6b3f3136
config: x86_64-randconfig-s1-20200423 (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from ./usr/include/linux/btrfs.h:24:0,
                    from <command-line>:32:
>> ./usr/include/linux/btrfs_tree.h:1110:2: error: unknown type name 'u8'
     u8 tree_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1111:2: error: unknown type name 'u8'
     u8 chunk_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1112:2: error: unknown type name 'u8'
     u8 extent_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1113:2: error: unknown type name 'u8'
     u8 fs_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1114:2: error: unknown type name 'u8'
     u8 dev_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1115:2: error: unknown type name 'u8'
     u8 csum_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1117:2: error: unknown type name 'u8'
     u8 unused_8[10];
     ^~
   ./usr/include/linux/btrfs_tree.h:1133:2: error: unknown type name 'u8'
     u8 csum[BTRFS_CSUM_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1135:2: error: unknown type name 'u8'
     u8 fsid[BTRFS_FSID_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1162:2: error: unknown type name 'u8'
     u8 root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1163:2: error: unknown type name 'u8'
     u8 chunk_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1164:2: error: unknown type name 'u8'
     u8 log_root_level;
     ^~
   ./usr/include/linux/btrfs_tree.h:1173:2: error: unknown type name 'u8'
     u8 metadata_uuid[BTRFS_FSID_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1177:2: error: unknown type name 'u8'
     u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1268:2: error: unknown type name 'u8'
     u8 csum[BTRFS_CSUM_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1269:2: error: unknown type name 'u8'
     u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
     ^~
   ./usr/include/linux/btrfs_tree.h:1274:2: error: unknown type name 'u8'
     u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
     ^~
   ./usr/include/linux/btrfs_tree.h:1278:2: error: unknown type name 'u8'
     u8 level;
     ^~

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33108 bytes --]

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-21 11:41     ` Qu Wenruo
@ 2020-04-24 20:24       ` David Sterba
  2020-04-25  7:14         ` Christoph Hellwig
  0 siblings, 1 reply; 13+ messages in thread
From: David Sterba @ 2020-04-24 20:24 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Christoph Hellwig, linux-btrfs

On Tue, Apr 21, 2020 at 07:41:40PM +0800, Qu Wenruo wrote:
> 
> 
> On 2020/4/21 下午7:31, Christoph Hellwig wrote:
> > On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
> >> These structures all are on-disk format. Move them to btrfs_tree.h
> >>
> >> This allows us to sync the header to different projects, who need to
> >> read btrfs filesystem, like U-boot, grub.
> > 
> > Please use a separate header for that.  btrfs_tree.h is a UAPI header
> > with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
> > can be copied into the projects where is needed.
> > 
> Doesn't on-disk format itself need strict ABI guarantees?
> 
> Thus it looks like the perfect location for on-disk format definitions.

Right now I'm not sure if it's a good idea to put the on-disk format to
the UAPI path or not. The exported code is to support the ioctls,
there's an overlap with the on-disk format but providing all the on-disk
structures for general might become an unnecessry burden.

We know that there's a small number of projects that want to sync the
on-disk format so I don't think it's going to be a problem for them to
use a private header.

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-24 20:24       ` David Sterba
@ 2020-04-25  7:14         ` Christoph Hellwig
  2020-04-25  7:25           ` Qu Wenruo
  0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2020-04-25  7:14 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, Christoph Hellwig, linux-btrfs

On Fri, Apr 24, 2020 at 10:24:45PM +0200, David Sterba wrote:
> On Tue, Apr 21, 2020 at 07:41:40PM +0800, Qu Wenruo wrote:
> > 
> > 
> > On 2020/4/21 下午7:31, Christoph Hellwig wrote:
> > > On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
> > >> These structures all are on-disk format. Move them to btrfs_tree.h
> > >>
> > >> This allows us to sync the header to different projects, who need to
> > >> read btrfs filesystem, like U-boot, grub.
> > > 
> > > Please use a separate header for that.  btrfs_tree.h is a UAPI header
> > > with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
> > > can be copied into the projects where is needed.
> > > 
> > Doesn't on-disk format itself need strict ABI guarantees?
> > 
> > Thus it looks like the perfect location for on-disk format definitions.
> 
> Right now I'm not sure if it's a good idea to put the on-disk format to
> the UAPI path or not. The exported code is to support the ioctls,
> there's an overlap with the on-disk format but providing all the on-disk
> structures for general might become an unnecessry burden.
> 
> We know that there's a small number of projects that want to sync the
> on-disk format so I don't think it's going to be a problem for them to
> use a private header.

And the usual way is to just ensure the format header is self-contained
and suitably licensed that they can easily copy it and rely on the
version they checked in.  That avoids the problems of

 a) the tools relying on installed kernel headers new enough for the
    feature they want to support
 b) ifdef magic for newer features in the tools
 c) the need to keep changes to the kernel format header to be backwards
    compatible at the compiler level (as there can be disk format
    compatible changes that still break users, e.g. introducing a named
    union, or splitting / merging struct definitions)

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-25  7:14         ` Christoph Hellwig
@ 2020-04-25  7:25           ` Qu Wenruo
  2020-04-30  7:29             ` Qu Wenruo
  0 siblings, 1 reply; 13+ messages in thread
From: Qu Wenruo @ 2020-04-25  7:25 UTC (permalink / raw)
  To: Christoph Hellwig, dsterba, linux-btrfs



On 2020/4/25 下午3:14, Christoph Hellwig wrote:
> On Fri, Apr 24, 2020 at 10:24:45PM +0200, David Sterba wrote:
>> On Tue, Apr 21, 2020 at 07:41:40PM +0800, Qu Wenruo wrote:
>>>
>>>
>>> On 2020/4/21 下午7:31, Christoph Hellwig wrote:
>>>> On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
>>>>> These structures all are on-disk format. Move them to btrfs_tree.h
>>>>>
>>>>> This allows us to sync the header to different projects, who need to
>>>>> read btrfs filesystem, like U-boot, grub.
>>>>
>>>> Please use a separate header for that.  btrfs_tree.h is a UAPI header
>>>> with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
>>>> can be copied into the projects where is needed.
>>>>
>>> Doesn't on-disk format itself need strict ABI guarantees?
>>>
>>> Thus it looks like the perfect location for on-disk format definitions.
>>
>> Right now I'm not sure if it's a good idea to put the on-disk format to
>> the UAPI path or not. The exported code is to support the ioctls,
>> there's an overlap with the on-disk format but providing all the on-disk
>> structures for general might become an unnecessry burden.
>>
>> We know that there's a small number of projects that want to sync the
>> on-disk format so I don't think it's going to be a problem for them to
>> use a private header.
> 
> And the usual way is to just ensure the format header is self-contained
> and suitably licensed that they can easily copy it and rely on the
> version they checked in.  That avoids the problems of
> 
>  a) the tools relying on installed kernel headers new enough for the
>     feature they want to support
>  b) ifdef magic for newer features in the tools
>  c) the need to keep changes to the kernel format header to be backwards
>     compatible at the compiler level (as there can be disk format
>     compatible changes that still break users, e.g. introducing a named
>     union, or splitting / merging struct definitions)
> 
One last question.

Btrfs has one ioctl, which allow users to search btrfs on-disk data.

And the ioctl returns the on-disk data directly to user space, which
means the on-disk format is also used by ioctl.

In that case, do we still need to put the on-disk format internal other
than as a uapi?

Thanks,
Qu

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

* Re: [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h
  2020-04-25  7:25           ` Qu Wenruo
@ 2020-04-30  7:29             ` Qu Wenruo
  0 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-04-30  7:29 UTC (permalink / raw)
  To: Christoph Hellwig, dsterba, linux-btrfs



On 2020/4/25 下午3:25, Qu Wenruo wrote:
> 
> 
> On 2020/4/25 下午3:14, Christoph Hellwig wrote:
>> On Fri, Apr 24, 2020 at 10:24:45PM +0200, David Sterba wrote:
>>> On Tue, Apr 21, 2020 at 07:41:40PM +0800, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2020/4/21 下午7:31, Christoph Hellwig wrote:
>>>>> On Wed, Apr 15, 2020 at 04:41:12PM +0800, Qu Wenruo wrote:
>>>>>> These structures all are on-disk format. Move them to btrfs_tree.h
>>>>>>
>>>>>> This allows us to sync the header to different projects, who need to
>>>>>> read btrfs filesystem, like U-boot, grub.
>>>>>
>>>>> Please use a separate header for that.  btrfs_tree.h is a UAPI header
>>>>> with strict ABI guarantees.  Just add a fs/btrfs/btrfs_format.h that
>>>>> can be copied into the projects where is needed.
>>>>>
>>>> Doesn't on-disk format itself need strict ABI guarantees?
>>>>
>>>> Thus it looks like the perfect location for on-disk format definitions.
>>>
>>> Right now I'm not sure if it's a good idea to put the on-disk format to
>>> the UAPI path or not. The exported code is to support the ioctls,
>>> there's an overlap with the on-disk format but providing all the on-disk
>>> structures for general might become an unnecessry burden.
>>>
>>> We know that there's a small number of projects that want to sync the
>>> on-disk format so I don't think it's going to be a problem for them to
>>> use a private header.
>>
>> And the usual way is to just ensure the format header is self-contained
>> and suitably licensed that they can easily copy it and rely on the
>> version they checked in.  That avoids the problems of
>>
>>  a) the tools relying on installed kernel headers new enough for the
>>     feature they want to support
>>  b) ifdef magic for newer features in the tools
>>  c) the need to keep changes to the kernel format header to be backwards
>>     compatible at the compiler level (as there can be disk format
>>     compatible changes that still break users, e.g. introducing a named
>>     union, or splitting / merging struct definitions)
>>
> One last question.
> 
> Btrfs has one ioctl, which allow users to search btrfs on-disk data.
> 
> And the ioctl returns the on-disk data directly to user space, which
> means the on-disk format is also used by ioctl.
> 
> In that case, do we still need to put the on-disk format internal other
> than as a uapi?

After some tries, there are still problems especially for some flags
shared by ioctl and on-disk data.

E.g. We have a qgroup related ioctl, which uses some flag like
BTRFS_QGROUP_LIMIT_MAX_RFER.

But we also use that flag on-disk (obviously, we don't want spend try
time/code to do mapping of these flags).

Putting such flags to ioctl header, then we need to sync two headers to
other projects, while we care nothing about the ioctl part.

Putting such flags to on-disk format header, and put that header inside
fs/btrfs other than uapi, then ioctl doesn't have the definition of such
flags, and break user space programms.

So it looks like we'd better keep a on-disk format uapi header, and put
such flags to on-disk format uapi header, and ioctl header just includes
on-disk format header.

For user space, all needed flags are still kept as is.
For bootloader projects, they only need the on-disk format header.

Any extra ideas/feedback on this?

Thanks,
Qu

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

end of thread, other threads:[~2020-04-30  7:34 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-15  8:41 [PATCH v4 0/2] btrfs: Reformat and make btrfs_tree.h self-contained Qu Wenruo
2020-04-15  8:41 ` [PATCH v4 1/2] btrfs: Move on-disk structure definition to btrfs_tree.h Qu Wenruo
2020-04-21 11:31   ` Christoph Hellwig
2020-04-21 11:41     ` Qu Wenruo
2020-04-24 20:24       ` David Sterba
2020-04-25  7:14         ` Christoph Hellwig
2020-04-25  7:25           ` Qu Wenruo
2020-04-30  7:29             ` Qu Wenruo
2020-04-23 11:32   ` kbuild test robot
2020-04-23 11:32     ` kbuild test robot
2020-04-15  8:41 ` [PATCH v4 2/2] btrfs: Reformat btrfs_tree.h comments Qu Wenruo
2020-04-16 10:20   ` David Sterba
2020-04-16 12:54   ` 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.