[PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-16 Thread Christoph Hellwig
Use the bvec offset and len members to prepare for multipage bvecs.

Signed-off-by: Christoph Hellwig 
---
 fs/btrfs/compression.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 8618ac3..27e9feb 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -445,6 +445,13 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 
start,
return 0;
 }
 
+static u64 bio_end_offset(struct bio *bio)
+{
+   struct bio_vec *last = &bio->bi_io_vec[bio->bi_vcnt - 1];
+
+   return page_offset(last->bv_page) + last->bv_len - last->bv_offset;
+}
+
 static noinline int add_ra_bio_pages(struct inode *inode,
 u64 compressed_end,
 struct compressed_bio *cb)
@@ -463,8 +470,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
u64 end;
int misses = 0;
 
-   page = cb->orig_bio->bi_io_vec[cb->orig_bio->bi_vcnt - 1].bv_page;
-   last_offset = (page_offset(page) + PAGE_SIZE);
+   last_offset = bio_end_offset(cb->orig_bio);
em_tree = &BTRFS_I(inode)->extent_tree;
tree = &BTRFS_I(inode)->io_tree;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-18 Thread Omar Sandoval
On Wed, Nov 16, 2016 at 01:52:13PM +0100, Christoph Hellwig wrote:
> Use the bvec offset and len members to prepare for multipage bvecs.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  fs/btrfs/compression.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 8618ac3..27e9feb 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -445,6 +445,13 @@ int btrfs_submit_compressed_write(struct inode *inode, 
> u64 start,
>   return 0;
>  }
>  
> +static u64 bio_end_offset(struct bio *bio)
> +{
> + struct bio_vec *last = &bio->bi_io_vec[bio->bi_vcnt - 1];
> +
> + return page_offset(last->bv_page) + last->bv_len - last->bv_offset;

Why is this minus bv_offset and not plus? Am I misunderstanding
bv_offset?

> +}
> +
>  static noinline int add_ra_bio_pages(struct inode *inode,
>u64 compressed_end,
>struct compressed_bio *cb)
> @@ -463,8 +470,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>   u64 end;
>   int misses = 0;
>  
> - page = cb->orig_bio->bi_io_vec[cb->orig_bio->bi_vcnt - 1].bv_page;
> - last_offset = (page_offset(page) + PAGE_SIZE);
> + last_offset = bio_end_offset(cb->orig_bio);
>   em_tree = &BTRFS_I(inode)->extent_tree;
>   tree = &BTRFS_I(inode)->io_tree;
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-22 Thread Christoph Hellwig
On Fri, Nov 18, 2016 at 12:04:38PM -0800, Omar Sandoval wrote:
> > +static u64 bio_end_offset(struct bio *bio)
> > +{
> > +   struct bio_vec *last = &bio->bi_io_vec[bio->bi_vcnt - 1];
> > +
> > +   return page_offset(last->bv_page) + last->bv_len - last->bv_offset;
> 
> Why is this minus bv_offset and not plus? Am I misunderstanding
> bv_offset?

This should be a plus, thanks.

Can anyone help me on how to get test coverage for the compression
code?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-22 Thread Omar Sandoval
On Tue, Nov 22, 2016 at 10:42:40AM +0100, Christoph Hellwig wrote:
> On Fri, Nov 18, 2016 at 12:04:38PM -0800, Omar Sandoval wrote:
> > > +static u64 bio_end_offset(struct bio *bio)
> > > +{
> > > + struct bio_vec *last = &bio->bi_io_vec[bio->bi_vcnt - 1];
> > > +
> > > + return page_offset(last->bv_page) + last->bv_len - last->bv_offset;
> > 
> > Why is this minus bv_offset and not plus? Am I misunderstanding
> > bv_offset?
> 
> This should be a plus, thanks.
> 
> Can anyone help me on how to get test coverage for the compression
> code?

I'm not surprised xfstests missed this one since it's just readahead.
You might be able to get better coverage with

export MOUNT_OPTS="-o compress-force"

-- 
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-22 Thread Anand Jain




Can anyone help me on how to get test coverage for the compression
code?


I'm not surprised xfstests missed this one since it's just readahead.
You might be able to get better coverage with

export MOUNT_OPTS="-o compress-force"


 And where the data is /dev/urandom the btrfs compression will
 bail out, so xfstest cases which uses /dev/urandom won't test
 the compression code.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-22 Thread Omar Sandoval
On Wed, Nov 23, 2016 at 12:21:41PM +0800, Anand Jain wrote:
> 
> 
> > > Can anyone help me on how to get test coverage for the compression
> > > code?
> > 
> > I'm not surprised xfstests missed this one since it's just readahead.
> > You might be able to get better coverage with
> > 
> > export MOUNT_OPTS="-o compress-force"
> 
>  And where the data is /dev/urandom the btrfs compression will
>  bail out, so xfstest cases which uses /dev/urandom won't test
>  the compression code.

Isn't that just with "-o compress"? That's why I recommended "-o
compress-force".

-- 
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-22 Thread Anand Jain



On 11/23/16 12:21, Omar Sandoval wrote:

On Wed, Nov 23, 2016 at 12:21:41PM +0800, Anand Jain wrote:




Can anyone help me on how to get test coverage for the compression
code?


I'm not surprised xfstests missed this one since it's just readahead.
You might be able to get better coverage with

export MOUNT_OPTS="-o compress-force"


 And where the data is /dev/urandom the btrfs compression will
 bail out, so xfstest cases which uses /dev/urandom won't test
 the compression code.


Isn't that just with "-o compress"? That's why I recommended "-o
compress-force".


 Nope. compress-force doesn't enforce compress even if the data
 isn't compressible. I am not sure if its a bug, but its been
 like that.

 The difference between compress and compress-force is that
 compress-force will never give up and compress will give up
 compress by setting nocompress flag if the first extent is
 not compressible.

Thanks, Anand
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] btrfs: calculate end of bio offset properly

2016-11-22 Thread Omar Sandoval
On Wed, Nov 23, 2016 at 12:32:26PM +0800, Anand Jain wrote:
> 
> 
> On 11/23/16 12:21, Omar Sandoval wrote:
> > On Wed, Nov 23, 2016 at 12:21:41PM +0800, Anand Jain wrote:
> > > 
> > > 
> > > > > Can anyone help me on how to get test coverage for the compression
> > > > > code?
> > > > 
> > > > I'm not surprised xfstests missed this one since it's just readahead.
> > > > You might be able to get better coverage with
> > > > 
> > > > export MOUNT_OPTS="-o compress-force"
> > > 
> > >  And where the data is /dev/urandom the btrfs compression will
> > >  bail out, so xfstest cases which uses /dev/urandom won't test
> > >  the compression code.
> > 
> > Isn't that just with "-o compress"? That's why I recommended "-o
> > compress-force".
> 
>  Nope. compress-force doesn't enforce compress even if the data
>  isn't compressible. I am not sure if its a bug, but its been
>  like that.
> 
>  The difference between compress and compress-force is that
>  compress-force will never give up and compress will give up
>  compress by setting nocompress flag if the first extent is
>  not compressible.
> 
> Thanks, Anand

Huh, you're right. The wording in man 5 btrfs could use some
clarification.

-- 
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html