[RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[Sidong Yang]

This patch adds an subcommand in inspect-internal. It dumps file extents of
the file that user provided. It helps to show the internal information
about file extents comprise the file.

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
Hi, I'm writing an patch that adds an subcommand that dumps file extents of
the file. I don't have much idea about btrfs but I referenced btrfs
wiki, dev-docs, and also Zygo's bee. It's just like draft.

I have some questions below.

The command works like below,
# ./btrfs inspect-internal dump-file-extents /mnt/bb/TAGS
begin = 0x0, end = 0x20000, physical = 0x4fdf5000, physical_len = 0xc000
begin = 0x20000, end = 0x40000, physical = 0x4fe01000, physical_len = 0xc000
begin = 0x40000, end = 0x47000, physical = 0x4fe0d000, physical_len = 0x3000

What format would be better?
Is it better to just use the variable name as it is? like disk_bytenr
not like physical_len?

And what option do we need? like showing compression or file extent type?

Any comments will be welcome. Thanks!
---
 Makefile                         |   2 +-
 cmds/commands.h                  |   2 +-
 cmds/inspect-dump-file-extents.c | 130 +++++++++++++++++++++++++++++++
 cmds/inspect.c                   |   1 +
 4 files changed, 133 insertions(+), 2 deletions(-)
 create mode 100644 cmds/inspect-dump-file-extents.c

diff --git a/Makefile b/Makefile
index a1cc457b..911e16de 100644
--- a/Makefile
+++ b/Makefile
@@ -156,7 +156,7 @@ cmds_objects = cmds/subvolume.o cmds/filesystem.o cmds/device.o cmds/scrub.o \
 	       cmds/restore.o cmds/rescue.o cmds/rescue-chunk-recover.o \
 	       cmds/rescue-super-recover.o \
 	       cmds/property.o cmds/filesystem-usage.o cmds/inspect-dump-tree.o \
-	       cmds/inspect-dump-super.o cmds/inspect-tree-stats.o cmds/filesystem-du.o \
+	       cmds/inspect-dump-super.o cmds/inspect-tree-stats.o cmds/inspect-dump-file-extents.o cmds/filesystem-du.o \
 	       mkfs/common.o check/mode-common.o check/mode-lowmem.o
 libbtrfs_objects = common/send-stream.o common/send-utils.o kernel-lib/rbtree.o btrfs-list.o \
 		   kernel-lib/radix-tree.o common/extent-cache.o kernel-shared/extent_io.o \
diff --git a/cmds/commands.h b/cmds/commands.h
index 8fa85d6c..55de248e 100644
--- a/cmds/commands.h
+++ b/cmds/commands.h
@@ -154,5 +154,5 @@ DECLARE_COMMAND(select_super);
 DECLARE_COMMAND(dump_super);
 DECLARE_COMMAND(debug_tree);
 DECLARE_COMMAND(rescue);
-
+DECLARE_COMMAND(inspect_dump_file_extents);
 #endif
diff --git a/cmds/inspect-dump-file-extents.c b/cmds/inspect-dump-file-extents.c
new file mode 100644
index 00000000..99aec7d7
--- /dev/null
+++ b/cmds/inspect-dump-file-extents.c
@@ -0,0 +1,130 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <fcntl.h>
+
+#include <sys/ioctl.h>
+
+#include "common/utils.h"
+#include "cmds/commands.h"
+
+static const char * const cmd_inspect_dump_file_extents_usage[] = {
+	"btrfs inspect-internal dump-extent path",
+	"Dump file extent in a textual form",
+	NULL
+};
+
+static int cmd_inspect_dump_file_extents(const struct cmd_struct *cmd,
+										 int argc, char **argv)
+{
+	int fd;
+	struct stat statbuf;
+	struct btrfs_ioctl_ino_lookup_args lookup;
+	struct btrfs_ioctl_search_args args;
+	struct btrfs_ioctl_search_key *sk = &args.key;
+	struct btrfs_file_extent_item *extent_item;
+	struct btrfs_ioctl_search_header *header;
+	u64 pos;
+	u64 buf_off;
+	u64 len;
+	u64 begin;
+	u64 physical;
+	u64 offset;
+	u64 physical_len;
+	int ret;
+	int i;
+
+	fd = open(argv[optind], O_RDONLY);
+	if (fd < 0) {
+		error("cannot open %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	if (fstat(fd, &statbuf) < 0) {
+		error("failed to fstat %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	lookup.treeid = 0;
+	lookup.objectid = BTRFS_FIRST_FREE_OBJECTID;
+
+	if (ioctl(fd, BTRFS_IOC_INO_LOOKUP, &lookup) < 0) {
+		error("failed to lookup inode %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	pos = 0;
+
+	sk->tree_id = lookup.treeid;
+	sk->min_objectid = statbuf.st_ino;
+	sk->max_objectid = statbuf.st_ino;
+
+	sk->max_offset = UINT64_MAX;
+	sk->min_transid = 0;
+	sk->max_transid = UINT64_MAX;
+	sk->min_type = sk->max_type = BTRFS_EXTENT_DATA_KEY;
+	sk->nr_items = 4096;
+
+	while (statbuf.st_size > pos) {
+		sk->min_offset = pos;
+		if (ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args)) {
+			error("failed to search tree ioctl %s: %m", argv[optind]);
+			ret = 1;
+			goto out;
+		}
+
+		buf_off = 0;
+		for(i=0; i<sk->nr_items; ++i) {
+			header = (struct btrfs_ioctl_search_header *)(args.buf + buf_off);
+
+			if (btrfs_search_header_type(header) == BTRFS_EXTENT_DATA_KEY) {
+				extent_item = (struct btrfs_file_extent_item *)(header + 1);
+				begin = btrfs_search_header_offset(header);
+
+				printf("begin = 0x%llx, ", begin);
+				switch (extent_item->type) {
+				case BTRFS_FILE_EXTENT_INLINE:
+					len = le64_to_cpu(extent_item->ram_bytes);
+					printf("end = 0x%llx\n", begin + len);
+					break;
+				case BTRFS_FILE_EXTENT_REG:
+				case BTRFS_FILE_EXTENT_PREALLOC:
+					len = le64_to_cpu(extent_item->num_bytes);
+					physical = le64_to_cpu(extent_item->disk_bytenr);
+					physical_len = le64_to_cpu(extent_item->disk_num_bytes);
+					offset = le64_to_cpu(extent_item->offset);
+					printf("end = 0x%llx, physical = 0x%llx, physical_len = 0x%llx\n",
+						   begin + len, physical, physical_len);
+					break;
+				}
+
+			}
+			buf_off += sizeof(*header) + btrfs_search_header_len(header);
+			pos += len;
+		}
+
+	}
+	ret = 0;
+out:
+	close(fd);
+	return ret;
+}
+DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
diff --git a/cmds/inspect.c b/cmds/inspect.c
index 2ef5f4b6..dfb0e27b 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
 		&cmd_struct_inspect_dump_tree,
 		&cmd_struct_inspect_dump_super,
 		&cmd_struct_inspect_tree_stats,
+		&cmd_struct_inspect_dump_file_extents,
 		NULL
 	}
 };
-- 
2.25.1

Re: [RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[Qu Wenruo]

On 2021/7/10 下午10:41, Sidong Yang wrote:
> This patch adds an subcommand in inspect-internal. It dumps file extents of
> the file that user provided. It helps to show the internal information
> about file extents comprise the file.

So this is going to be the combined command of filemap + btrfs-map-logical.

But how do you handle dirty pages which hasn't yet been flushed to disk?

Thanks,
Qu
>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> Hi, I'm writing an patch that adds an subcommand that dumps file extents of
> the file. I don't have much idea about btrfs but I referenced btrfs
> wiki, dev-docs, and also Zygo's bee. It's just like draft.
>
> I have some questions below.
>
> The command works like below,
> # ./btrfs inspect-internal dump-file-extents /mnt/bb/TAGS
> begin = 0x0, end = 0x20000, physical = 0x4fdf5000, physical_len = 0xc000
> begin = 0x20000, end = 0x40000, physical = 0x4fe01000, physical_len = 0xc000
> begin = 0x40000, end = 0x47000, physical = 0x4fe0d000, physical_len = 0x3000
>
> What format would be better?
> Is it better to just use the variable name as it is? like disk_bytenr
> not like physical_len?
>
> And what option do we need? like showing compression or file extent type?
>
> Any comments will be welcome. Thanks!
> ---
>   Makefile                         |   2 +-
>   cmds/commands.h                  |   2 +-
>   cmds/inspect-dump-file-extents.c | 130 +++++++++++++++++++++++++++++++
>   cmds/inspect.c                   |   1 +
>   4 files changed, 133 insertions(+), 2 deletions(-)
>   create mode 100644 cmds/inspect-dump-file-extents.c
>
> diff --git a/Makefile b/Makefile
> index a1cc457b..911e16de 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -156,7 +156,7 @@ cmds_objects = cmds/subvolume.o cmds/filesystem.o cmds/device.o cmds/scrub.o \
>   	       cmds/restore.o cmds/rescue.o cmds/rescue-chunk-recover.o \
>   	       cmds/rescue-super-recover.o \
>   	       cmds/property.o cmds/filesystem-usage.o cmds/inspect-dump-tree.o \
> -	       cmds/inspect-dump-super.o cmds/inspect-tree-stats.o cmds/filesystem-du.o \
> +	       cmds/inspect-dump-super.o cmds/inspect-tree-stats.o cmds/inspect-dump-file-extents.o cmds/filesystem-du.o \
>   	       mkfs/common.o check/mode-common.o check/mode-lowmem.o
>   libbtrfs_objects = common/send-stream.o common/send-utils.o kernel-lib/rbtree.o btrfs-list.o \
>   		   kernel-lib/radix-tree.o common/extent-cache.o kernel-shared/extent_io.o \
> diff --git a/cmds/commands.h b/cmds/commands.h
> index 8fa85d6c..55de248e 100644
> --- a/cmds/commands.h
> +++ b/cmds/commands.h
> @@ -154,5 +154,5 @@ DECLARE_COMMAND(select_super);
>   DECLARE_COMMAND(dump_super);
>   DECLARE_COMMAND(debug_tree);
>   DECLARE_COMMAND(rescue);
> -
> +DECLARE_COMMAND(inspect_dump_file_extents);
>   #endif
> diff --git a/cmds/inspect-dump-file-extents.c b/cmds/inspect-dump-file-extents.c
> new file mode 100644
> index 00000000..99aec7d7
> --- /dev/null
> +++ b/cmds/inspect-dump-file-extents.c
> @@ -0,0 +1,130 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the
> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> + * Boston, MA 021110-1307, USA.
> + */
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +
> +#include <sys/ioctl.h>
> +
> +#include "common/utils.h"
> +#include "cmds/commands.h"
> +
> +static const char * const cmd_inspect_dump_file_extents_usage[] = {
> +	"btrfs inspect-internal dump-extent path",
> +	"Dump file extent in a textual form",
> +	NULL
> +};
> +
> +static int cmd_inspect_dump_file_extents(const struct cmd_struct *cmd,
> +										 int argc, char **argv)
> +{
> +	int fd;
> +	struct stat statbuf;
> +	struct btrfs_ioctl_ino_lookup_args lookup;
> +	struct btrfs_ioctl_search_args args;
> +	struct btrfs_ioctl_search_key *sk = &args.key;
> +	struct btrfs_file_extent_item *extent_item;
> +	struct btrfs_ioctl_search_header *header;
> +	u64 pos;
> +	u64 buf_off;
> +	u64 len;
> +	u64 begin;
> +	u64 physical;
> +	u64 offset;
> +	u64 physical_len;
> +	int ret;
> +	int i;
> +
> +	fd = open(argv[optind], O_RDONLY);
> +	if (fd < 0) {
> +		error("cannot open %s: %m", argv[optind]);
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	if (fstat(fd, &statbuf) < 0) {
> +		error("failed to fstat %s: %m", argv[optind]);
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	lookup.treeid = 0;
> +	lookup.objectid = BTRFS_FIRST_FREE_OBJECTID;
> +
> +	if (ioctl(fd, BTRFS_IOC_INO_LOOKUP, &lookup) < 0) {
> +		error("failed to lookup inode %s: %m", argv[optind]);
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	pos = 0;
> +
> +	sk->tree_id = lookup.treeid;
> +	sk->min_objectid = statbuf.st_ino;
> +	sk->max_objectid = statbuf.st_ino;
> +
> +	sk->max_offset = UINT64_MAX;
> +	sk->min_transid = 0;
> +	sk->max_transid = UINT64_MAX;
> +	sk->min_type = sk->max_type = BTRFS_EXTENT_DATA_KEY;
> +	sk->nr_items = 4096;
> +
> +	while (statbuf.st_size > pos) {
> +		sk->min_offset = pos;
> +		if (ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args)) {
> +			error("failed to search tree ioctl %s: %m", argv[optind]);
> +			ret = 1;
> +			goto out;
> +		}
> +
> +		buf_off = 0;
> +		for(i=0; i<sk->nr_items; ++i) {
> +			header = (struct btrfs_ioctl_search_header *)(args.buf + buf_off);
> +
> +			if (btrfs_search_header_type(header) == BTRFS_EXTENT_DATA_KEY) {
> +				extent_item = (struct btrfs_file_extent_item *)(header + 1);
> +				begin = btrfs_search_header_offset(header);
> +
> +				printf("begin = 0x%llx, ", begin);
> +				switch (extent_item->type) {
> +				case BTRFS_FILE_EXTENT_INLINE:
> +					len = le64_to_cpu(extent_item->ram_bytes);
> +					printf("end = 0x%llx\n", begin + len);
> +					break;
> +				case BTRFS_FILE_EXTENT_REG:
> +				case BTRFS_FILE_EXTENT_PREALLOC:
> +					len = le64_to_cpu(extent_item->num_bytes);
> +					physical = le64_to_cpu(extent_item->disk_bytenr);
> +					physical_len = le64_to_cpu(extent_item->disk_num_bytes);
> +					offset = le64_to_cpu(extent_item->offset);
> +					printf("end = 0x%llx, physical = 0x%llx, physical_len = 0x%llx\n",
> +						   begin + len, physical, physical_len);
> +					break;
> +				}
> +
> +			}
> +			buf_off += sizeof(*header) + btrfs_search_header_len(header);
> +			pos += len;
> +		}
> +
> +	}
> +	ret = 0;
> +out:
> +	close(fd);
> +	return ret;
> +}
> +DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
> diff --git a/cmds/inspect.c b/cmds/inspect.c
> index 2ef5f4b6..dfb0e27b 100644
> --- a/cmds/inspect.c
> +++ b/cmds/inspect.c
> @@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
>   		&cmd_struct_inspect_dump_tree,
>   		&cmd_struct_inspect_dump_super,
>   		&cmd_struct_inspect_tree_stats,
> +		&cmd_struct_inspect_dump_file_extents,
>   		NULL
>   	}
>   };
>

Re: [RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[Qu Wenruo]

On 2021/7/11 上午6:38, Qu Wenruo wrote:
>
>
> On 2021/7/10 下午10:41, Sidong Yang wrote:
>> This patch adds an subcommand in inspect-internal. It dumps file
>> extents of
>> the file that user provided. It helps to show the internal information
>> about file extents comprise the file.
>
> So this is going to be the combined command of filemap + btrfs-map-logical.
>
> But how do you handle dirty pages which hasn't yet been flushed to disk?
>
> Thanks,
> Qu
>>
>> Signed-off-by: Sidong Yang <realwakka@gmail.com>
>> ---
>> Hi, I'm writing an patch that adds an subcommand that dumps file
>> extents of
>> the file. I don't have much idea about btrfs but I referenced btrfs
>> wiki, dev-docs, and also Zygo's bee. It's just like draft.
>>
>> I have some questions below.
>>
>> The command works like below,
>> # ./btrfs inspect-internal dump-file-extents /mnt/bb/TAGS
>> begin = 0x0, end = 0x20000, physical = 0x4fdf5000, physical_len = 0xc000
>> begin = 0x20000, end = 0x40000, physical = 0x4fe01000, physical_len =
>> 0xc000
>> begin = 0x40000, end = 0x47000, physical = 0x4fe0d000, physical_len =
>> 0x3000
>>
>> What format would be better?
>> Is it better to just use the variable name as it is? like disk_bytenr
>> not like physical_len?
>>
>> And what option do we need? like showing compression or file extent type?
>>
>> Any comments will be welcome. Thanks!
>> ---
>>   Makefile                         |   2 +-
>>   cmds/commands.h                  |   2 +-
>>   cmds/inspect-dump-file-extents.c | 130 +++++++++++++++++++++++++++++++
>>   cmds/inspect.c                   |   1 +
>>   4 files changed, 133 insertions(+), 2 deletions(-)
>>   create mode 100644 cmds/inspect-dump-file-extents.c
>>
>> diff --git a/Makefile b/Makefile
>> index a1cc457b..911e16de 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -156,7 +156,7 @@ cmds_objects = cmds/subvolume.o cmds/filesystem.o
>> cmds/device.o cmds/scrub.o \
>>              cmds/restore.o cmds/rescue.o cmds/rescue-chunk-recover.o \
>>              cmds/rescue-super-recover.o \
>>              cmds/property.o cmds/filesystem-usage.o
>> cmds/inspect-dump-tree.o \
>> -           cmds/inspect-dump-super.o cmds/inspect-tree-stats.o
>> cmds/filesystem-du.o \
>> +           cmds/inspect-dump-super.o cmds/inspect-tree-stats.o
>> cmds/inspect-dump-file-extents.o cmds/filesystem-du.o \
>>              mkfs/common.o check/mode-common.o check/mode-lowmem.o
>>   libbtrfs_objects = common/send-stream.o common/send-utils.o
>> kernel-lib/rbtree.o btrfs-list.o \
>>              kernel-lib/radix-tree.o common/extent-cache.o
>> kernel-shared/extent_io.o \
>> diff --git a/cmds/commands.h b/cmds/commands.h
>> index 8fa85d6c..55de248e 100644
>> --- a/cmds/commands.h
>> +++ b/cmds/commands.h
>> @@ -154,5 +154,5 @@ DECLARE_COMMAND(select_super);
>>   DECLARE_COMMAND(dump_super);
>>   DECLARE_COMMAND(debug_tree);
>>   DECLARE_COMMAND(rescue);
>> -
>> +DECLARE_COMMAND(inspect_dump_file_extents);
>>   #endif
>> diff --git a/cmds/inspect-dump-file-extents.c
>> b/cmds/inspect-dump-file-extents.c
>> new file mode 100644
>> index 00000000..99aec7d7
>> --- /dev/null
>> +++ b/cmds/inspect-dump-file-extents.c
>> @@ -0,0 +1,130 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this program; if not, write to the
>> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>> + * Boston, MA 021110-1307, USA.
>> + */
>> +
>> +#include <unistd.h>
>> +#include <stdio.h>
>> +#include <fcntl.h>
>> +
>> +#include <sys/ioctl.h>
>> +
>> +#include "common/utils.h"
>> +#include "cmds/commands.h"
>> +
>> +static const char * const cmd_inspect_dump_file_extents_usage[] = {
>> +    "btrfs inspect-internal dump-extent path",
>> +    "Dump file extent in a textual form",
>> +    NULL
>> +};
>> +
>> +static int cmd_inspect_dump_file_extents(const struct cmd_struct *cmd,
>> +                                         int argc, char **argv)
>> +{
>> +    int fd;
>> +    struct stat statbuf;
>> +    struct btrfs_ioctl_ino_lookup_args lookup;
>> +    struct btrfs_ioctl_search_args args;
>> +    struct btrfs_ioctl_search_key *sk = &args.key;
>> +    struct btrfs_file_extent_item *extent_item;
>> +    struct btrfs_ioctl_search_header *header;
>> +    u64 pos;
>> +    u64 buf_off;
>> +    u64 len;
>> +    u64 begin;
>> +    u64 physical;
>> +    u64 offset;
>> +    u64 physical_len;
>> +    int ret;
>> +    int i;
>> +
>> +    fd = open(argv[optind], O_RDONLY);
>> +    if (fd < 0) {
>> +        error("cannot open %s: %m", argv[optind]);
>> +        ret = 1;
>> +        goto out;
>> +    }
>> +
>> +    if (fstat(fd, &statbuf) < 0) {
>> +        error("failed to fstat %s: %m", argv[optind]);
>> +        ret = 1;
>> +        goto out;
>> +    }
>> +
>> +    lookup.treeid = 0;
>> +    lookup.objectid = BTRFS_FIRST_FREE_OBJECTID;
>> +
>> +    if (ioctl(fd, BTRFS_IOC_INO_LOOKUP, &lookup) < 0) {
>> +        error("failed to lookup inode %s: %m", argv[optind]);
>> +        ret = 1;
>> +        goto out;
>> +    }
>> +
>> +    pos = 0;
>> +
>> +    sk->tree_id = lookup.treeid;
>> +    sk->min_objectid = statbuf.st_ino;
>> +    sk->max_objectid = statbuf.st_ino;
>> +
>> +    sk->max_offset = UINT64_MAX;
>> +    sk->min_transid = 0;
>> +    sk->max_transid = UINT64_MAX;
>> +    sk->min_type = sk->max_type = BTRFS_EXTENT_DATA_KEY;
>> +    sk->nr_items = 4096;
>> +
>> +    while (statbuf.st_size > pos) {
>> +        sk->min_offset = pos;
>> +        if (ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args)) {
>> +            error("failed to search tree ioctl %s: %m", argv[optind]);
>> +            ret = 1;
>> +            goto out;
>> +        }
>> +
>> +        buf_off = 0;
>> +        for(i=0; i<sk->nr_items; ++i) {
>> +            header = (struct btrfs_ioctl_search_header *)(args.buf +
>> buf_off);
>> +
>> +            if (btrfs_search_header_type(header) ==
>> BTRFS_EXTENT_DATA_KEY) {
>> +                extent_item = (struct btrfs_file_extent_item
>> *)(header + 1);
>> +                begin = btrfs_search_header_offset(header);
>> +
>> +                printf("begin = 0x%llx, ", begin);
>> +                switch (extent_item->type) {
>> +                case BTRFS_FILE_EXTENT_INLINE:
>> +                    len = le64_to_cpu(extent_item->ram_bytes);
>> +                    printf("end = 0x%llx\n", begin + len);
>> +                    break;
>> +                case BTRFS_FILE_EXTENT_REG:


OK, so you're doing on-disk file extent search.

This is fine, but under most case fiemap ioctl would be enough, not to
mention fiemap can also handle pages not yet written to disk (by writing
them back though).

Although this manual search provides much better info for compressed
extent, but unfortunately here you didn't do any extra handling for them.

Thus so far this is no better than fiemap to get the logical bytenr.

And I can't be more wrong, by thinking you're also doing the chunk
lookup, which you didn't.

So I don't see any benefit compared to regular fiemap.

In fact, fiemap can provide more info than your initial draft.
Fiemap can already show if the map is compressed (but can't show the
compressed size yet).

Your draft can be improved to:

- Show the compression algorithm
   Which fiemap can't

- Show the compressed size
   Which fiemap can't either.


>> +                case BTRFS_FILE_EXTENT_PREALLOC:
>> +                    len = le64_to_cpu(extent_item->num_bytes);
>> +                    physical = le64_to_cpu(extent_item->disk_bytenr);
>> +                    physical_len =
>> le64_to_cpu(extent_item->disk_num_bytes);
>> +                    offset = le64_to_cpu(extent_item->offset);
>> +                    printf("end = 0x%llx, physical = 0x%llx,
>> physical_len = 0x%llx\n",

Currently we only use %llx for flags, for bytenr we always %llu.
You could refer to print-tree.c to see more examples.

And I don't really like the word "physical" here.

In fact the bytenr are all btrfs logical bytenr, which makes no direct
sense for its physical location on disk.

For real physical bytenr, we need something like (device id, physical
offset), and needs to do a chunk map lookup.

Thanks,
Qu

>> +                           begin + len, physical, physical_len);
>> +                    break;
>> +                }

>> +
>> +            }
>> +            buf_off += sizeof(*header) +
>> btrfs_search_header_len(header);
>> +            pos += len;
>> +        }
>> +
>> +    }
>> +    ret = 0;
>> +out:
>> +    close(fd);
>> +    return ret;
>> +}
>> +DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
>> diff --git a/cmds/inspect.c b/cmds/inspect.c
>> index 2ef5f4b6..dfb0e27b 100644
>> --- a/cmds/inspect.c
>> +++ b/cmds/inspect.c
>> @@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
>>           &cmd_struct_inspect_dump_tree,
>>           &cmd_struct_inspect_dump_super,
>>           &cmd_struct_inspect_tree_stats,
>> +        &cmd_struct_inspect_dump_file_extents,
>>           NULL
>>       }
>>   };
>>

Re: [RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[Sidong Yang]

On Sun, Jul 11, 2021 at 09:33:43AM +0800, Qu Wenruo wrote:

Hi, I really thank you for review.
> 
> 
> On 2021/7/11 上午6:38, Qu Wenruo wrote:
> > 
> > 
> > On 2021/7/10 下午10:41, Sidong Yang wrote:
> > > This patch adds an subcommand in inspect-internal. It dumps file
> > > extents of
> > > the file that user provided. It helps to show the internal information
> > > about file extents comprise the file.
> > 
> > So this is going to be the combined command of filemap + btrfs-map-logical.
> > 
> > But how do you handle dirty pages which hasn't yet been flushed to disk?

I have no idea about this. Is there any references to get dirty pages
information?

> > 
> > Thanks,
> > Qu
> > > 
> > > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > > ---
> > > Hi, I'm writing an patch that adds an subcommand that dumps file
> > > extents of
> > > the file. I don't have much idea about btrfs but I referenced btrfs
> > > wiki, dev-docs, and also Zygo's bee. It's just like draft.
> > > 
> > > I have some questions below.
> > > 
> > > The command works like below,
> > > # ./btrfs inspect-internal dump-file-extents /mnt/bb/TAGS
> > > begin = 0x0, end = 0x20000, physical = 0x4fdf5000, physical_len = 0xc000
> > > begin = 0x20000, end = 0x40000, physical = 0x4fe01000, physical_len =
> > > 0xc000
> > > begin = 0x40000, end = 0x47000, physical = 0x4fe0d000, physical_len =
> > > 0x3000
> > > 
> > > What format would be better?
> > > Is it better to just use the variable name as it is? like disk_bytenr
> > > not like physical_len?
> > > 
> > > And what option do we need? like showing compression or file extent type?
> > > 
> > > Any comments will be welcome. Thanks!
> > > ---
> > >   Makefile                         |   2 +-
> > >   cmds/commands.h                  |   2 +-
> > >   cmds/inspect-dump-file-extents.c | 130 +++++++++++++++++++++++++++++++
> > >   cmds/inspect.c                   |   1 +
> > >   4 files changed, 133 insertions(+), 2 deletions(-)
> > >   create mode 100644 cmds/inspect-dump-file-extents.c
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index a1cc457b..911e16de 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -156,7 +156,7 @@ cmds_objects = cmds/subvolume.o cmds/filesystem.o
> > > cmds/device.o cmds/scrub.o \
> > >              cmds/restore.o cmds/rescue.o cmds/rescue-chunk-recover.o \
> > >              cmds/rescue-super-recover.o \
> > >              cmds/property.o cmds/filesystem-usage.o
> > > cmds/inspect-dump-tree.o \
> > > -           cmds/inspect-dump-super.o cmds/inspect-tree-stats.o
> > > cmds/filesystem-du.o \
> > > +           cmds/inspect-dump-super.o cmds/inspect-tree-stats.o
> > > cmds/inspect-dump-file-extents.o cmds/filesystem-du.o \
> > >              mkfs/common.o check/mode-common.o check/mode-lowmem.o
> > >   libbtrfs_objects = common/send-stream.o common/send-utils.o
> > > kernel-lib/rbtree.o btrfs-list.o \
> > >              kernel-lib/radix-tree.o common/extent-cache.o
> > > kernel-shared/extent_io.o \
> > > diff --git a/cmds/commands.h b/cmds/commands.h
> > > index 8fa85d6c..55de248e 100644
> > > --- a/cmds/commands.h
> > > +++ b/cmds/commands.h
> > > @@ -154,5 +154,5 @@ DECLARE_COMMAND(select_super);
> > >   DECLARE_COMMAND(dump_super);
> > >   DECLARE_COMMAND(debug_tree);
> > >   DECLARE_COMMAND(rescue);
> > > -
> > > +DECLARE_COMMAND(inspect_dump_file_extents);
> > >   #endif
> > > diff --git a/cmds/inspect-dump-file-extents.c
> > > b/cmds/inspect-dump-file-extents.c
> > > new file mode 100644
> > > index 00000000..99aec7d7
> > > --- /dev/null
> > > +++ b/cmds/inspect-dump-file-extents.c
> > > @@ -0,0 +1,130 @@
> > > +/*
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public
> > > + * License v2 as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public
> > > + * License along with this program; if not, write to the
> > > + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> > > + * Boston, MA 021110-1307, USA.
> > > + */
> > > +
> > > +#include <unistd.h>
> > > +#include <stdio.h>
> > > +#include <fcntl.h>
> > > +
> > > +#include <sys/ioctl.h>
> > > +
> > > +#include "common/utils.h"
> > > +#include "cmds/commands.h"
> > > +
> > > +static const char * const cmd_inspect_dump_file_extents_usage[] = {
> > > +    "btrfs inspect-internal dump-extent path",
> > > +    "Dump file extent in a textual form",
> > > +    NULL
> > > +};
> > > +
> > > +static int cmd_inspect_dump_file_extents(const struct cmd_struct *cmd,
> > > +                                         int argc, char **argv)
> > > +{
> > > +    int fd;
> > > +    struct stat statbuf;
> > > +    struct btrfs_ioctl_ino_lookup_args lookup;
> > > +    struct btrfs_ioctl_search_args args;
> > > +    struct btrfs_ioctl_search_key *sk = &args.key;
> > > +    struct btrfs_file_extent_item *extent_item;
> > > +    struct btrfs_ioctl_search_header *header;
> > > +    u64 pos;
> > > +    u64 buf_off;
> > > +    u64 len;
> > > +    u64 begin;
> > > +    u64 physical;
> > > +    u64 offset;
> > > +    u64 physical_len;
> > > +    int ret;
> > > +    int i;
> > > +
> > > +    fd = open(argv[optind], O_RDONLY);
> > > +    if (fd < 0) {
> > > +        error("cannot open %s: %m", argv[optind]);
> > > +        ret = 1;
> > > +        goto out;
> > > +    }
> > > +
> > > +    if (fstat(fd, &statbuf) < 0) {
> > > +        error("failed to fstat %s: %m", argv[optind]);
> > > +        ret = 1;
> > > +        goto out;
> > > +    }
> > > +
> > > +    lookup.treeid = 0;
> > > +    lookup.objectid = BTRFS_FIRST_FREE_OBJECTID;
> > > +
> > > +    if (ioctl(fd, BTRFS_IOC_INO_LOOKUP, &lookup) < 0) {
> > > +        error("failed to lookup inode %s: %m", argv[optind]);
> > > +        ret = 1;
> > > +        goto out;
> > > +    }
> > > +
> > > +    pos = 0;
> > > +
> > > +    sk->tree_id = lookup.treeid;
> > > +    sk->min_objectid = statbuf.st_ino;
> > > +    sk->max_objectid = statbuf.st_ino;
> > > +
> > > +    sk->max_offset = UINT64_MAX;
> > > +    sk->min_transid = 0;
> > > +    sk->max_transid = UINT64_MAX;
> > > +    sk->min_type = sk->max_type = BTRFS_EXTENT_DATA_KEY;
> > > +    sk->nr_items = 4096;
> > > +
> > > +    while (statbuf.st_size > pos) {
> > > +        sk->min_offset = pos;
> > > +        if (ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args)) {
> > > +            error("failed to search tree ioctl %s: %m", argv[optind]);
> > > +            ret = 1;
> > > +            goto out;
> > > +        }
> > > +
> > > +        buf_off = 0;
> > > +        for(i=0; i<sk->nr_items; ++i) {
> > > +            header = (struct btrfs_ioctl_search_header *)(args.buf +
> > > buf_off);
> > > +
> > > +            if (btrfs_search_header_type(header) ==
> > > BTRFS_EXTENT_DATA_KEY) {
> > > +                extent_item = (struct btrfs_file_extent_item
> > > *)(header + 1);
> > > +                begin = btrfs_search_header_offset(header);
> > > +
> > > +                printf("begin = 0x%llx, ", begin);
> > > +                switch (extent_item->type) {
> > > +                case BTRFS_FILE_EXTENT_INLINE:
> > > +                    len = le64_to_cpu(extent_item->ram_bytes);
> > > +                    printf("end = 0x%llx\n", begin + len);
> > > +                    break;
> > > +                case BTRFS_FILE_EXTENT_REG:
> 
> 
> OK, so you're doing on-disk file extent search.
> 
> This is fine, but under most case fiemap ioctl would be enough, not to
> mention fiemap can also handle pages not yet written to disk (by writing
> them back though).

It would be better that this patch also can do it.

> 
> Although this manual search provides much better info for compressed
> extent, but unfortunately here you didn't do any extra handling for them.

It also good to show compression information.

> 
> Thus so far this is no better than fiemap to get the logical bytenr.
> 
> And I can't be more wrong, by thinking you're also doing the chunk
> lookup, which you didn't.
Sorry, I'm confused. Is it needed to do the chunk lookup?
> 
> So I don't see any benefit compared to regular fiemap.
> 
> In fact, fiemap can provide more info than your initial draft.
> Fiemap can already show if the map is compressed (but can't show the
> compressed size yet).
> 
> Your draft can be improved to:
> 
> - Show the compression algorithm
>   Which fiemap can't
> 
> - Show the compressed size
>   Which fiemap can't either.
> 
I understand that the point is that this command is nothing better than
fiemap ioctl now. but It can be improved by showing more information
that fiemap doesn't provide like compression algorithm.
> 
> > > +                case BTRFS_FILE_EXTENT_PREALLOC:
> > > +                    len = le64_to_cpu(extent_item->num_bytes);
> > > +                    physical = le64_to_cpu(extent_item->disk_bytenr);
> > > +                    physical_len =
> > > le64_to_cpu(extent_item->disk_num_bytes);
> > > +                    offset = le64_to_cpu(extent_item->offset);
> > > +                    printf("end = 0x%llx, physical = 0x%llx,
> > > physical_len = 0x%llx\n",
> 
> Currently we only use %llx for flags, for bytenr we always %llu.
> You could refer to print-tree.c to see more examples.
Thanks! I'll fix this.
> 
> And I don't really like the word "physical" here.
> 
> In fact the bytenr are all btrfs logical bytenr, which makes no direct
> sense for its physical location on disk.
Yeah, the word "physical" is not good. would It be better to write as
"disk_bytenr"?

Thanks,
Sidong
> 
> For real physical bytenr, we need something like (device id, physical
> offset), and needs to do a chunk map lookup.
> 
> Thanks,
> Qu
> 
> > > +                           begin + len, physical, physical_len);
> > > +                    break;
> > > +                }
> 
> > > +
> > > +            }
> > > +            buf_off += sizeof(*header) +
> > > btrfs_search_header_len(header);
> > > +            pos += len;
> > > +        }
> > > +
> > > +    }
> > > +    ret = 0;
> > > +out:
> > > +    close(fd);
> > > +    return ret;
> > > +}
> > > +DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
> > > diff --git a/cmds/inspect.c b/cmds/inspect.c
> > > index 2ef5f4b6..dfb0e27b 100644
> > > --- a/cmds/inspect.c
> > > +++ b/cmds/inspect.c
> > > @@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
> > >           &cmd_struct_inspect_dump_tree,
> > >           &cmd_struct_inspect_dump_super,
> > >           &cmd_struct_inspect_tree_stats,
> > > +        &cmd_struct_inspect_dump_file_extents,
> > >           NULL
> > >       }
> > >   };
> > > 

Re: [RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[Qu Wenruo]

On 2021/7/11 下午5:02, Sidong Yang wrote:
> On Sun, Jul 11, 2021 at 09:33:43AM +0800, Qu Wenruo wrote:
>
> Hi, I really thank you for review.
>>
>>
>> On 2021/7/11 上午6:38, Qu Wenruo wrote:
>>>
>>>
>>> On 2021/7/10 下午10:41, Sidong Yang wrote:
>>>> This patch adds an subcommand in inspect-internal. It dumps file
>>>> extents of
>>>> the file that user provided. It helps to show the internal information
>>>> about file extents comprise the file.
>>>
>>> So this is going to be the combined command of filemap + btrfs-map-logical.
>>>
>>> But how do you handle dirty pages which hasn't yet been flushed to disk?
>
> I have no idea about this. Is there any references to get dirty pages
> information?

For dirty pages there is no way to know their file extent info.
Until we write them back.

This is delayed allocation utilized by all modern filesystem, and is one
of the core concept to improve filesystem performance.

If you want to dig into the idea deeper, I would recommend to see how
btrfs_buffered_write() works.
It just does space reservation and copy the data to page cache, then
call it all day.

The real writeback happen when extent_writepages() happens, which then
allocate real on-disk file extents and emit the IO.


For the dirty page writeback part, you can check the common
fiemap_prep() function in fs/ioctl.c.

If we have FIEMAP_FLAG_SYNC, then it will inform the filesystem to
writeback the inode before calling the fs specific fiemap code.

With that FIEMAP_FLAG_SYNC flag, then it's pretty much the same behavior
of your draft, it just cares what's already in the subvolume tree,
ignore the dirty pages completely.

And in btrfs, extent_fiemap() function in fs/btrfs/extent_io.c, is doing
the main work.

It's mostly the enhanced version of your draft, but convert the output
to fiemap ioctl format, with extra work like merging extents if possible.

It would be a pretty educational experience to read the code, as it's
not that complex.

[...]
>>
>>
>> OK, so you're doing on-disk file extent search.
>>
>> This is fine, but under most case fiemap ioctl would be enough, not to
>> mention fiemap can also handle pages not yet written to disk (by writing
>> them back though).
>
> It would be better that this patch also can do it.

But since the fiemap ioctl currently can't report the real size of
compressed data, it can be sometimes pretty confusing to read the fiemap
result.

E.g.:

# mount /dev/test/test  -o compress /mnt/btrfs/
# xfs_io -f -c "pwrite 0 1M" /mnt/btrfs/file
# sync
# xfs_io  -f -c "fiemap -v" /mnt/btrfs/file
/mnt/btrfs/file:
  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
    0: [0..255]:        26624..26879       256   0x8
    1: [256..511]:      26632..26887       256   0x8
    2: [512..767]:      26640..26895       256   0x8
    3: [768..1023]:     26648..26903       256   0x8
    4: [1024..1279]:    26656..26911       256   0x8
    5: [1280..1535]:    26664..26919       256   0x8
    6: [1536..1791]:    26672..26927       256   0x8
    7: [1792..2047]:    26680..26935       256   0x9

Note the 0x8 flag, which indicates FIEMAP_EXTENT_ENCODED, and in btrfs
it means it's compressed.
But it can't show any btrfs specific info, the compression algorithm,
nor the compressed size.

And the final one, has one extra bit 0x1, which means FIEMAP_FLAG_LAST,
means it's the last extent of the file.


But if you check the result more carefully, you will find that the
ranges are overlapping.
This is the limitation of current fiemap ioctl, it always assume every
extent has the same size on-disk, thus causing above overlapping layout.

That's what your draft can do better, but personally speaking, I would
prefer to enhance the fiemap ioctl other than creating a btrfs specific
interface.

>
>>
>> Although this manual search provides much better info for compressed
>> extent, but unfortunately here you didn't do any extra handling for them.
>
> It also good to show compression information.
>
>>
>> Thus so far this is no better than fiemap to get the logical bytenr.
>>
>> And I can't be more wrong, by thinking you're also doing the chunk
>> lookup, which you didn't.
> Sorry, I'm confused. Is it needed to do the chunk lookup?

No, it's me getting confused by the "physical" words.

You don't need to do that at all.

>>
>> So I don't see any benefit compared to regular fiemap.
>>
>> In fact, fiemap can provide more info than your initial draft.
>> Fiemap can already show if the map is compressed (but can't show the
>> compressed size yet).
>>
>> Your draft can be improved to:
>>
>> - Show the compression algorithm
>>    Which fiemap can't
>>
>> - Show the compressed size
>>    Which fiemap can't either.
>>
> I understand that the point is that this command is nothing better than
> fiemap ioctl now. but It can be improved by showing more information
> that fiemap doesn't provide like compression algorithm.

Yes, but as mentioned, it would be better to enhance fiemap ioctl to do
more work.

It may be acceptable as a debug tool for a while, but a generic
interface would be the ultimate solution.

>>
>>>> +                case BTRFS_FILE_EXTENT_PREALLOC:
>>>> +                    len = le64_to_cpu(extent_item->num_bytes);
>>>> +                    physical = le64_to_cpu(extent_item->disk_bytenr);
>>>> +                    physical_len =
>>>> le64_to_cpu(extent_item->disk_num_bytes);
>>>> +                    offset = le64_to_cpu(extent_item->offset);
>>>> +                    printf("end = 0x%llx, physical = 0x%llx,
>>>> physical_len = 0x%llx\n",
>>
>> Currently we only use %llx for flags, for bytenr we always %llu.
>> You could refer to print-tree.c to see more examples.
> Thanks! I'll fix this.
>>
>> And I don't really like the word "physical" here.
>>
>> In fact the bytenr are all btrfs logical bytenr, which makes no direct
>> sense for its physical location on disk.
> Yeah, the word "physical" is not good. would It be better to write as
> "disk_bytenr"?

That would be really much better, then it completely follows the naming
in file extent item.

Thanks,
Qu

>
> Thanks,
> Sidong
>>
>> For real physical bytenr, we need something like (device id, physical
>> offset), and needs to do a chunk map lookup.
>>
>> Thanks,
>> Qu
>>
>>>> +                           begin + len, physical, physical_len);
>>>> +                    break;
>>>> +                }
>>
>>>> +
>>>> +            }
>>>> +            buf_off += sizeof(*header) +
>>>> btrfs_search_header_len(header);
>>>> +            pos += len;
>>>> +        }
>>>> +
>>>> +    }
>>>> +    ret = 0;
>>>> +out:
>>>> +    close(fd);
>>>> +    return ret;
>>>> +}
>>>> +DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
>>>> diff --git a/cmds/inspect.c b/cmds/inspect.c
>>>> index 2ef5f4b6..dfb0e27b 100644
>>>> --- a/cmds/inspect.c
>>>> +++ b/cmds/inspect.c
>>>> @@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
>>>>            &cmd_struct_inspect_dump_tree,
>>>>            &cmd_struct_inspect_dump_super,
>>>>            &cmd_struct_inspect_tree_stats,
>>>> +        &cmd_struct_inspect_dump_file_extents,
>>>>            NULL
>>>>        }
>>>>    };
>>>>

Re: [RFC] btrfs-progs: cmds: Add subcommand that dumps file extents

[Sidong Yang]

On Sun, Jul 11, 2021 at 06:58:49PM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/7/11 下午5:02, Sidong Yang wrote:
> > On Sun, Jul 11, 2021 at 09:33:43AM +0800, Qu Wenruo wrote:
> > 
> > Hi, I really thank you for review.
> > > 
> > > 
> > > On 2021/7/11 上午6:38, Qu Wenruo wrote:
> > > > 
> > > > 
> > > > On 2021/7/10 下午10:41, Sidong Yang wrote:
> > > > > This patch adds an subcommand in inspect-internal. It dumps file
> > > > > extents of
> > > > > the file that user provided. It helps to show the internal information
> > > > > about file extents comprise the file.
> > > > 
> > > > So this is going to be the combined command of filemap + btrfs-map-logical.
> > > > 
> > > > But how do you handle dirty pages which hasn't yet been flushed to disk?
> > 
> > I have no idea about this. Is there any references to get dirty pages
> > information?
> 
> For dirty pages there is no way to know their file extent info.
> Until we write them back.
> 
> This is delayed allocation utilized by all modern filesystem, and is one
> of the core concept to improve filesystem performance.
> 
> If you want to dig into the idea deeper, I would recommend to see how
> btrfs_buffered_write() works.
> It just does space reservation and copy the data to page cache, then
> call it all day.
> 
> The real writeback happen when extent_writepages() happens, which then
> allocate real on-disk file extents and emit the IO.
> 
> 
> For the dirty page writeback part, you can check the common
> fiemap_prep() function in fs/ioctl.c.
> 
> If we have FIEMAP_FLAG_SYNC, then it will inform the filesystem to
> writeback the inode before calling the fs specific fiemap code.
> 
> With that FIEMAP_FLAG_SYNC flag, then it's pretty much the same behavior
> of your draft, it just cares what's already in the subvolume tree,
> ignore the dirty pages completely.
> 
> And in btrfs, extent_fiemap() function in fs/btrfs/extent_io.c, is doing
> the main work.
> 
> It's mostly the enhanced version of your draft, but convert the output
> to fiemap ioctl format, with extra work like merging extents if possible.
> 
> It would be a pretty educational experience to read the code, as it's
> not that complex.

Yeah, I'm reading the code you said. It helps me to understand
filesystem. Thanks for detailed guide!

> 
> [...]
> > > 
> > > 
> > > OK, so you're doing on-disk file extent search.
> > > 
> > > This is fine, but under most case fiemap ioctl would be enough, not to
> > > mention fiemap can also handle pages not yet written to disk (by writing
> > > them back though).
> > 
> > It would be better that this patch also can do it.
> 
> But since the fiemap ioctl currently can't report the real size of
> compressed data, it can be sometimes pretty confusing to read the fiemap
> result.
> 
> E.g.:
> 
> # mount /dev/test/test  -o compress /mnt/btrfs/
> # xfs_io -f -c "pwrite 0 1M" /mnt/btrfs/file
> # sync
> # xfs_io  -f -c "fiemap -v" /mnt/btrfs/file
> /mnt/btrfs/file:
>  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>    0: [0..255]:        26624..26879       256   0x8
>    1: [256..511]:      26632..26887       256   0x8
>    2: [512..767]:      26640..26895       256   0x8
>    3: [768..1023]:     26648..26903       256   0x8
>    4: [1024..1279]:    26656..26911       256   0x8
>    5: [1280..1535]:    26664..26919       256   0x8
>    6: [1536..1791]:    26672..26927       256   0x8
>    7: [1792..2047]:    26680..26935       256   0x9
> 
> Note the 0x8 flag, which indicates FIEMAP_EXTENT_ENCODED, and in btrfs
> it means it's compressed.
> But it can't show any btrfs specific info, the compression algorithm,
> nor the compressed size.
> 
> And the final one, has one extra bit 0x1, which means FIEMAP_FLAG_LAST,
> means it's the last extent of the file.
> 
> 
> But if you check the result more carefully, you will find that the
> ranges are overlapping.
> This is the limitation of current fiemap ioctl, it always assume every
> extent has the same size on-disk, thus causing above overlapping layout.

I understood. I checked disk_bytenr and disk_num_bytes on my patch. And
the output wasn't overlapped.

> 
> That's what your draft can do better, but personally speaking, I would
> prefer to enhance the fiemap ioctl other than creating a btrfs specific
> interface.

I agree. Maybe many filesystem has a problem about this.
> 
> > 
> > > 
> > > Although this manual search provides much better info for compressed
> > > extent, but unfortunately here you didn't do any extra handling for them.
> > 
> > It also good to show compression information.
> > 
> > > 
> > > Thus so far this is no better than fiemap to get the logical bytenr.
> > > 
> > > And I can't be more wrong, by thinking you're also doing the chunk
> > > lookup, which you didn't.
> > Sorry, I'm confused. Is it needed to do the chunk lookup?
> 
> No, it's me getting confused by the "physical" words.
> 
> You don't need to do that at all.
> 
> > > 
> > > So I don't see any benefit compared to regular fiemap.
> > > 
> > > In fact, fiemap can provide more info than your initial draft.
> > > Fiemap can already show if the map is compressed (but can't show the
> > > compressed size yet).
> > > 
> > > Your draft can be improved to:
> > > 
> > > - Show the compression algorithm
> > >    Which fiemap can't
> > > 
> > > - Show the compressed size
> > >    Which fiemap can't either.
> > > 
> > I understand that the point is that this command is nothing better than
> > fiemap ioctl now. but It can be improved by showing more information
> > that fiemap doesn't provide like compression algorithm.
> 
> Yes, but as mentioned, it would be better to enhance fiemap ioctl to do
> more work.
> 
> It may be acceptable as a debug tool for a while, but a generic
> interface would be the ultimate solution.
> 
> > > 
> > > > > +                case BTRFS_FILE_EXTENT_PREALLOC:
> > > > > +                    len = le64_to_cpu(extent_item->num_bytes);
> > > > > +                    physical = le64_to_cpu(extent_item->disk_bytenr);
> > > > > +                    physical_len =
> > > > > le64_to_cpu(extent_item->disk_num_bytes);
> > > > > +                    offset = le64_to_cpu(extent_item->offset);
> > > > > +                    printf("end = 0x%llx, physical = 0x%llx,
> > > > > physical_len = 0x%llx\n",
> > > 
> > > Currently we only use %llx for flags, for bytenr we always %llu.
> > > You could refer to print-tree.c to see more examples.
> > Thanks! I'll fix this.
> > > 
> > > And I don't really like the word "physical" here.
> > > 
> > > In fact the bytenr are all btrfs logical bytenr, which makes no direct
> > > sense for its physical location on disk.
> > Yeah, the word "physical" is not good. would It be better to write as
> > "disk_bytenr"?
> 
> That would be really much better, then it completely follows the naming
> in file extent item.

Thanks for kind comments. I'll write the next version.

Thanks,
Sidong

> 
> Thanks,
> Qu
> 
> > 
> > Thanks,
> > Sidong
> > > 
> > > For real physical bytenr, we need something like (device id, physical
> > > offset), and needs to do a chunk map lookup.
> > > 
> > > Thanks,
> > > Qu
> > > 
> > > > > +                           begin + len, physical, physical_len);
> > > > > +                    break;
> > > > > +                }
> > > 
> > > > > +
> > > > > +            }
> > > > > +            buf_off += sizeof(*header) +
> > > > > btrfs_search_header_len(header);
> > > > > +            pos += len;
> > > > > +        }
> > > > > +
> > > > > +    }
> > > > > +    ret = 0;
> > > > > +out:
> > > > > +    close(fd);
> > > > > +    return ret;
> > > > > +}
> > > > > +DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
> > > > > diff --git a/cmds/inspect.c b/cmds/inspect.c
> > > > > index 2ef5f4b6..dfb0e27b 100644
> > > > > --- a/cmds/inspect.c
> > > > > +++ b/cmds/inspect.c
> > > > > @@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
> > > > >            &cmd_struct_inspect_dump_tree,
> > > > >            &cmd_struct_inspect_dump_super,
> > > > >            &cmd_struct_inspect_tree_stats,
> > > > > +        &cmd_struct_inspect_dump_file_extents,
> > > > >            NULL
> > > > >        }
> > > > >    };
> > > > >