| Message ID | 20170520184039.31362-3-nefelim4ag@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sunday, May 21, 2017 12:10:39 AM IST Timofey Titovets wrote: > Btrfs already skip store of data where compression didn't free at least one byte. > So make logic better and make check that compression free at least one PAGE_SIZE, > because in another case it useless to store this data compressed > > Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> > --- > fs/btrfs/lzo.c | 5 ++++- > fs/btrfs/zlib.c | 3 ++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c > index bd0b0938..39678499 100644 > --- a/fs/btrfs/lzo.c > +++ b/fs/btrfs/lzo.c > @@ -229,8 +229,11 @@ static int lzo_compress_pages(struct list_head *ws, > in_len = min(bytes_left, PAGE_SIZE); > } > > - if (tot_out > tot_in) > + /* Compression must save at least one PAGE_SIZE */ > + if (tot_out + PAGE_SIZE > tot_in) { > + ret = -E2BIG; > goto out; > + } Apologies for the delayed response. I am not really sure if compression code must save atleast one sectorsize worth of space. But if other developers agree to it, then the above 'if' condition can be replaced with, u32 sectorsize = btrfs_inode_sectorsize(mapping->host); ... ... if (tot_out + sectorsize > tot_in) { > > /* store the size of all chunks of compressed data */ > cpage_out = kmap(pages[0]); > diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c > index 135b1082..11e117b5 100644 > --- a/fs/btrfs/zlib.c > +++ b/fs/btrfs/zlib.c > @@ -191,7 +191,8 @@ static int zlib_compress_pages(struct list_head *ws, > goto out; > } > > - if (workspace->strm.total_out >= workspace->strm.total_in) { > + /* Compression must save at least one PAGE_SIZE */ > + if (workspace->strm.total_out + PAGE_SIZE > workspace->strm.total_in) { > ret = -E2BIG; > goto out; > } > -- > 2.13.0 > -- > 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 > >
2017-05-25 15:51 GMT+03:00 Chandan Rajendra <chandan@linux.vnet.ibm.com>: ... > Apologies for the delayed response. > > I am not really sure if compression code must save atleast one sectorsize > worth of space. But if other developers agree to it, then the above > 'if' condition can be replaced with, > > u32 sectorsize = btrfs_inode_sectorsize(mapping->host); > ... > ... > > if (tot_out + sectorsize > tot_in) { > -- > chandan > Thanks a lot! This approach much simplier then i imagined, i will update patch set and resend. Thank you!
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c index bd0b0938..39678499 100644 --- a/fs/btrfs/lzo.c +++ b/fs/btrfs/lzo.c @@ -229,8 +229,11 @@ static int lzo_compress_pages(struct list_head *ws, in_len = min(bytes_left, PAGE_SIZE); } - if (tot_out > tot_in) + /* Compression must save at least one PAGE_SIZE */ + if (tot_out + PAGE_SIZE > tot_in) { + ret = -E2BIG; goto out; + } /* store the size of all chunks of compressed data */ cpage_out = kmap(pages[0]); diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c index 135b1082..11e117b5 100644 --- a/fs/btrfs/zlib.c +++ b/fs/btrfs/zlib.c @@ -191,7 +191,8 @@ static int zlib_compress_pages(struct list_head *ws, goto out; } - if (workspace->strm.total_out >= workspace->strm.total_in) { + /* Compression must save at least one PAGE_SIZE */ + if (workspace->strm.total_out + PAGE_SIZE > workspace->strm.total_in) { ret = -E2BIG; goto out; }
Btrfs already skip store of data where compression didn't free at least one byte. So make logic better and make check that compression free at least one PAGE_SIZE, because in another case it useless to store this data compressed Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/lzo.c | 5 ++++- fs/btrfs/zlib.c | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) -- 2.13.0 -- 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