* [PATCH 0/2] btrfs: remove btrfs_bio::logical member
@ 2021-10-08 7:29 Qu Wenruo
2021-10-08 7:29 ` [PATCH 1/2] btrfs: rename btrfs_dio_private::logical_offset to file_offset Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Qu Wenruo @ 2021-10-08 7:29 UTC (permalink / raw)
To: linux-btrfs
Although we have two call sites setting btrfs_bio::logical, but only one
call site it really reading btrfs_bio::logical.
Furthermore, that only reader can grab the same info from
btrfs_dio_private without any hassle.
Thus it means btrfs_bio::logical duplicated and can be removed.
This in fact proves my initial suspicion, that btrfs_bio::logical is
duplicated, and the logical bytenr can be fetched using
bio->bi_iter.bi_sector.
This saves 8 bytes from btrfs_bio, without any complicated refactor.
Qu Wenruo (2):
btrfs: rename btrfs_dio_private::logical_offset to file_offset
btrfs: remove btrfs_bio::logical member
fs/btrfs/btrfs_inode.h | 7 ++++++-
fs/btrfs/extent_io.c | 1 -
fs/btrfs/inode.c | 29 ++++++++++++++---------------
fs/btrfs/volumes.h | 1 -
4 files changed, 20 insertions(+), 18 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] btrfs: rename btrfs_dio_private::logical_offset to file_offset
2021-10-08 7:29 [PATCH 0/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
@ 2021-10-08 7:29 ` Qu Wenruo
2021-10-08 7:30 ` [PATCH 2/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
2021-10-13 14:30 ` [PATCH 0/2] " David Sterba
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2021-10-08 7:29 UTC (permalink / raw)
To: linux-btrfs
The naming of "logical_offset" can be confused with logical bytenr of
the dio range.
In fact it's file offset, and the naming "file_offset" is already widely
used in all other sites.
Just do the rename to avoid confusion.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/btrfs_inode.h | 7 ++++++-
fs/btrfs/inode.c | 12 ++++++------
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 602b426c286d..ab2a4a52e0bb 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -356,7 +356,12 @@ static inline bool btrfs_inode_in_log(struct btrfs_inode *inode, u64 generation)
struct btrfs_dio_private {
struct inode *inode;
- u64 logical_offset;
+
+ /*
+ * Since DIO can use anonymous page, we cannot use page_offset() to
+ * grab the file offset, thus need a dedicated member for file offset.
+ */
+ u64 file_offset;
u64 disk_bytenr;
/* Used for bio::bi_size */
u32 bytes;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f5faa6bedbd4..d65f7d6b7df1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8055,13 +8055,13 @@ static void btrfs_dio_private_put(struct btrfs_dio_private *dip)
if (btrfs_op(dip->dio_bio) == BTRFS_MAP_WRITE) {
__endio_write_update_ordered(BTRFS_I(dip->inode),
- dip->logical_offset,
+ dip->file_offset,
dip->bytes,
!dip->dio_bio->bi_status);
} else {
unlock_extent(&BTRFS_I(dip->inode)->io_tree,
- dip->logical_offset,
- dip->logical_offset + dip->bytes - 1);
+ dip->file_offset,
+ dip->file_offset + dip->bytes - 1);
}
bio_endio(dip->dio_bio);
@@ -8176,7 +8176,7 @@ static void btrfs_end_dio_bio(struct bio *bio)
if (err)
dip->dio_bio->bi_status = err;
- btrfs_record_physical_zoned(dip->inode, dip->logical_offset, bio);
+ btrfs_record_physical_zoned(dip->inode, dip->file_offset, bio);
bio_put(bio);
btrfs_dio_private_put(dip);
@@ -8218,7 +8218,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
} else {
u64 csum_offset;
- csum_offset = file_offset - dip->logical_offset;
+ csum_offset = file_offset - dip->file_offset;
csum_offset >>= fs_info->sectorsize_bits;
csum_offset *= fs_info->csum_size;
btrfs_bio(bio)->csum = dip->csums + csum_offset;
@@ -8256,7 +8256,7 @@ static struct btrfs_dio_private *btrfs_create_dio_private(struct bio *dio_bio,
return NULL;
dip->inode = inode;
- dip->logical_offset = file_offset;
+ dip->file_offset = file_offset;
dip->bytes = dio_bio->bi_iter.bi_size;
dip->disk_bytenr = dio_bio->bi_iter.bi_sector << 9;
dip->dio_bio = dio_bio;
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] btrfs: remove btrfs_bio::logical member
2021-10-08 7:29 [PATCH 0/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
2021-10-08 7:29 ` [PATCH 1/2] btrfs: rename btrfs_dio_private::logical_offset to file_offset Qu Wenruo
@ 2021-10-08 7:30 ` Qu Wenruo
2021-10-13 14:30 ` [PATCH 0/2] " David Sterba
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2021-10-08 7:30 UTC (permalink / raw)
To: linux-btrfs
The member btrfs_bio::logical is only initialized by two call sites:
- btrfs_repair_one_sector()
No corresponding site to utilize it.
- btrfs_submit_direct()
The corresponding site to utilize it is btrfs_check_read_dio_bio().
However for btrfs_check_read_dio_bio(), we can grab the file_offset from
btrfs_dio_private::file_offset directly.
Thus it turns out we don't really that btrfs_bio::logical member at all.
For btrfs_bio, the logical bytenr can be fetched from its
bio->bi_iter.bi_sector directly.
So let's just remove the member to save 8 bytes for structure btrfs_bio.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent_io.c | 1 -
fs/btrfs/inode.c | 17 ++++++++---------
fs/btrfs/volumes.h | 1 -
3 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index b10dc75eef1c..4e03a6d3aa32 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2673,7 +2673,6 @@ int btrfs_repair_one_sector(struct inode *inode,
}
bio_add_page(repair_bio, page, failrec->len, pgoff);
- repair_bbio->logical = failrec->start;
repair_bbio->iter = repair_bio->bi_iter;
btrfs_debug(btrfs_sb(inode->i_sb),
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d65f7d6b7df1..b9c70a073a24 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8089,10 +8089,11 @@ static blk_status_t submit_dio_repair_bio(struct inode *inode, struct bio *bio,
return ret;
}
-static blk_status_t btrfs_check_read_dio_bio(struct inode *inode,
+static blk_status_t btrfs_check_read_dio_bio(struct btrfs_dio_private *dip,
struct btrfs_bio *bbio,
const bool uptodate)
{
+ struct inode *inode = dip->inode;
struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
const u32 sectorsize = fs_info->sectorsize;
struct extent_io_tree *failure_tree = &BTRFS_I(inode)->io_failure_tree;
@@ -8100,7 +8101,8 @@ static blk_status_t btrfs_check_read_dio_bio(struct inode *inode,
const bool csum = !(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM);
struct bio_vec bvec;
struct bvec_iter iter;
- u64 start = bbio->logical;
+ const u64 orig_file_offset = dip->file_offset;
+ u64 start = orig_file_offset;
u32 bio_offset = 0;
blk_status_t err = BLK_STS_OK;
@@ -8122,10 +8124,10 @@ static blk_status_t btrfs_check_read_dio_bio(struct inode *inode,
} else {
int ret;
- ASSERT((start - bbio->logical) < UINT_MAX);
+ ASSERT((start - orig_file_offset) < UINT_MAX);
ret = btrfs_repair_one_sector(inode,
&bbio->bio,
- start - bbio->logical,
+ start - orig_file_offset,
bvec.bv_page, pgoff,
start, bbio->mirror_num,
submit_dio_repair_bio);
@@ -8168,10 +8170,8 @@ static void btrfs_end_dio_bio(struct bio *bio)
bio->bi_opf, bio->bi_iter.bi_sector,
bio->bi_iter.bi_size, err);
- if (bio_op(bio) == REQ_OP_READ) {
- err = btrfs_check_read_dio_bio(dip->inode,
- btrfs_bio(bio), !err);
- }
+ if (bio_op(bio) == REQ_OP_READ)
+ err = btrfs_check_read_dio_bio(dip, btrfs_bio(bio), !err);
if (err)
dip->dio_bio->bi_status = err;
@@ -8337,7 +8337,6 @@ static blk_qc_t btrfs_submit_direct(const struct iomap_iter *iter,
bio = btrfs_bio_clone_partial(dio_bio, clone_offset, clone_len);
bio->bi_private = dip;
bio->bi_end_io = btrfs_end_dio_bio;
- btrfs_bio(bio)->logical = file_offset;
if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
status = extract_ordered_extent(BTRFS_I(inode), bio,
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 83075d6855db..49322a43593a 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -313,7 +313,6 @@ struct btrfs_bio {
/* @device is for stripe IO submission. */
struct btrfs_device *device;
- u64 logical;
u8 *csum;
u8 csum_inline[BTRFS_BIO_INLINE_CSUM_SIZE];
struct bvec_iter iter;
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] btrfs: remove btrfs_bio::logical member
2021-10-08 7:29 [PATCH 0/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
2021-10-08 7:29 ` [PATCH 1/2] btrfs: rename btrfs_dio_private::logical_offset to file_offset Qu Wenruo
2021-10-08 7:30 ` [PATCH 2/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
@ 2021-10-13 14:30 ` David Sterba
2 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2021-10-13 14:30 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Fri, Oct 08, 2021 at 03:29:58PM +0800, Qu Wenruo wrote:
> Although we have two call sites setting btrfs_bio::logical, but only one
> call site it really reading btrfs_bio::logical.
>
> Furthermore, that only reader can grab the same info from
> btrfs_dio_private without any hassle.
>
> Thus it means btrfs_bio::logical duplicated and can be removed.
>
> This in fact proves my initial suspicion, that btrfs_bio::logical is
> duplicated, and the logical bytenr can be fetched using
> bio->bi_iter.bi_sector.
>
> This saves 8 bytes from btrfs_bio, without any complicated refactor.
Great, thanks. Added to misc-next.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-13 14:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-08 7:29 [PATCH 0/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
2021-10-08 7:29 ` [PATCH 1/2] btrfs: rename btrfs_dio_private::logical_offset to file_offset Qu Wenruo
2021-10-08 7:30 ` [PATCH 2/2] btrfs: remove btrfs_bio::logical member Qu Wenruo
2021-10-13 14:30 ` [PATCH 0/2] " 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.