[PATCH 6/9] btrfs: calculate end of bio offset properly
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
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
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
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
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
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
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
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