[0/2] btrfs: paramater refactors for data and metadata endio call backs

Message

Qu Wenruo Nov. 9, 2020, 11:54 a.m. UTC

This is another cleanup exposed when I'm fixing my subpage patchset.

Dating back to the old time where we still have hooks for data/metadata
endio, we have a parameter called @phy_offset for both hooks.

That @phy_offset is the number of sectors compared to the bio on-disk
bytenr, and is used to grab the csum from btrfs_io_bio.

This is far from straightforward, and costs reader tons of time to grasp
the basic.

This patchset will change it by:
- Remove phy_offset completely for metadata
  Since metadata doesn't use btrfs_io_bio::csums[] at all, there is no
  need for it.

- Use @disk_bytenr to replace @phy_offset/@icsum
  Let the callee, check_data_csum() to calculate the offset from
  @disk_bytenr and bio to get the csum offset.

Also, since we know the @disk_bytenr should alwasy be inside the bio
range, add ASSERT() to check such assumption.

Qu Wenruo (2):
  btrfs: remove the phy_offset parameter for
    btrfs_validate_metadata_buffer()
  btrfs: use more straightforward disk_bytenr to replace icsum for
    check_data_csum()

 fs/btrfs/disk-io.c   |  2 +-
 fs/btrfs/disk-io.h   |  2 +-
 fs/btrfs/extent_io.c | 16 +++++++++-------
 fs/btrfs/inode.c     | 35 ++++++++++++++++++++++++++---------
 4 files changed, 37 insertions(+), 18 deletions(-)

Comments

Qu Wenruo Nov. 9, 2020, 11:57 a.m. UTC | #1

On 2020/11/9 下午7:54, Qu Wenruo wrote:
> This is another cleanup exposed when I'm fixing my subpage patchset.
> 
> Dating back to the old time where we still have hooks for data/metadata
> endio, we have a parameter called @phy_offset for both hooks.
> 
> That @phy_offset is the number of sectors compared to the bio on-disk

Well, phy_offset is the number of *bytes*, other than sectors.

I guess the mistake has already prove the point, it's really easy to get
screwed up for the @phy_offset and @icsum bad naming.

Thanks,
Qu

> bytenr, and is used to grab the csum from btrfs_io_bio.
> 
> This is far from straightforward, and costs reader tons of time to grasp
> the basic.
> 
> This patchset will change it by:
> - Remove phy_offset completely for metadata
>   Since metadata doesn't use btrfs_io_bio::csums[] at all, there is no
>   need for it.
> 
> - Use @disk_bytenr to replace @phy_offset/@icsum
>   Let the callee, check_data_csum() to calculate the offset from
>   @disk_bytenr and bio to get the csum offset.
> 
> Also, since we know the @disk_bytenr should alwasy be inside the bio
> range, add ASSERT() to check such assumption.
> 
> Qu Wenruo (2):
>   btrfs: remove the phy_offset parameter for
>     btrfs_validate_metadata_buffer()
>   btrfs: use more straightforward disk_bytenr to replace icsum for
>     check_data_csum()
> 
>  fs/btrfs/disk-io.c   |  2 +-
>  fs/btrfs/disk-io.h   |  2 +-
>  fs/btrfs/extent_io.c | 16 +++++++++-------
>  fs/btrfs/inode.c     | 35 ++++++++++++++++++++++++++---------
>  4 files changed, 37 insertions(+), 18 deletions(-)
>