| Message ID | 1479300736-9724-7-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 <hch@lst.de> > --- > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"
>> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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".
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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;
Use the bvec offset and len members to prepare for multipage bvecs. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/compression.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)