* [PATCH 1/3] btrfs: scrub: get rid of sector_t
2017-10-06 12:29 [PATCH 0/3] Cleanup sector_t David Sterba
@ 2017-10-06 12:29 ` David Sterba
2017-10-06 15:22 ` Edmund Nadolski
2017-10-07 0:15 ` Liu Bo
2017-10-06 12:29 ` [PATCH 2/3] btrfs: rename page offset parameter in submit_extent_page David Sterba
2017-10-06 12:30 ` [PATCH 3/3] btrfs: get rid of sector_t and use u64 offset " David Sterba
2 siblings, 2 replies; 9+ messages in thread
From: David Sterba @ 2017-10-06 12:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: bo.li.liu, David Sterba
The use of sector_t is not necessry, it's just for a warning. Switch to
u64 and rename the variable. The messages are adjusted as well.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/scrub.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index e3f6c49e5c4d..34ea2e7048c2 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -231,7 +231,7 @@ struct scrub_warning {
struct btrfs_path *path;
u64 extent_item_size;
const char *errstr;
- sector_t sector;
+ u64 physical;
u64 logical;
struct btrfs_device *dev;
};
@@ -797,10 +797,10 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
*/
for (i = 0; i < ipath->fspath->elem_cnt; ++i)
btrfs_warn_in_rcu(fs_info,
- "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
+"%s at logical %llu on dev %s, physical %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
swarn->errstr, swarn->logical,
rcu_str_deref(swarn->dev->name),
- (unsigned long long)swarn->sector,
+ swarn->physical,
root, inum, offset,
min(isize - offset, (u64)PAGE_SIZE), nlink,
(char *)(unsigned long)ipath->fspath->val[i]);
@@ -813,7 +813,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
"%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu: path resolving failed with ret=%d",
swarn->errstr, swarn->logical,
rcu_str_deref(swarn->dev->name),
- (unsigned long long)swarn->sector,
+ swarn->physical,
root, inum, offset, ret);
free_ipath(ipath);
@@ -845,7 +845,7 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
if (!path)
return;
- swarn.sector = (sblock->pagev[0]->physical) >> 9;
+ swarn.physical = sblock->pagev[0]->physical;
swarn.logical = sblock->pagev[0]->logical;
swarn.errstr = errstr;
swarn.dev = NULL;
@@ -868,10 +868,10 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
item_size, &ref_root,
&ref_level);
btrfs_warn_in_rcu(fs_info,
- "%s at logical %llu on dev %s, sector %llu: metadata %s (level %d) in tree %llu",
+"%s at logical %llu on dev %s, physical %llu: metadata %s (level %d) in tree %llu",
errstr, swarn.logical,
rcu_str_deref(dev->name),
- (unsigned long long)swarn.sector,
+ swarn.physical,
ref_level ? "node" : "leaf",
ret < 0 ? -1 : ref_level,
ret < 0 ? -1 : ref_root);
--
2.14.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] btrfs: scrub: get rid of sector_t
2017-10-06 12:29 ` [PATCH 1/3] btrfs: scrub: get rid of sector_t David Sterba
@ 2017-10-06 15:22 ` Edmund Nadolski
2017-10-06 15:48 ` David Sterba
2017-10-07 0:15 ` Liu Bo
1 sibling, 1 reply; 9+ messages in thread
From: Edmund Nadolski @ 2017-10-06 15:22 UTC (permalink / raw)
To: David Sterba, linux-btrfs; +Cc: bo.li.liu
On 10/06/2017 06:29 AM, David Sterba wrote:
> The use of sector_t is not necessry, it's just for a warning. Switch to
> u64 and rename the variable. The messages are adjusted as well.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/scrub.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index e3f6c49e5c4d..34ea2e7048c2 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -231,7 +231,7 @@ struct scrub_warning {
> struct btrfs_path *path;
> u64 extent_item_size;
> const char *errstr;
> - sector_t sector;
> + u64 physical;
> u64 logical;
> struct btrfs_device *dev;
> };
Just a thought, 'physical' alone seems a bit ambiguous, so
a comment to add context would help clarify.
> @@ -797,10 +797,10 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
> */
> for (i = 0; i < ipath->fspath->elem_cnt; ++i)
> btrfs_warn_in_rcu(fs_info,
> - "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
> +"%s at logical %llu on dev %s, physical %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
> swarn->errstr, swarn->logical,
> rcu_str_deref(swarn->dev->name),
> - (unsigned long long)swarn->sector,
> + swarn->physical,
> root, inum, offset,
> min(isize - offset, (u64)PAGE_SIZE), nlink,
> (char *)(unsigned long)ipath->fspath->val[i]);
> @@ -813,7 +813,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
> "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu: path resolving failed with ret=%d",
This still says 'sector' here but was changed for the other print strings.
> swarn->errstr, swarn->logical,
> rcu_str_deref(swarn->dev->name),
> - (unsigned long long)swarn->sector,
> + swarn->physical,
> root, inum, offset, ret);
>
> free_ipath(ipath);
> @@ -845,7 +845,7 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
> if (!path)
> return;
>
> - swarn.sector = (sblock->pagev[0]->physical) >> 9;
> + swarn.physical = sblock->pagev[0]->physical;
Dropping the '>> 9', so this is a semantic change in addition to the rename?
Thx,
Ed
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] btrfs: scrub: get rid of sector_t
2017-10-06 15:22 ` Edmund Nadolski
@ 2017-10-06 15:48 ` David Sterba
0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2017-10-06 15:48 UTC (permalink / raw)
To: Edmund Nadolski; +Cc: David Sterba, linux-btrfs, bo.li.liu
On Fri, Oct 06, 2017 at 09:22:42AM -0600, Edmund Nadolski wrote:
> On 10/06/2017 06:29 AM, David Sterba wrote:
> > The use of sector_t is not necessry, it's just for a warning. Switch to
> > u64 and rename the variable. The messages are adjusted as well.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> > fs/btrfs/scrub.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> > index e3f6c49e5c4d..34ea2e7048c2 100644
> > --- a/fs/btrfs/scrub.c
> > +++ b/fs/btrfs/scrub.c
> > @@ -231,7 +231,7 @@ struct scrub_warning {
> > struct btrfs_path *path;
> > u64 extent_item_size;
> > const char *errstr;
> > - sector_t sector;
> > + u64 physical;
> > u64 logical;
> > struct btrfs_device *dev;
> > };
>
> Just a thought, 'physical' alone seems a bit ambiguous, so
> a comment to add context would help clarify.
The other thing that came to my mind was 'offset', but this would be
confusing as well, as there is already the 'logical' member. The naming
physical/logical is used in other structures, like btrfs_bio_stripe or
scrub_page so I think this is kind of following the convention.
>
>
> > @@ -797,10 +797,10 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
> > */
> > for (i = 0; i < ipath->fspath->elem_cnt; ++i)
> > btrfs_warn_in_rcu(fs_info,
> > - "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
> > +"%s at logical %llu on dev %s, physical %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
> > swarn->errstr, swarn->logical,
> > rcu_str_deref(swarn->dev->name),
> > - (unsigned long long)swarn->sector,
> > + swarn->physical,
> > root, inum, offset,
> > min(isize - offset, (u64)PAGE_SIZE), nlink,
> > (char *)(unsigned long)ipath->fspath->val[i]);
> > @@ -813,7 +813,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
> > "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu: path resolving failed with ret=%d",
>
> This still says 'sector' here but was changed for the other print strings.
Right, I'll fix it.
> > swarn->errstr, swarn->logical,
> > rcu_str_deref(swarn->dev->name),
> > - (unsigned long long)swarn->sector,
> > + swarn->physical,
> > root, inum, offset, ret);
> >
> > free_ipath(ipath);
> > @@ -845,7 +845,7 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
> > if (!path)
> > return;
> >
> > - swarn.sector = (sblock->pagev[0]->physical) >> 9;
> > + swarn.physical = sblock->pagev[0]->physical;
>
> Dropping the '>> 9', so this is a semantic change in addition to the rename?
Yes, sector_t is traditionally in 512b units but we want "1b" where
possible. I'll update the changelog so it's clear.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] btrfs: scrub: get rid of sector_t
2017-10-06 12:29 ` [PATCH 1/3] btrfs: scrub: get rid of sector_t David Sterba
2017-10-06 15:22 ` Edmund Nadolski
@ 2017-10-07 0:15 ` Liu Bo
1 sibling, 0 replies; 9+ messages in thread
From: Liu Bo @ 2017-10-07 0:15 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Fri, Oct 06, 2017 at 02:29:56PM +0200, David Sterba wrote:
> The use of sector_t is not necessry, it's just for a warning. Switch to
> u64 and rename the variable. The messages are adjusted as well.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/scrub.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index e3f6c49e5c4d..34ea2e7048c2 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -231,7 +231,7 @@ struct scrub_warning {
> struct btrfs_path *path;
> u64 extent_item_size;
> const char *errstr;
> - sector_t sector;
> + u64 physical;
> u64 logical;
> struct btrfs_device *dev;
> };
> @@ -797,10 +797,10 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
> */
> for (i = 0; i < ipath->fspath->elem_cnt; ++i)
> btrfs_warn_in_rcu(fs_info,
> - "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
> +"%s at logical %llu on dev %s, physical %llu, root %llu, inode %llu, offset %llu, length %llu, links %u (path: %s)",
> swarn->errstr, swarn->logical,
> rcu_str_deref(swarn->dev->name),
> - (unsigned long long)swarn->sector,
> + swarn->physical,
> root, inum, offset,
> min(isize - offset, (u64)PAGE_SIZE), nlink,
> (char *)(unsigned long)ipath->fspath->val[i]);
> @@ -813,7 +813,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
> "%s at logical %llu on dev %s, sector %llu, root %llu, inode %llu, offset %llu: path resolving failed with ret=%d",
> swarn->errstr, swarn->logical,
> rcu_str_deref(swarn->dev->name),
> - (unsigned long long)swarn->sector,
> + swarn->physical,
> root, inum, offset, ret);
>
> free_ipath(ipath);
> @@ -845,7 +845,7 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
> if (!path)
> return;
>
> - swarn.sector = (sblock->pagev[0]->physical) >> 9;
> + swarn.physical = sblock->pagev[0]->physical;
> swarn.logical = sblock->pagev[0]->logical;
> swarn.errstr = errstr;
> swarn.dev = NULL;
> @@ -868,10 +868,10 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
> item_size, &ref_root,
> &ref_level);
> btrfs_warn_in_rcu(fs_info,
> - "%s at logical %llu on dev %s, sector %llu: metadata %s (level %d) in tree %llu",
> +"%s at logical %llu on dev %s, physical %llu: metadata %s (level %d) in tree %llu",
> errstr, swarn.logical,
> rcu_str_deref(dev->name),
> - (unsigned long long)swarn.sector,
> + swarn.physical,
> ref_level ? "node" : "leaf",
> ret < 0 ? -1 : ref_level,
> ret < 0 ? -1 : ref_root);
I've got used to ambiguous physical/logical, so not a problem for me.
Looks good to me.
Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
-liubo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] btrfs: rename page offset parameter in submit_extent_page
2017-10-06 12:29 [PATCH 0/3] Cleanup sector_t David Sterba
2017-10-06 12:29 ` [PATCH 1/3] btrfs: scrub: get rid of sector_t David Sterba
@ 2017-10-06 12:29 ` David Sterba
2017-10-07 0:11 ` Liu Bo
2017-10-06 12:30 ` [PATCH 3/3] btrfs: get rid of sector_t and use u64 offset " David Sterba
2 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2017-10-06 12:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: bo.li.liu, David Sterba
We're going to remove sector_t and will use 'offset', so this patch
frees the name.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/extent_io.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 12ab19a4b93e..ea9ab934794b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2762,7 +2762,7 @@ static int merge_bio(struct extent_io_tree *tree, struct page *page,
static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
struct writeback_control *wbc,
struct page *page, sector_t sector,
- size_t size, unsigned long offset,
+ size_t size, unsigned long pg_offset,
struct block_device *bdev,
struct bio **bio_ret,
bio_end_io_t end_io_func,
@@ -2786,8 +2786,8 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
if (prev_bio_flags != bio_flags || !contig ||
force_bio_submit ||
- merge_bio(tree, page, offset, page_size, bio, bio_flags) ||
- bio_add_page(bio, page, page_size, offset) < page_size) {
+ merge_bio(tree, page, pg_offset, page_size, bio, bio_flags) ||
+ bio_add_page(bio, page, page_size, pg_offset) < page_size) {
ret = submit_one_bio(bio, mirror_num, prev_bio_flags);
if (ret < 0) {
*bio_ret = NULL;
@@ -2802,7 +2802,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
}
bio = btrfs_bio_alloc(bdev, sector << 9);
- bio_add_page(bio, page, page_size, offset);
+ bio_add_page(bio, page, page_size, pg_offset);
bio->bi_end_io = end_io_func;
bio->bi_private = tree;
bio->bi_write_hint = page->mapping->host->i_write_hint;
--
2.14.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] btrfs: rename page offset parameter in submit_extent_page
2017-10-06 12:29 ` [PATCH 2/3] btrfs: rename page offset parameter in submit_extent_page David Sterba
@ 2017-10-07 0:11 ` Liu Bo
0 siblings, 0 replies; 9+ messages in thread
From: Liu Bo @ 2017-10-07 0:11 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Fri, Oct 06, 2017 at 02:29:58PM +0200, David Sterba wrote:
> We're going to remove sector_t and will use 'offset', so this patch
> frees the name.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/extent_io.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 12ab19a4b93e..ea9ab934794b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2762,7 +2762,7 @@ static int merge_bio(struct extent_io_tree *tree, struct page *page,
> static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
> struct writeback_control *wbc,
> struct page *page, sector_t sector,
> - size_t size, unsigned long offset,
> + size_t size, unsigned long pg_offset,
> struct block_device *bdev,
> struct bio **bio_ret,
> bio_end_io_t end_io_func,
> @@ -2786,8 +2786,8 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
>
> if (prev_bio_flags != bio_flags || !contig ||
> force_bio_submit ||
> - merge_bio(tree, page, offset, page_size, bio, bio_flags) ||
> - bio_add_page(bio, page, page_size, offset) < page_size) {
> + merge_bio(tree, page, pg_offset, page_size, bio, bio_flags) ||
> + bio_add_page(bio, page, page_size, pg_offset) < page_size) {
> ret = submit_one_bio(bio, mirror_num, prev_bio_flags);
> if (ret < 0) {
> *bio_ret = NULL;
> @@ -2802,7 +2802,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
> }
>
> bio = btrfs_bio_alloc(bdev, sector << 9);
> - bio_add_page(bio, page, page_size, offset);
> + bio_add_page(bio, page, page_size, pg_offset);
> bio->bi_end_io = end_io_func;
> bio->bi_private = tree;
> bio->bi_write_hint = page->mapping->host->i_write_hint;
Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
-liubo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] btrfs: get rid of sector_t and use u64 offset in submit_extent_page
2017-10-06 12:29 [PATCH 0/3] Cleanup sector_t David Sterba
2017-10-06 12:29 ` [PATCH 1/3] btrfs: scrub: get rid of sector_t David Sterba
2017-10-06 12:29 ` [PATCH 2/3] btrfs: rename page offset parameter in submit_extent_page David Sterba
@ 2017-10-06 12:30 ` David Sterba
2017-10-07 0:21 ` Liu Bo
2 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2017-10-06 12:30 UTC (permalink / raw)
To: linux-btrfs; +Cc: bo.li.liu, David Sterba
The use of sector_t in the callchain of submit_extent_page is not
necessary, we'll pass plain u64 and avoid any con(tro)versions of sector_t.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/extent_io.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index ea9ab934794b..8d0834ee792b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2761,7 +2761,7 @@ static int merge_bio(struct extent_io_tree *tree, struct page *page,
*/
static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
struct writeback_control *wbc,
- struct page *page, sector_t sector,
+ struct page *page, u64 offset,
size_t size, unsigned long pg_offset,
struct block_device *bdev,
struct bio **bio_ret,
@@ -2776,6 +2776,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
int contig = 0;
int old_compressed = prev_bio_flags & EXTENT_BIO_COMPRESSED;
size_t page_size = min_t(size_t, size, PAGE_SIZE);
+ sector_t sector = offset >> 9;
if (bio_ret && *bio_ret) {
bio = *bio_ret;
@@ -2801,7 +2802,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
}
}
- bio = btrfs_bio_alloc(bdev, sector << 9);
+ bio = btrfs_bio_alloc(bdev, offset);
bio_add_page(bio, page, page_size, pg_offset);
bio->bi_end_io = end_io_func;
bio->bi_private = tree;
@@ -2892,7 +2893,6 @@ static int __do_readpage(struct extent_io_tree *tree,
u64 last_byte = i_size_read(inode);
u64 block_start;
u64 cur_end;
- sector_t sector;
struct extent_map *em;
struct block_device *bdev;
int ret = 0;
@@ -2928,6 +2928,7 @@ static int __do_readpage(struct extent_io_tree *tree,
}
while (cur <= end) {
bool force_bio_submit = false;
+ u64 offset;
if (cur >= last_byte) {
char *userpage;
@@ -2967,9 +2968,9 @@ static int __do_readpage(struct extent_io_tree *tree,
iosize = ALIGN(iosize, blocksize);
if (this_bio_flag & EXTENT_BIO_COMPRESSED) {
disk_io_size = em->block_len;
- sector = em->block_start >> 9;
+ offset = em->block_start;
} else {
- sector = (em->block_start + extent_offset) >> 9;
+ offset = em->block_start + extent_offset;
disk_io_size = iosize;
}
bdev = em->bdev;
@@ -3062,8 +3063,8 @@ static int __do_readpage(struct extent_io_tree *tree,
}
ret = submit_extent_page(REQ_OP_READ | read_flags, tree, NULL,
- page, sector, disk_io_size, pg_offset,
- bdev, bio,
+ page, offset, disk_io_size,
+ pg_offset, bdev, bio,
end_bio_extent_readpage, mirror_num,
*bio_flags,
this_bio_flag,
@@ -3324,7 +3325,6 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
u64 extent_offset;
u64 block_start;
u64 iosize;
- sector_t sector;
struct extent_map *em;
struct block_device *bdev;
size_t pg_offset = 0;
@@ -3367,6 +3367,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
while (cur <= end) {
u64 em_end;
+ u64 offset;
if (cur >= i_size) {
if (tree->ops && tree->ops->writepage_end_io_hook)
@@ -3388,7 +3389,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
BUG_ON(end < cur);
iosize = min(em_end - cur, end - cur + 1);
iosize = ALIGN(iosize, blocksize);
- sector = (em->block_start + extent_offset) >> 9;
+ offset = em->block_start + extent_offset;
bdev = em->bdev;
block_start = em->block_start;
compressed = test_bit(EXTENT_FLAG_COMPRESSED, &em->flags);
@@ -3431,7 +3432,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
}
ret = submit_extent_page(REQ_OP_WRITE | write_flags, tree, wbc,
- page, sector, iosize, pg_offset,
+ page, offset, iosize, pg_offset,
bdev, &epd->bio,
end_bio_extent_writepage,
0, 0, 0, false);
@@ -3748,7 +3749,7 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
clear_page_dirty_for_io(p);
set_page_writeback(p);
ret = submit_extent_page(REQ_OP_WRITE | write_flags, tree, wbc,
- p, offset >> 9, PAGE_SIZE, 0, bdev,
+ p, offset, PAGE_SIZE, 0, bdev,
&epd->bio,
end_bio_extent_buffer_writepage,
0, epd->bio_flags, bio_flags, false);
--
2.14.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] btrfs: get rid of sector_t and use u64 offset in submit_extent_page
2017-10-06 12:30 ` [PATCH 3/3] btrfs: get rid of sector_t and use u64 offset " David Sterba
@ 2017-10-07 0:21 ` Liu Bo
0 siblings, 0 replies; 9+ messages in thread
From: Liu Bo @ 2017-10-07 0:21 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Fri, Oct 06, 2017 at 02:30:00PM +0200, David Sterba wrote:
> The use of sector_t in the callchain of submit_extent_page is not
> necessary, we'll pass plain u64 and avoid any con(tro)versions of sector_t.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/extent_io.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index ea9ab934794b..8d0834ee792b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2761,7 +2761,7 @@ static int merge_bio(struct extent_io_tree *tree, struct page *page,
> */
> static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
> struct writeback_control *wbc,
> - struct page *page, sector_t sector,
> + struct page *page, u64 offset,
> size_t size, unsigned long pg_offset,
> struct block_device *bdev,
> struct bio **bio_ret,
> @@ -2776,6 +2776,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
> int contig = 0;
> int old_compressed = prev_bio_flags & EXTENT_BIO_COMPRESSED;
> size_t page_size = min_t(size_t, size, PAGE_SIZE);
> + sector_t sector = offset >> 9;
>
> if (bio_ret && *bio_ret) {
> bio = *bio_ret;
> @@ -2801,7 +2802,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
> }
> }
>
> - bio = btrfs_bio_alloc(bdev, sector << 9);
> + bio = btrfs_bio_alloc(bdev, offset);
> bio_add_page(bio, page, page_size, pg_offset);
> bio->bi_end_io = end_io_func;
> bio->bi_private = tree;
> @@ -2892,7 +2893,6 @@ static int __do_readpage(struct extent_io_tree *tree,
> u64 last_byte = i_size_read(inode);
> u64 block_start;
> u64 cur_end;
> - sector_t sector;
> struct extent_map *em;
> struct block_device *bdev;
> int ret = 0;
> @@ -2928,6 +2928,7 @@ static int __do_readpage(struct extent_io_tree *tree,
> }
> while (cur <= end) {
> bool force_bio_submit = false;
> + u64 offset;
>
> if (cur >= last_byte) {
> char *userpage;
> @@ -2967,9 +2968,9 @@ static int __do_readpage(struct extent_io_tree *tree,
> iosize = ALIGN(iosize, blocksize);
> if (this_bio_flag & EXTENT_BIO_COMPRESSED) {
> disk_io_size = em->block_len;
> - sector = em->block_start >> 9;
> + offset = em->block_start;
> } else {
> - sector = (em->block_start + extent_offset) >> 9;
> + offset = em->block_start + extent_offset;
> disk_io_size = iosize;
> }
> bdev = em->bdev;
> @@ -3062,8 +3063,8 @@ static int __do_readpage(struct extent_io_tree *tree,
> }
>
> ret = submit_extent_page(REQ_OP_READ | read_flags, tree, NULL,
> - page, sector, disk_io_size, pg_offset,
> - bdev, bio,
> + page, offset, disk_io_size,
> + pg_offset, bdev, bio,
> end_bio_extent_readpage, mirror_num,
> *bio_flags,
> this_bio_flag,
> @@ -3324,7 +3325,6 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
> u64 extent_offset;
> u64 block_start;
> u64 iosize;
> - sector_t sector;
> struct extent_map *em;
> struct block_device *bdev;
> size_t pg_offset = 0;
> @@ -3367,6 +3367,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
>
> while (cur <= end) {
> u64 em_end;
> + u64 offset;
>
> if (cur >= i_size) {
> if (tree->ops && tree->ops->writepage_end_io_hook)
> @@ -3388,7 +3389,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
> BUG_ON(end < cur);
> iosize = min(em_end - cur, end - cur + 1);
> iosize = ALIGN(iosize, blocksize);
> - sector = (em->block_start + extent_offset) >> 9;
> + offset = em->block_start + extent_offset;
> bdev = em->bdev;
> block_start = em->block_start;
> compressed = test_bit(EXTENT_FLAG_COMPRESSED, &em->flags);
> @@ -3431,7 +3432,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
> }
>
> ret = submit_extent_page(REQ_OP_WRITE | write_flags, tree, wbc,
> - page, sector, iosize, pg_offset,
> + page, offset, iosize, pg_offset,
> bdev, &epd->bio,
> end_bio_extent_writepage,
> 0, 0, 0, false);
> @@ -3748,7 +3749,7 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
> clear_page_dirty_for_io(p);
> set_page_writeback(p);
> ret = submit_extent_page(REQ_OP_WRITE | write_flags, tree, wbc,
> - p, offset >> 9, PAGE_SIZE, 0, bdev,
> + p, offset, PAGE_SIZE, 0, bdev,
> &epd->bio,
> end_bio_extent_buffer_writepage,
> 0, epd->bio_flags, bio_flags, false);
You can have
Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
-liubo
^ permalink raw reply [flat|nested] 9+ messages in thread