On 17.12.19 г. 8:48 ч., Qu Wenruo wrote:
> All btrfs_file_extent_item members don't take checksum size into
> consideration.
>
> This bad comment looks like from early days where inlined data checksum
> (checksum is stored along with data) is being considered.
> But the reality is, we never support inlined data checksum since btrfs
> is mainlined.
>
> Remove this dead comment, add a new comment explaining how data checksum is
> stored, and remove the unnecessary data csum reference.
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> include/uapi/linux/btrfs_tree.h | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index 8e322e2c7e78..bfe6f38031a3 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -776,15 +776,16 @@ struct btrfs_file_extent_item {
> __u8 type;
>
> /*
> - * disk space consumed by the extent, checksum blocks are included
> - * in these numbers
> + * disk space consumed by the data extent.
> + * Checksum is stored in csum tree, thus no bytenr/length takes
> + * csum into consideration.
This wording is awkward. Simply say that checksum blocks are excluded
> *
> * 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)
> + * the logical offset in file blocks
> * 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
> @@ -792,8 +793,8 @@ struct btrfs_file_extent_item {
> */
> __le64 offset;
> /*
> - * the logical number of file blocks (no csums included). This
> - * always reflects the size uncompressed and without encoding.
> + * the logical number of file blocks. This always reflects the size
> + * uncompressed and without encoding.
I don't think this is better. Because one has to actually read the
comment about disk_bytenr to understand whether the following members
include exclude checksums. I prefer to explicitly state whether each
member includes/excludes the checksums. As it stands, a sensible
correction is to simply state (checksums excluded).
> */
> __le64 num_bytes;
>
>
