diff mbox

btrfs compression: merge inflate and deflate z_streams

Message ID 1404142330-1621-1-git-send-email-sergey.senozhatsky@gmail.com (mailing list archive)
State Accepted
Headers show

Commit Message

Sergey Senozhatsky June 30, 2014, 3:32 p.m. UTC 
`struct workspace' used for zlib compression contains two zlib
z_stream-s: `def_strm' used in zlib_compress_pages(), and `inf_strm'
used in zlib_decompress/zlib_decompress_biovec(). None of these
functions use `inf_strm' and `def_strm' simultaniously, meaning that
for every compress/decompress operation we need only one z_stream
(out of two available).

`inf_strm' and `def_strm' are different in size of ->workspace. For
inflate stream we vmalloc() zlib_inflate_workspacesize() bytes, for
deflate stream - zlib_deflate_workspacesize() bytes. On my system zlib
returns the following workspace sizes, correspondingly: 42312 and 268104
(+ guard pages).

Keep only one `z_stream' in `struct workspace' and use it for both
compression and decompression. Hence, instead of vmalloc() of two
z_stream->worskpace-s, allocate only one of size:
	max(zlib_deflate_workspacesize(), zlib_inflate_workspacesize())

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 fs/btrfs/zlib.c | 138 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 68 insertions(+), 70 deletions(-)

Comments

David Sterba July 1, 2014, 2:44 p.m. UTC | #1 
On Tue, Jul 01, 2014 at 12:32:10AM +0900, Sergey Senozhatsky wrote:
> `struct workspace' used for zlib compression contains two zlib
> z_stream-s: `def_strm' used in zlib_compress_pages(), and `inf_strm'
> used in zlib_decompress/zlib_decompress_biovec(). None of these
> functions use `inf_strm' and `def_strm' simultaniously, meaning that
> for every compress/decompress operation we need only one z_stream
> (out of two available).
> 
> `inf_strm' and `def_strm' are different in size of ->workspace. For
> inflate stream we vmalloc() zlib_inflate_workspacesize() bytes, for
> deflate stream - zlib_deflate_workspacesize() bytes. On my system zlib
> returns the following workspace sizes, correspondingly: 42312 and 268104
> (+ guard pages).
> 
> Keep only one `z_stream' in `struct workspace' and use it for both
> compression and decompression. Hence, instead of vmalloc() of two
> z_stream->worskpace-s, allocate only one of size:
> 	max(zlib_deflate_workspacesize(), zlib_inflate_workspacesize())
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Reviewed-by: David Sterba <dsterba@suse.cz>
--
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
Sergey Senozhatsky July 7, 2014, 2:29 p.m. UTC | #2 
On (07/01/14 16:44), David Sterba wrote:
> On Tue, Jul 01, 2014 at 12:32:10AM +0900, Sergey Senozhatsky wrote:
> > `struct workspace' used for zlib compression contains two zlib
> > z_stream-s: `def_strm' used in zlib_compress_pages(), and `inf_strm'
> > used in zlib_decompress/zlib_decompress_biovec(). None of these
> > functions use `inf_strm' and `def_strm' simultaniously, meaning that
> > for every compress/decompress operation we need only one z_stream
> > (out of two available).
> > 
> > `inf_strm' and `def_strm' are different in size of ->workspace. For
> > inflate stream we vmalloc() zlib_inflate_workspacesize() bytes, for
> > deflate stream - zlib_deflate_workspacesize() bytes. On my system zlib
> > returns the following workspace sizes, correspondingly: 42312 and 268104
> > (+ guard pages).
> > 
> > Keep only one `z_stream' in `struct workspace' and use it for both
> > compression and decompression. Hence, instead of vmalloc() of two
> > z_stream->worskpace-s, allocate only one of size:
> > 	max(zlib_deflate_workspacesize(), zlib_inflate_workspacesize())
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> Reviewed-by: David Sterba <dsterba@suse.cz>
> 

Hello,

the patch does not apply against linux-next rc4-20140707 due to 130d5b415a091e.
unhappy hunk is:

+		if (workspace->strm.total_in > 8192 &&
+		    workspace->strm.total_in <
+		    workspace->strm.total_out) {
 			ret = -EIO;

now it should be:
+		if (workspace->strm.total_in > 8192 &&
+		    workspace->strm.total_in <
+		    workspace->strm.total_out) {
 			ret = -E2BIG;


I'll rebase and resend.

	-ss
--
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
diff mbox

Patch

diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 4f19631..280123c 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -33,8 +33,7 @@ 
 #include "compression.h"
 
 struct workspace {
-	z_stream inf_strm;
-	z_stream def_strm;
+	z_stream strm;
 	char *buf;
 	struct list_head list;
 };
@@ -43,8 +42,7 @@  static void zlib_free_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 
-	vfree(workspace->def_strm.workspace);
-	vfree(workspace->inf_strm.workspace);
+	vfree(workspace->strm.workspace);
 	kfree(workspace->buf);
 	kfree(workspace);
 }
@@ -52,17 +50,17 @@  static void zlib_free_workspace(struct list_head *ws)
 static struct list_head *zlib_alloc_workspace(void)
 {
 	struct workspace *workspace;
+	int workspacesize;
 
 	workspace = kzalloc(sizeof(*workspace), GFP_NOFS);
 	if (!workspace)
 		return ERR_PTR(-ENOMEM);
 
-	workspace->def_strm.workspace = vmalloc(zlib_deflate_workspacesize(
-						MAX_WBITS, MAX_MEM_LEVEL));
-	workspace->inf_strm.workspace = vmalloc(zlib_inflate_workspacesize());
+	workspacesize = max(zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL),
+			zlib_inflate_workspacesize());
+	workspace->strm.workspace = vmalloc(workspacesize);
 	workspace->buf = kmalloc(PAGE_CACHE_SIZE, GFP_NOFS);
-	if (!workspace->def_strm.workspace ||
-	    !workspace->inf_strm.workspace || !workspace->buf)
+	if (!workspace->strm.workspace || !workspace->buf)
 		goto fail;
 
 	INIT_LIST_HEAD(&workspace->list);
@@ -96,14 +94,14 @@  static int zlib_compress_pages(struct list_head *ws,
 	*total_out = 0;
 	*total_in = 0;
 
-	if (Z_OK != zlib_deflateInit(&workspace->def_strm, 3)) {
+	if (Z_OK != zlib_deflateInit(&workspace->strm, 3)) {
 		printk(KERN_WARNING "BTRFS: deflateInit failed\n");
 		ret = -EIO;
 		goto out;
 	}
 
-	workspace->def_strm.total_in = 0;
-	workspace->def_strm.total_out = 0;
+	workspace->strm.total_in = 0;
+	workspace->strm.total_out = 0;
 
 	in_page = find_get_page(mapping, start >> PAGE_CACHE_SHIFT);
 	data_in = kmap(in_page);
@@ -117,25 +115,25 @@  static int zlib_compress_pages(struct list_head *ws,
 	pages[0] = out_page;
 	nr_pages = 1;
 
-	workspace->def_strm.next_in = data_in;
-	workspace->def_strm.next_out = cpage_out;
-	workspace->def_strm.avail_out = PAGE_CACHE_SIZE;
-	workspace->def_strm.avail_in = min(len, PAGE_CACHE_SIZE);
+	workspace->strm.next_in = data_in;
+	workspace->strm.next_out = cpage_out;
+	workspace->strm.avail_out = PAGE_CACHE_SIZE;
+	workspace->strm.avail_in = min(len, PAGE_CACHE_SIZE);
 
-	while (workspace->def_strm.total_in < len) {
-		ret = zlib_deflate(&workspace->def_strm, Z_SYNC_FLUSH);
+	while (workspace->strm.total_in < len) {
+		ret = zlib_deflate(&workspace->strm, Z_SYNC_FLUSH);
 		if (ret != Z_OK) {
 			printk(KERN_DEBUG "BTRFS: deflate in loop returned %d\n",
 			       ret);
-			zlib_deflateEnd(&workspace->def_strm);
+			zlib_deflateEnd(&workspace->strm);
 			ret = -EIO;
 			goto out;
 		}
 
 		/* we're making it bigger, give up */
-		if (workspace->def_strm.total_in > 8192 &&
-		    workspace->def_strm.total_in <
-		    workspace->def_strm.total_out) {
+		if (workspace->strm.total_in > 8192 &&
+		    workspace->strm.total_in <
+		    workspace->strm.total_out) {
 			ret = -EIO;
 			goto out;
 		}
@@ -143,7 +141,7 @@  static int zlib_compress_pages(struct list_head *ws,
 		 * before the total_in so we will pull in a new page for
 		 * the stream end if required
 		 */
-		if (workspace->def_strm.avail_out == 0) {
+		if (workspace->strm.avail_out == 0) {
 			kunmap(out_page);
 			if (nr_pages == nr_dest_pages) {
 				out_page = NULL;
@@ -158,19 +156,19 @@  static int zlib_compress_pages(struct list_head *ws,
 			cpage_out = kmap(out_page);
 			pages[nr_pages] = out_page;
 			nr_pages++;
-			workspace->def_strm.avail_out = PAGE_CACHE_SIZE;
-			workspace->def_strm.next_out = cpage_out;
+			workspace->strm.avail_out = PAGE_CACHE_SIZE;
+			workspace->strm.next_out = cpage_out;
 		}
 		/* we're all done */
-		if (workspace->def_strm.total_in >= len)
+		if (workspace->strm.total_in >= len)
 			break;
 
 		/* we've read in a full page, get a new one */
-		if (workspace->def_strm.avail_in == 0) {
-			if (workspace->def_strm.total_out > max_out)
+		if (workspace->strm.avail_in == 0) {
+			if (workspace->strm.total_out > max_out)
 				break;
 
-			bytes_left = len - workspace->def_strm.total_in;
+			bytes_left = len - workspace->strm.total_in;
 			kunmap(in_page);
 			page_cache_release(in_page);
 
@@ -178,28 +176,28 @@  static int zlib_compress_pages(struct list_head *ws,
 			in_page = find_get_page(mapping,
 						start >> PAGE_CACHE_SHIFT);
 			data_in = kmap(in_page);
-			workspace->def_strm.avail_in = min(bytes_left,
+			workspace->strm.avail_in = min(bytes_left,
 							   PAGE_CACHE_SIZE);
-			workspace->def_strm.next_in = data_in;
+			workspace->strm.next_in = data_in;
 		}
 	}
-	workspace->def_strm.avail_in = 0;
-	ret = zlib_deflate(&workspace->def_strm, Z_FINISH);
-	zlib_deflateEnd(&workspace->def_strm);
+	workspace->strm.avail_in = 0;
+	ret = zlib_deflate(&workspace->strm, Z_FINISH);
+	zlib_deflateEnd(&workspace->strm);
 
 	if (ret != Z_STREAM_END) {
 		ret = -EIO;
 		goto out;
 	}
 
-	if (workspace->def_strm.total_out >= workspace->def_strm.total_in) {
+	if (workspace->strm.total_out >= workspace->strm.total_in) {
 		ret = -E2BIG;
 		goto out;
 	}
 
 	ret = 0;
-	*total_out = workspace->def_strm.total_out;
-	*total_in = workspace->def_strm.total_in;
+	*total_out = workspace->strm.total_out;
+	*total_in = workspace->strm.total_in;
 out:
 	*out_pages = nr_pages;
 	if (out_page)
@@ -231,13 +229,13 @@  static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in,
 	unsigned long pg_offset;
 
 	data_in = kmap(pages_in[page_in_index]);
-	workspace->inf_strm.next_in = data_in;
-	workspace->inf_strm.avail_in = min_t(size_t, srclen, PAGE_CACHE_SIZE);
-	workspace->inf_strm.total_in = 0;
+	workspace->strm.next_in = data_in;
+	workspace->strm.avail_in = min_t(size_t, srclen, PAGE_CACHE_SIZE);
+	workspace->strm.total_in = 0;
 
-	workspace->inf_strm.total_out = 0;
-	workspace->inf_strm.next_out = workspace->buf;
-	workspace->inf_strm.avail_out = PAGE_CACHE_SIZE;
+	workspace->strm.total_out = 0;
+	workspace->strm.next_out = workspace->buf;
+	workspace->strm.avail_out = PAGE_CACHE_SIZE;
 	pg_offset = 0;
 
 	/* If it's deflate, and it's got no preset dictionary, then
@@ -247,21 +245,21 @@  static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in,
 	    !(((data_in[0]<<8) + data_in[1]) % 31)) {
 
 		wbits = -((data_in[0] >> 4) + 8);
-		workspace->inf_strm.next_in += 2;
-		workspace->inf_strm.avail_in -= 2;
+		workspace->strm.next_in += 2;
+		workspace->strm.avail_in -= 2;
 	}
 
-	if (Z_OK != zlib_inflateInit2(&workspace->inf_strm, wbits)) {
+	if (Z_OK != zlib_inflateInit2(&workspace->strm, wbits)) {
 		printk(KERN_WARNING "BTRFS: inflateInit failed\n");
 		return -EIO;
 	}
-	while (workspace->inf_strm.total_in < srclen) {
-		ret = zlib_inflate(&workspace->inf_strm, Z_NO_FLUSH);
+	while (workspace->strm.total_in < srclen) {
+		ret = zlib_inflate(&workspace->strm, Z_NO_FLUSH);
 		if (ret != Z_OK && ret != Z_STREAM_END)
 			break;
 
 		buf_start = total_out;
-		total_out = workspace->inf_strm.total_out;
+		total_out = workspace->strm.total_out;
 
 		/* we didn't make progress in this inflate call, we're done */
 		if (buf_start == total_out)
@@ -276,10 +274,10 @@  static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in,
 			goto done;
 		}
 
-		workspace->inf_strm.next_out = workspace->buf;
-		workspace->inf_strm.avail_out = PAGE_CACHE_SIZE;
+		workspace->strm.next_out = workspace->buf;
+		workspace->strm.avail_out = PAGE_CACHE_SIZE;
 
-		if (workspace->inf_strm.avail_in == 0) {
+		if (workspace->strm.avail_in == 0) {
 			unsigned long tmp;
 			kunmap(pages_in[page_in_index]);
 			page_in_index++;
@@ -288,9 +286,9 @@  static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in,
 				break;
 			}
 			data_in = kmap(pages_in[page_in_index]);
-			workspace->inf_strm.next_in = data_in;
-			tmp = srclen - workspace->inf_strm.total_in;
-			workspace->inf_strm.avail_in = min(tmp,
+			workspace->strm.next_in = data_in;
+			tmp = srclen - workspace->strm.total_in;
+			workspace->strm.avail_in = min(tmp,
 							   PAGE_CACHE_SIZE);
 		}
 	}
@@ -299,7 +297,7 @@  static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in,
 	else
 		ret = 0;
 done:
-	zlib_inflateEnd(&workspace->inf_strm);
+	zlib_inflateEnd(&workspace->strm);
 	if (data_in)
 		kunmap(pages_in[page_in_index]);
 	return ret;
@@ -317,13 +315,13 @@  static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 	unsigned long total_out = 0;
 	char *kaddr;
 
-	workspace->inf_strm.next_in = data_in;
-	workspace->inf_strm.avail_in = srclen;
-	workspace->inf_strm.total_in = 0;
+	workspace->strm.next_in = data_in;
+	workspace->strm.avail_in = srclen;
+	workspace->strm.total_in = 0;
 
-	workspace->inf_strm.next_out = workspace->buf;
-	workspace->inf_strm.avail_out = PAGE_CACHE_SIZE;
-	workspace->inf_strm.total_out = 0;
+	workspace->strm.next_out = workspace->buf;
+	workspace->strm.avail_out = PAGE_CACHE_SIZE;
+	workspace->strm.total_out = 0;
 	/* If it's deflate, and it's got no preset dictionary, then
 	   we can tell zlib to skip the adler32 check. */
 	if (srclen > 2 && !(data_in[1] & PRESET_DICT) &&
@@ -331,11 +329,11 @@  static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 	    !(((data_in[0]<<8) + data_in[1]) % 31)) {
 
 		wbits = -((data_in[0] >> 4) + 8);
-		workspace->inf_strm.next_in += 2;
-		workspace->inf_strm.avail_in -= 2;
+		workspace->strm.next_in += 2;
+		workspace->strm.avail_in -= 2;
 	}
 
-	if (Z_OK != zlib_inflateInit2(&workspace->inf_strm, wbits)) {
+	if (Z_OK != zlib_inflateInit2(&workspace->strm, wbits)) {
 		printk(KERN_WARNING "BTRFS: inflateInit failed\n");
 		return -EIO;
 	}
@@ -346,12 +344,12 @@  static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 		unsigned long bytes;
 		unsigned long pg_offset = 0;
 
-		ret = zlib_inflate(&workspace->inf_strm, Z_NO_FLUSH);
+		ret = zlib_inflate(&workspace->strm, Z_NO_FLUSH);
 		if (ret != Z_OK && ret != Z_STREAM_END)
 			break;
 
 		buf_start = total_out;
-		total_out = workspace->inf_strm.total_out;
+		total_out = workspace->strm.total_out;
 
 		if (total_out == buf_start) {
 			ret = -EIO;
@@ -377,8 +375,8 @@  static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 		pg_offset += bytes;
 		bytes_left -= bytes;
 next:
-		workspace->inf_strm.next_out = workspace->buf;
-		workspace->inf_strm.avail_out = PAGE_CACHE_SIZE;
+		workspace->strm.next_out = workspace->buf;
+		workspace->strm.avail_out = PAGE_CACHE_SIZE;
 	}
 
 	if (ret != Z_STREAM_END && bytes_left != 0)
@@ -386,7 +384,7 @@  next:
 	else
 		ret = 0;
 
-	zlib_inflateEnd(&workspace->inf_strm);
+	zlib_inflateEnd(&workspace->strm);
 	return ret;
 }