diff mbox series

[v2,2/4] btrfs: add extra comments on extent_map members

Message ID 98da90ba55445f69030a7664ae5029d710e4efbd.1712187452.git.wqu@suse.com (mailing list archive)
State New
Headers show
Series btrfs: more explaination on extent_map members | expand

Commit Message

Qu Wenruo April 3, 2024, 11:41 p.m. UTC
The extent_map structure is very critical to btrfs, as it is involved
for both read and write paths.

Unfortunately the structure is not properly explained, making it pretty
hard to understand nor to do further improvement.

This patch adds extra comments explaining the major members based on
my code reading.
Hopefully we can find more members to cleanup in the future.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_map.h | 54 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

Comments

Filipe Manana April 4, 2024, 10:33 a.m. UTC | #1
On Thu, Apr 4, 2024 at 12:47 AM Qu Wenruo <wqu@suse.com> wrote:
>
> The extent_map structure is very critical to btrfs, as it is involved
> for both read and write paths.
>
> Unfortunately the structure is not properly explained, making it pretty
> hard to understand nor to do further improvement.
>
> This patch adds extra comments explaining the major members based on
> my code reading.
> Hopefully we can find more members to cleanup in the future.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent_map.h | 54 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index 10e9491865c9..82768288c6da 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -35,19 +35,71 @@ enum {
>  };
>
>  /*
> + * This structure represents file extents and holes.
> + *
>   * Keep this structure as compact as possible, as we can have really large
>   * amounts of allocated extent maps at any time.
>   */
>  struct extent_map {
>         struct rb_node rb_node;
>
> -       /* all of these are in bytes */
> +       /* All of these are in bytes. */
> +
> +       /* File offset matching the offset of a BTRFS_EXTENT_ITEM_KEY key. */
>         u64 start;
> +
> +       /*
> +        * Length of the file extent.
> +        *
> +        * For non-inlined file extents it's btrfs_file_extent_item::num_bytes.
> +        * For inline extents it's sectorsize, since inline data starts at
> +        * offsetof(struct , disk_bytenr) thus

Missing the structure's name (btrfs_file_extent_item).

> +        * btrfs_file_extent_item::num_bytes is not valid.
> +        */
>         u64 len;
> +
> +       /*
> +        * The file offset of the original file extent before splitting.
> +        *
> +        * This is an in-memory only member, matching
> +        * extent_map::start - btrfs_file_extent_item::offset for
> +        * regular/preallocated extents. EXTENT_MAP_HOLE otherwise.
> +        */
>         u64 orig_start;
> +
> +       /*
> +        * The full on-disk extent length, matching
> +        * btrfs_file_extent_item::disk_num_bytes.
> +        */
>         u64 orig_block_len;
> +
> +       /*
> +        * The decompressed size of the whole on-disk extent, matching
> +        * btrfs_file_extent_item::ram_bytes.
> +        *
> +        * For non-compressed extents, this matches orig_block_len.

It always matches btrfs_file_extent_item::ram_bytes, regardless of compression.

Thanks.

> +        */
>         u64 ram_bytes;
> +
> +       /*
> +        * The on-disk logical bytenr for the file extent.
> +        *
> +        * For compressed extents it matches btrfs_file_extent_item::disk_bytenr.
> +        * For uncompressed extents it matches
> +        * btrfs_file_extent_item::disk_bytenr + btrfs_file_extent_item::offset
> +        *
> +        * For holes it is EXTENT_MAP_HOLE and for inline extents it is
> +        * EXTENT_MAP_INLINE.
> +        */
>         u64 block_start;
> +
> +       /*
> +        * The on-disk length for the file extent.
> +        *
> +        * For compressed extents it matches btrfs_file_extent_item::disk_num_bytes.
> +        * For uncompressed extents it matches extent_map::len.
> +        * For holes and inline extents it's -1 and shouldn't be used.
> +        */
>         u64 block_len;
>
>         /*
> --
> 2.44.0
>
>
Qu Wenruo April 4, 2024, 9:11 p.m. UTC | #2
在 2024/4/4 21:03, Filipe Manana 写道:
> On Thu, Apr 4, 2024 at 12:47 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> The extent_map structure is very critical to btrfs, as it is involved
>> for both read and write paths.
>>
>> Unfortunately the structure is not properly explained, making it pretty
>> hard to understand nor to do further improvement.
>>
>> This patch adds extra comments explaining the major members based on
>> my code reading.
>> Hopefully we can find more members to cleanup in the future.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/extent_map.h | 54 ++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 53 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
>> index 10e9491865c9..82768288c6da 100644
>> --- a/fs/btrfs/extent_map.h
>> +++ b/fs/btrfs/extent_map.h
>> @@ -35,19 +35,71 @@ enum {
>>   };
>>
>>   /*
>> + * This structure represents file extents and holes.
>> + *
>>    * Keep this structure as compact as possible, as we can have really large
>>    * amounts of allocated extent maps at any time.
>>    */
>>   struct extent_map {
>>          struct rb_node rb_node;
>>
>> -       /* all of these are in bytes */
>> +       /* All of these are in bytes. */
>> +
>> +       /* File offset matching the offset of a BTRFS_EXTENT_ITEM_KEY key. */
>>          u64 start;
>> +
>> +       /*
>> +        * Length of the file extent.
>> +        *
>> +        * For non-inlined file extents it's btrfs_file_extent_item::num_bytes.
>> +        * For inline extents it's sectorsize, since inline data starts at
>> +        * offsetof(struct , disk_bytenr) thus
>
> Missing the structure's name (btrfs_file_extent_item).
>
>> +        * btrfs_file_extent_item::num_bytes is not valid.
>> +        */
>>          u64 len;
>> +
>> +       /*
>> +        * The file offset of the original file extent before splitting.
>> +        *
>> +        * This is an in-memory only member, matching
>> +        * extent_map::start - btrfs_file_extent_item::offset for
>> +        * regular/preallocated extents. EXTENT_MAP_HOLE otherwise.
>> +        */
>>          u64 orig_start;
>> +
>> +       /*
>> +        * The full on-disk extent length, matching
>> +        * btrfs_file_extent_item::disk_num_bytes.
>> +        */
>>          u64 orig_block_len;
>> +
>> +       /*
>> +        * The decompressed size of the whole on-disk extent, matching
>> +        * btrfs_file_extent_item::ram_bytes.
>> +        *
>> +        * For non-compressed extents, this matches orig_block_len.
>
> It always matches btrfs_file_extent_item::ram_bytes, regardless of compression.

It always matches btrfs_file_extent_item::ram_bytes, but for
non-compressed extents it also matches em::orig_block_len.

Or should I just remove it as it doesn't provide extra info?

Thanks,
Qu
>
> Thanks.
>
>> +        */
>>          u64 ram_bytes;
>> +
>> +       /*
>> +        * The on-disk logical bytenr for the file extent.
>> +        *
>> +        * For compressed extents it matches btrfs_file_extent_item::disk_bytenr.
>> +        * For uncompressed extents it matches
>> +        * btrfs_file_extent_item::disk_bytenr + btrfs_file_extent_item::offset
>> +        *
>> +        * For holes it is EXTENT_MAP_HOLE and for inline extents it is
>> +        * EXTENT_MAP_INLINE.
>> +        */
>>          u64 block_start;
>> +
>> +       /*
>> +        * The on-disk length for the file extent.
>> +        *
>> +        * For compressed extents it matches btrfs_file_extent_item::disk_num_bytes.
>> +        * For uncompressed extents it matches extent_map::len.
>> +        * For holes and inline extents it's -1 and shouldn't be used.
>> +        */
>>          u64 block_len;
>>
>>          /*
>> --
>> 2.44.0
>>
>>
>
diff mbox series

Patch

diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
index 10e9491865c9..82768288c6da 100644
--- a/fs/btrfs/extent_map.h
+++ b/fs/btrfs/extent_map.h
@@ -35,19 +35,71 @@  enum {
 };
 
 /*
+ * This structure represents file extents and holes.
+ *
  * Keep this structure as compact as possible, as we can have really large
  * amounts of allocated extent maps at any time.
  */
 struct extent_map {
 	struct rb_node rb_node;
 
-	/* all of these are in bytes */
+	/* All of these are in bytes. */
+
+	/* File offset matching the offset of a BTRFS_EXTENT_ITEM_KEY key. */
 	u64 start;
+
+	/*
+	 * Length of the file extent.
+	 *
+	 * For non-inlined file extents it's btrfs_file_extent_item::num_bytes.
+	 * For inline extents it's sectorsize, since inline data starts at
+	 * offsetof(struct , disk_bytenr) thus
+	 * btrfs_file_extent_item::num_bytes is not valid.
+	 */
 	u64 len;
+
+	/*
+	 * The file offset of the original file extent before splitting.
+	 *
+	 * This is an in-memory only member, matching
+	 * extent_map::start - btrfs_file_extent_item::offset for
+	 * regular/preallocated extents. EXTENT_MAP_HOLE otherwise.
+	 */
 	u64 orig_start;
+
+	/*
+	 * The full on-disk extent length, matching
+	 * btrfs_file_extent_item::disk_num_bytes.
+	 */
 	u64 orig_block_len;
+
+	/*
+	 * The decompressed size of the whole on-disk extent, matching
+	 * btrfs_file_extent_item::ram_bytes.
+	 *
+	 * For non-compressed extents, this matches orig_block_len.
+	 */
 	u64 ram_bytes;
+
+	/*
+	 * The on-disk logical bytenr for the file extent.
+	 *
+	 * For compressed extents it matches btrfs_file_extent_item::disk_bytenr.
+	 * For uncompressed extents it matches
+	 * btrfs_file_extent_item::disk_bytenr + btrfs_file_extent_item::offset
+	 *
+	 * For holes it is EXTENT_MAP_HOLE and for inline extents it is
+	 * EXTENT_MAP_INLINE.
+	 */
 	u64 block_start;
+
+	/*
+	 * The on-disk length for the file extent.
+	 *
+	 * For compressed extents it matches btrfs_file_extent_item::disk_num_bytes.
+	 * For uncompressed extents it matches extent_map::len.
+	 * For holes and inline extents it's -1 and shouldn't be used.
+	 */
 	u64 block_len;
 
 	/*