All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Squashfs Whitelist and Compression Threshold
@ 2017-09-22 21:55 Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 1/5] Squashfs: remove the FILE_CACHE option Daniel Rosenberg
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Daniel Rosenberg @ 2017-09-22 21:55 UTC (permalink / raw)
  To: Phillip Lougher, linux-kernel; +Cc: adrien, Daniel Rosenberg

These patches contain several optimizations to Squashfs. ll_rw_block is
replaced with submit_bio. readpages is implemented to support asynchronous
readahead. Uncompressed file reads are optimized, no longer requiring the
entire block to be read if the block doesn't need to be decompressed, which
greatly improves random read speeds in uncompressed files. There is a separate
set of patches to the userspace tools adding a whitelist of files to not
compress, and an ability to set a compression threshold to avoid compressing
files where there isn't a significant advantage made by compressing.

Adrien Schildknecht (5):
  Squashfs: remove the FILE_CACHE option
  Squashfs: refactor page_actor
  Squashfs: replace buffer_head with BIO
  Squashfs: implement .readpages()
  Squashfs: optimize reading uncompressed data

 fs/squashfs/Kconfig          |  28 ---
 fs/squashfs/Makefile         |   3 +-
 fs/squashfs/block.c          | 547 ++++++++++++++++++++++++++++++++-----------
 fs/squashfs/cache.c          |  73 +++---
 fs/squashfs/decompressor.c   |  55 +++--
 fs/squashfs/file.c           | 137 ++++++++---
 fs/squashfs/file_cache.c     |  38 ---
 fs/squashfs/file_direct.c    | 240 +++++++++----------
 fs/squashfs/lz4_wrapper.c    |  29 +--
 fs/squashfs/lzo_wrapper.c    |  40 +---
 fs/squashfs/page_actor.c     | 175 +++++++++-----
 fs/squashfs/page_actor.h     |  84 +++----
 fs/squashfs/squashfs.h       |   9 +-
 fs/squashfs/squashfs_fs_sb.h |   2 +-
 fs/squashfs/super.c          |   7 +
 fs/squashfs/xz_wrapper.c     |  15 +-
 fs/squashfs/zlib_wrapper.c   |  14 +-
 17 files changed, 876 insertions(+), 620 deletions(-)
 delete mode 100644 fs/squashfs/file_cache.c

-- 
2.14.1.821.g8fa685d3b7-goog

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] Squashfs: remove the FILE_CACHE option
  2017-09-22 21:55 [PATCH 0/5] Squashfs Whitelist and Compression Threshold Daniel Rosenberg
@ 2017-09-22 21:55 ` Daniel Rosenberg
  2017-09-23  2:01   ` Phillip Lougher
  2017-09-22 21:55 ` [PATCH 2/5] Squashfs: refactor page_actor Daniel Rosenberg
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Daniel Rosenberg @ 2017-09-22 21:55 UTC (permalink / raw)
  To: Phillip Lougher, linux-kernel
  Cc: adrien, Adrien Schildknecht, Daniel Rosenberg

From: Adrien Schildknecht <adriens@google.com>

FILE_DIRECT is working fine and offers faster results and lower memory
footprint.

Removing FILE_CACHE makes our life easier because we don't have to
maintain 2 differents function that does the same thing.

Signed-off-by: Adrien Schildknecht <adriens@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/squashfs/Kconfig      | 28 ----------------------------
 fs/squashfs/Makefile     |  3 +--
 fs/squashfs/file_cache.c | 38 --------------------------------------
 fs/squashfs/page_actor.h | 42 +-----------------------------------------
 4 files changed, 2 insertions(+), 109 deletions(-)
 delete mode 100644 fs/squashfs/file_cache.c

diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
index 1adb3346b9d6..6c81bf620067 100644
--- a/fs/squashfs/Kconfig
+++ b/fs/squashfs/Kconfig
@@ -25,34 +25,6 @@ config SQUASHFS
 
 	  If unsure, say N.
 
-choice
-	prompt "File decompression options"
-	depends on SQUASHFS
-	help
-	  Squashfs now supports two options for decompressing file
-	  data.  Traditionally Squashfs has decompressed into an
-	  intermediate buffer and then memcopied it into the page cache.
-	  Squashfs now supports the ability to decompress directly into
-	  the page cache.
-
-	  If unsure, select "Decompress file data into an intermediate buffer"
-
-config SQUASHFS_FILE_CACHE
-	bool "Decompress file data into an intermediate buffer"
-	help
-	  Decompress file data into an intermediate buffer and then
-	  memcopy it into the page cache.
-
-config SQUASHFS_FILE_DIRECT
-	bool "Decompress files directly into the page cache"
-	help
-	  Directly decompress file data into the page cache.
-	  Doing so can significantly improve performance because
-	  it eliminates a memcpy and it also removes the lock contention
-	  on the single buffer.
-
-endchoice
-
 choice
 	prompt "Decompressor parallelisation options"
 	depends on SQUASHFS
diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 6655631c53ae..225330ab7723 100644
--- a/fs/squashfs/Makefile
+++ b/fs/squashfs/Makefile
@@ -5,8 +5,7 @@
 obj-$(CONFIG_SQUASHFS) += squashfs.o
 squashfs-y += block.o cache.o dir.o export.o file.o fragment.o id.o inode.o
 squashfs-y += namei.o super.o symlink.o decompressor.o
-squashfs-$(CONFIG_SQUASHFS_FILE_CACHE) += file_cache.o
-squashfs-$(CONFIG_SQUASHFS_FILE_DIRECT) += file_direct.o page_actor.o
+squashfs-y += file_direct.o page_actor.o
 squashfs-$(CONFIG_SQUASHFS_DECOMP_SINGLE) += decompressor_single.o
 squashfs-$(CONFIG_SQUASHFS_DECOMP_MULTI) += decompressor_multi.o
 squashfs-$(CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU) += decompressor_multi_percpu.o
diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
deleted file mode 100644
index f2310d2a2019..000000000000
--- a/fs/squashfs/file_cache.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Copyright (c) 2013
- * Phillip Lougher <phillip@squashfs.org.uk>
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- */
-
-#include <linux/fs.h>
-#include <linux/vfs.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/string.h>
-#include <linux/pagemap.h>
-#include <linux/mutex.h>
-
-#include "squashfs_fs.h"
-#include "squashfs_fs_sb.h"
-#include "squashfs_fs_i.h"
-#include "squashfs.h"
-
-/* Read separately compressed datablock and memcopy into page cache */
-int squashfs_readpage_block(struct page *page, u64 block, int bsize)
-{
-	struct inode *i = page->mapping->host;
-	struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
-		block, bsize);
-	int res = buffer->error;
-
-	if (res)
-		ERROR("Unable to read page, block %llx, size %x\n", block,
-			bsize);
-	else
-		squashfs_copy_cache(page, buffer, buffer->length, 0);
-
-	squashfs_cache_put(buffer);
-	return res;
-}
diff --git a/fs/squashfs/page_actor.h b/fs/squashfs/page_actor.h
index 98537eab27e2..d2df0544e0df 100644
--- a/fs/squashfs/page_actor.h
+++ b/fs/squashfs/page_actor.h
@@ -8,46 +8,6 @@
  * the COPYING file in the top-level directory.
  */
 
-#ifndef CONFIG_SQUASHFS_FILE_DIRECT
-struct squashfs_page_actor {
-	void	**page;
-	int	pages;
-	int	length;
-	int	next_page;
-};
-
-static inline struct squashfs_page_actor *squashfs_page_actor_init(void **page,
-	int pages, int length)
-{
-	struct squashfs_page_actor *actor = kmalloc(sizeof(*actor), GFP_KERNEL);
-
-	if (actor == NULL)
-		return NULL;
-
-	actor->length = length ? : pages * PAGE_SIZE;
-	actor->page = page;
-	actor->pages = pages;
-	actor->next_page = 0;
-	return actor;
-}
-
-static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
-{
-	actor->next_page = 1;
-	return actor->page[0];
-}
-
-static inline void *squashfs_next_page(struct squashfs_page_actor *actor)
-{
-	return actor->next_page == actor->pages ? NULL :
-		actor->page[actor->next_page++];
-}
-
-static inline void squashfs_finish_page(struct squashfs_page_actor *actor)
-{
-	/* empty */
-}
-#else
 struct squashfs_page_actor {
 	union {
 		void		**buffer;
@@ -77,5 +37,5 @@ static inline void squashfs_finish_page(struct squashfs_page_actor *actor)
 {
 	actor->squashfs_finish_page(actor);
 }
-#endif
+
 #endif
-- 
2.14.1.821.g8fa685d3b7-goog

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] Squashfs: refactor page_actor
  2017-09-22 21:55 [PATCH 0/5] Squashfs Whitelist and Compression Threshold Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 1/5] Squashfs: remove the FILE_CACHE option Daniel Rosenberg
@ 2017-09-22 21:55 ` Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 3/5] Squashfs: replace buffer_head with BIO Daniel Rosenberg
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel Rosenberg @ 2017-09-22 21:55 UTC (permalink / raw)
  To: Phillip Lougher, linux-kernel
  Cc: adrien, Adrien Schildknecht, Daniel Rosenberg

From: Adrien Schildknecht <adriens@google.com>

This patch essentially does 3 things:
  1/ Always use an array of page to store the data instead of a mix of
     buffers and pages.
  2/ It is now possible to have 'holes' in a page actor, i.e. NULL
     pages in the array.
     When reading a block (default 128K), squashfs tries to grab all
     the pages covering this block. If a single page is up-to-date or
     locked, it falls back to using an intermediate buffer to do the
     read and then copy the pages in the actor. Allowing holes in the
     page actor remove the need for this intermediate buffer.
  3/ Refactor the wrappers to share code that deals with page actors.

Signed-off-by: Adrien Schildknecht <adriens@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/squashfs/cache.c          |  73 +++++++-----------
 fs/squashfs/decompressor.c   |  55 +++++++-------
 fs/squashfs/file_direct.c    |   4 +-
 fs/squashfs/lz4_wrapper.c    |  29 +------
 fs/squashfs/lzo_wrapper.c    |  40 ++--------
 fs/squashfs/page_actor.c     | 175 ++++++++++++++++++++++++++++---------------
 fs/squashfs/page_actor.h     |  52 +++++++++----
 fs/squashfs/squashfs_fs_sb.h |   2 +-
 fs/squashfs/xz_wrapper.c     |  15 +++-
 fs/squashfs/zlib_wrapper.c   |  14 +++-
 10 files changed, 251 insertions(+), 208 deletions(-)

diff --git a/fs/squashfs/cache.c b/fs/squashfs/cache.c
index 23813c078cc9..05e42441d106 100644
--- a/fs/squashfs/cache.c
+++ b/fs/squashfs/cache.c
@@ -209,17 +209,14 @@ void squashfs_cache_put(struct squashfs_cache_entry *entry)
  */
 void squashfs_cache_delete(struct squashfs_cache *cache)
 {
-	int i, j;
+	int i;
 
 	if (cache == NULL)
 		return;
 
 	for (i = 0; i < cache->entries; i++) {
-		if (cache->entry[i].data) {
-			for (j = 0; j < cache->pages; j++)
-				kfree(cache->entry[i].data[j]);
-			kfree(cache->entry[i].data);
-		}
+		if (cache->entry[i].page)
+			free_page_array(cache->entry[i].page, cache->pages);
 		kfree(cache->entry[i].actor);
 	}
 
@@ -236,7 +233,7 @@ void squashfs_cache_delete(struct squashfs_cache *cache)
 struct squashfs_cache *squashfs_cache_init(char *name, int entries,
 	int block_size)
 {
-	int i, j;
+	int i;
 	struct squashfs_cache *cache = kzalloc(sizeof(*cache), GFP_KERNEL);
 
 	if (cache == NULL) {
@@ -268,22 +265,13 @@ struct squashfs_cache *squashfs_cache_init(char *name, int entries,
 		init_waitqueue_head(&cache->entry[i].wait_queue);
 		entry->cache = cache;
 		entry->block = SQUASHFS_INVALID_BLK;
-		entry->data = kcalloc(cache->pages, sizeof(void *), GFP_KERNEL);
-		if (entry->data == NULL) {
+		entry->page = alloc_page_array(cache->pages, GFP_KERNEL);
+		if (!entry->page) {
 			ERROR("Failed to allocate %s cache entry\n", name);
 			goto cleanup;
 		}
-
-		for (j = 0; j < cache->pages; j++) {
-			entry->data[j] = kmalloc(PAGE_SIZE, GFP_KERNEL);
-			if (entry->data[j] == NULL) {
-				ERROR("Failed to allocate %s buffer\n", name);
-				goto cleanup;
-			}
-		}
-
-		entry->actor = squashfs_page_actor_init(entry->data,
-						cache->pages, 0);
+		entry->actor = squashfs_page_actor_init(entry->page,
+			cache->pages, 0, NULL);
 		if (entry->actor == NULL) {
 			ERROR("Failed to allocate %s cache entry\n", name);
 			goto cleanup;
@@ -314,18 +302,20 @@ int squashfs_copy_data(void *buffer, struct squashfs_cache_entry *entry,
 		return min(length, entry->length - offset);
 
 	while (offset < entry->length) {
-		void *buff = entry->data[offset / PAGE_SIZE]
-				+ (offset % PAGE_SIZE);
+		void *buff = kmap_atomic(entry->page[offset / PAGE_SIZE])
+			     + (offset % PAGE_SIZE);
 		int bytes = min_t(int, entry->length - offset,
 				PAGE_SIZE - (offset % PAGE_SIZE));
 
 		if (bytes >= remaining) {
 			memcpy(buffer, buff, remaining);
+			kunmap_atomic(buff);
 			remaining = 0;
 			break;
 		}
 
 		memcpy(buffer, buff, bytes);
+		kunmap_atomic(buff);
 		buffer += bytes;
 		remaining -= bytes;
 		offset += bytes;
@@ -416,43 +406,38 @@ struct squashfs_cache_entry *squashfs_get_datablock(struct super_block *sb,
 void *squashfs_read_table(struct super_block *sb, u64 block, int length)
 {
 	int pages = (length + PAGE_SIZE - 1) >> PAGE_SHIFT;
-	int i, res;
-	void *table, *buffer, **data;
+	struct page **page;
+	void *buff;
+	int res;
 	struct squashfs_page_actor *actor;
 
-	table = buffer = kmalloc(length, GFP_KERNEL);
-	if (table == NULL)
+	page = alloc_page_array(pages, GFP_KERNEL);
+	if (!page)
 		return ERR_PTR(-ENOMEM);
 
-	data = kcalloc(pages, sizeof(void *), GFP_KERNEL);
-	if (data == NULL) {
-		res = -ENOMEM;
-		goto failed;
-	}
-
-	actor = squashfs_page_actor_init(data, pages, length);
+	actor = squashfs_page_actor_init(page, pages, length, NULL);
 	if (actor == NULL) {
 		res = -ENOMEM;
-		goto failed2;
+		goto failed;
 	}
 
-	for (i = 0; i < pages; i++, buffer += PAGE_SIZE)
-		data[i] = buffer;
-
 	res = squashfs_read_data(sb, block, length |
 		SQUASHFS_COMPRESSED_BIT_BLOCK, NULL, actor);
 
-	kfree(data);
-	kfree(actor);
-
 	if (res < 0)
-		goto failed;
+		goto failed2;
 
-	return table;
+	buff = kmalloc(length, GFP_KERNEL);
+	if (!buff)
+		goto failed2;
+	squashfs_actor_to_buf(actor, buff, length);
+	squashfs_page_actor_free(actor, 0);
+	free_page_array(page, pages);
+	return buff;
 
 failed2:
-	kfree(data);
+	squashfs_page_actor_free(actor, 0);
 failed:
-	kfree(table);
+	free_page_array(page, pages);
 	return ERR_PTR(res);
 }
diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c
index 836639810ea0..86831aaedb9e 100644
--- a/fs/squashfs/decompressor.c
+++ b/fs/squashfs/decompressor.c
@@ -24,7 +24,8 @@
 #include <linux/types.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
-#include <linux/buffer_head.h>
+#include <linux/highmem.h>
+#include <linux/fs.h>
 
 #include "squashfs_fs.h"
 #include "squashfs_fs_sb.h"
@@ -101,40 +102,44 @@ const struct squashfs_decompressor *squashfs_lookup_decompressor(int id)
 static void *get_comp_opts(struct super_block *sb, unsigned short flags)
 {
 	struct squashfs_sb_info *msblk = sb->s_fs_info;
-	void *buffer = NULL, *comp_opts;
+	void *comp_opts, *buffer = NULL;
+	struct page *page;
 	struct squashfs_page_actor *actor = NULL;
 	int length = 0;
 
+	if (!SQUASHFS_COMP_OPTS(flags))
+		return squashfs_comp_opts(msblk, buffer, length);
+
 	/*
 	 * Read decompressor specific options from file system if present
 	 */
-	if (SQUASHFS_COMP_OPTS(flags)) {
-		buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
-		if (buffer == NULL) {
-			comp_opts = ERR_PTR(-ENOMEM);
-			goto out;
-		}
-
-		actor = squashfs_page_actor_init(&buffer, 1, 0);
-		if (actor == NULL) {
-			comp_opts = ERR_PTR(-ENOMEM);
-			goto out;
-		}
-
-		length = squashfs_read_data(sb,
-			sizeof(struct squashfs_super_block), 0, NULL, actor);
-
-		if (length < 0) {
-			comp_opts = ERR_PTR(length);
-			goto out;
-		}
+
+	page = alloc_page(GFP_KERNEL);
+	if (!page)
+		return ERR_PTR(-ENOMEM);
+
+	actor = squashfs_page_actor_init(&page, 1, 0, NULL);
+	if (actor == NULL) {
+		comp_opts = ERR_PTR(-ENOMEM);
+		goto actor_error;
+	}
+
+	length = squashfs_read_data(sb,
+		sizeof(struct squashfs_super_block), 0, NULL, actor);
+
+	if (length < 0) {
+		comp_opts = ERR_PTR(length);
+		goto read_error;
 	}
 
+	buffer = kmap_atomic(page);
 	comp_opts = squashfs_comp_opts(msblk, buffer, length);
+	kunmap_atomic(buffer);
 
-out:
-	kfree(actor);
-	kfree(buffer);
+read_error:
+	squashfs_page_actor_free(actor, 0);
+actor_error:
+	__free_page(page);
 	return comp_opts;
 }
 
diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
index cb485d8e0e91..4642b869d931 100644
--- a/fs/squashfs/file_direct.c
+++ b/fs/squashfs/file_direct.c
@@ -52,7 +52,7 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
 	 * Create a "page actor" which will kmap and kunmap the
 	 * page cache pages appropriately within the decompressor
 	 */
-	actor = squashfs_page_actor_init_special(page, pages, 0);
+	actor = squashfs_page_actor_init(page, pages, 0, NULL);
 	if (actor == NULL)
 		goto out;
 
@@ -131,7 +131,7 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
 	}
 
 out:
-	kfree(actor);
+	squashfs_page_actor_free(actor, 0);
 	kfree(page);
 	return res;
 }
diff --git a/fs/squashfs/lz4_wrapper.c b/fs/squashfs/lz4_wrapper.c
index 95da65366548..854d5ece7d08 100644
--- a/fs/squashfs/lz4_wrapper.c
+++ b/fs/squashfs/lz4_wrapper.c
@@ -94,39 +94,18 @@ static int lz4_uncompress(struct squashfs_sb_info *msblk, void *strm,
 	struct buffer_head **bh, int b, int offset, int length,
 	struct squashfs_page_actor *output)
 {
+	int res;
 	struct squashfs_lz4 *stream = strm;
-	void *buff = stream->input, *data;
-	int avail, i, bytes = length, res;
-
-	for (i = 0; i < b; i++) {
-		avail = min(bytes, msblk->devblksize - offset);
-		memcpy(buff, bh[i]->b_data + offset, avail);
-		buff += avail;
-		bytes -= avail;
-		offset = 0;
-		put_bh(bh[i]);
-	}
 
+	squashfs_bh_to_buf(bh, b, stream->input, offset, length,
+		msblk->devblksize);
 	res = LZ4_decompress_safe(stream->input, stream->output,
 		length, output->length);
 
 	if (res < 0)
 		return -EIO;
 
-	bytes = res;
-	data = squashfs_first_page(output);
-	buff = stream->output;
-	while (data) {
-		if (bytes <= PAGE_SIZE) {
-			memcpy(data, buff, bytes);
-			break;
-		}
-		memcpy(data, buff, PAGE_SIZE);
-		buff += PAGE_SIZE;
-		bytes -= PAGE_SIZE;
-		data = squashfs_next_page(output);
-	}
-	squashfs_finish_page(output);
+	squashfs_buf_to_actor(stream->output, output, res);
 
 	return res;
 }
diff --git a/fs/squashfs/lzo_wrapper.c b/fs/squashfs/lzo_wrapper.c
index 934c17e96590..2c844d53a59e 100644
--- a/fs/squashfs/lzo_wrapper.c
+++ b/fs/squashfs/lzo_wrapper.c
@@ -79,45 +79,19 @@ static int lzo_uncompress(struct squashfs_sb_info *msblk, void *strm,
 	struct buffer_head **bh, int b, int offset, int length,
 	struct squashfs_page_actor *output)
 {
-	struct squashfs_lzo *stream = strm;
-	void *buff = stream->input, *data;
-	int avail, i, bytes = length, res;
+	int res;
 	size_t out_len = output->length;
+	struct squashfs_lzo *stream = strm;
 
-	for (i = 0; i < b; i++) {
-		avail = min(bytes, msblk->devblksize - offset);
-		memcpy(buff, bh[i]->b_data + offset, avail);
-		buff += avail;
-		bytes -= avail;
-		offset = 0;
-		put_bh(bh[i]);
-	}
-
+	squashfs_bh_to_buf(bh, b, stream->input, offset, length,
+		msblk->devblksize);
 	res = lzo1x_decompress_safe(stream->input, (size_t)length,
 					stream->output, &out_len);
 	if (res != LZO_E_OK)
-		goto failed;
+		return -EIO;
+	squashfs_buf_to_actor(stream->output, output, out_len);
 
-	res = bytes = (int)out_len;
-	data = squashfs_first_page(output);
-	buff = stream->output;
-	while (data) {
-		if (bytes <= PAGE_SIZE) {
-			memcpy(data, buff, bytes);
-			break;
-		} else {
-			memcpy(data, buff, PAGE_SIZE);
-			buff += PAGE_SIZE;
-			bytes -= PAGE_SIZE;
-			data = squashfs_next_page(output);
-		}
-	}
-	squashfs_finish_page(output);
-
-	return res;
-
-failed:
-	return -EIO;
+	return out_len;
 }
 
 const struct squashfs_decompressor squashfs_lzo_comp_ops = {
diff --git a/fs/squashfs/page_actor.c b/fs/squashfs/page_actor.c
index 9b7b1b6a7892..e348f5647fbd 100644
--- a/fs/squashfs/page_actor.c
+++ b/fs/squashfs/page_actor.c
@@ -9,39 +9,11 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/pagemap.h>
+#include <linux/buffer_head.h>
 #include "page_actor.h"
 
-/*
- * This file contains implementations of page_actor for decompressing into
- * an intermediate buffer, and for decompressing directly into the
- * page cache.
- *
- * Calling code should avoid sleeping between calls to squashfs_first_page()
- * and squashfs_finish_page().
- */
-
-/* Implementation of page_actor for decompressing into intermediate buffer */
-static void *cache_first_page(struct squashfs_page_actor *actor)
-{
-	actor->next_page = 1;
-	return actor->buffer[0];
-}
-
-static void *cache_next_page(struct squashfs_page_actor *actor)
-{
-	if (actor->next_page == actor->pages)
-		return NULL;
-
-	return actor->buffer[actor->next_page++];
-}
-
-static void cache_finish_page(struct squashfs_page_actor *actor)
-{
-	/* empty */
-}
-
-struct squashfs_page_actor *squashfs_page_actor_init(void **buffer,
-	int pages, int length)
+struct squashfs_page_actor *squashfs_page_actor_init(struct page **page,
+	int pages, int length, void (*release_pages)(struct page **, int, int))
 {
 	struct squashfs_page_actor *actor = kmalloc(sizeof(*actor), GFP_KERNEL);
 
@@ -49,52 +21,133 @@ struct squashfs_page_actor *squashfs_page_actor_init(void **buffer,
 		return NULL;
 
 	actor->length = length ? : pages * PAGE_SIZE;
-	actor->buffer = buffer;
+	actor->page = page;
 	actor->pages = pages;
 	actor->next_page = 0;
-	actor->squashfs_first_page = cache_first_page;
-	actor->squashfs_next_page = cache_next_page;
-	actor->squashfs_finish_page = cache_finish_page;
+	actor->pageaddr = NULL;
+	actor->release_pages = release_pages;
 	return actor;
 }
 
-/* Implementation of page_actor for decompressing directly into page cache. */
-static void *direct_first_page(struct squashfs_page_actor *actor)
+void squashfs_page_actor_free(struct squashfs_page_actor *actor, int error)
+{
+	if (!actor)
+		return;
+
+	if (actor->release_pages)
+		actor->release_pages(actor->page, actor->pages, error);
+	kfree(actor);
+}
+
+void squashfs_actor_to_buf(struct squashfs_page_actor *actor, void *buf,
+	int length)
 {
-	actor->next_page = 1;
-	return actor->pageaddr = kmap_atomic(actor->page[0]);
+	void *pageaddr;
+	int pos = 0, avail, i;
+
+	for (i = 0; i < actor->pages && pos < length; ++i) {
+		avail = min_t(int, length - pos, PAGE_SIZE);
+		if (actor->page[i]) {
+			pageaddr = kmap_atomic(actor->page[i]);
+			memcpy(buf + pos, pageaddr, avail);
+			kunmap_atomic(pageaddr);
+		}
+		pos += avail;
+	}
 }
 
-static void *direct_next_page(struct squashfs_page_actor *actor)
+void squashfs_buf_to_actor(void *buf, struct squashfs_page_actor *actor,
+	int length)
 {
-	if (actor->pageaddr)
-		kunmap_atomic(actor->pageaddr);
+	void *pageaddr;
+	int pos = 0, avail, i;
+
+	for (i = 0; i < actor->pages && pos < length; ++i) {
+		avail = min_t(int, length - pos, PAGE_SIZE);
+		if (actor->page[i]) {
+			pageaddr = kmap_atomic(actor->page[i]);
+			memcpy(pageaddr, buf + pos, avail);
+			kunmap_atomic(pageaddr);
+		}
+		pos += avail;
+	}
+}
 
-	return actor->pageaddr = actor->next_page == actor->pages ? NULL :
-		kmap_atomic(actor->page[actor->next_page++]);
+void squashfs_bh_to_actor(struct buffer_head **bh, int nr_buffers,
+	struct squashfs_page_actor *actor, int offset, int length, int blksz)
+{
+	void *kaddr = NULL;
+	int bytes = 0, pgoff = 0, b = 0, p = 0, avail, i;
+
+	while (bytes < length) {
+		if (actor->page[p]) {
+			kaddr = kmap_atomic(actor->page[p]);
+			while (pgoff < PAGE_SIZE && bytes < length) {
+				avail = min_t(int, blksz - offset,
+						PAGE_SIZE - pgoff);
+				memcpy(kaddr + pgoff, bh[b]->b_data + offset,
+				       avail);
+				pgoff += avail;
+				bytes += avail;
+				offset = (offset + avail) % blksz;
+				if (!offset) {
+					put_bh(bh[b]);
+					++b;
+				}
+			}
+			kunmap_atomic(kaddr);
+			pgoff = 0;
+		} else {
+			for (i = 0; i < PAGE_SIZE / blksz; ++i) {
+				if (bh[b])
+					put_bh(bh[b]);
+				++b;
+			}
+			bytes += PAGE_SIZE;
+		}
+		++p;
+	}
 }
 
-static void direct_finish_page(struct squashfs_page_actor *actor)
+void squashfs_bh_to_buf(struct buffer_head **bh, int nr_buffers, void *buf,
+	int offset, int length, int blksz)
 {
-	if (actor->pageaddr)
-		kunmap_atomic(actor->pageaddr);
+	int i, avail, bytes = 0;
+
+	for (i = 0; i < nr_buffers && bytes < length; ++i) {
+		avail = min_t(int, length - bytes, blksz - offset);
+		if (bh[i]) {
+			memcpy(buf + bytes, bh[i]->b_data + offset, avail);
+			put_bh(bh[i]);
+		}
+		bytes += avail;
+		offset = 0;
+	}
 }
 
-struct squashfs_page_actor *squashfs_page_actor_init_special(struct page **page,
-	int pages, int length)
+void free_page_array(struct page **page, int nr_pages)
 {
-	struct squashfs_page_actor *actor = kmalloc(sizeof(*actor), GFP_KERNEL);
+	int i;
 
-	if (actor == NULL)
-		return NULL;
+	for (i = 0; i < nr_pages; ++i)
+		__free_page(page[i]);
+	kfree(page);
+}
 
-	actor->length = length ? : pages * PAGE_SIZE;
-	actor->page = page;
-	actor->pages = pages;
-	actor->next_page = 0;
-	actor->pageaddr = NULL;
-	actor->squashfs_first_page = direct_first_page;
-	actor->squashfs_next_page = direct_next_page;
-	actor->squashfs_finish_page = direct_finish_page;
-	return actor;
+struct page **alloc_page_array(int nr_pages, int gfp_mask)
+{
+	int i;
+	struct page **page;
+
+	page = kcalloc(nr_pages, sizeof(struct page *), gfp_mask);
+	if (!page)
+		return NULL;
+	for (i = 0; i < nr_pages; ++i) {
+		page[i] = alloc_page(gfp_mask);
+		if (!page[i]) {
+			free_page_array(page, i);
+			return NULL;
+		}
+	}
+	return page;
 }
diff --git a/fs/squashfs/page_actor.h b/fs/squashfs/page_actor.h
index d2df0544e0df..aa1ed790b5a3 100644
--- a/fs/squashfs/page_actor.h
+++ b/fs/squashfs/page_actor.h
@@ -5,37 +5,61 @@
  * Phillip Lougher <phillip@squashfs.org.uk>
  *
  * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
+ * the COPYING file in the top-level squashfsory.
  */
 
 struct squashfs_page_actor {
-	union {
-		void		**buffer;
-		struct page	**page;
-	};
+	struct page	**page;
 	void	*pageaddr;
-	void    *(*squashfs_first_page)(struct squashfs_page_actor *);
-	void    *(*squashfs_next_page)(struct squashfs_page_actor *);
-	void    (*squashfs_finish_page)(struct squashfs_page_actor *);
 	int	pages;
 	int	length;
 	int	next_page;
+	void	(*release_pages)(struct page **, int, int);
 };
 
-extern struct squashfs_page_actor *squashfs_page_actor_init(void **, int, int);
-extern struct squashfs_page_actor *squashfs_page_actor_init_special(struct page
-							 **, int, int);
+extern struct squashfs_page_actor *squashfs_page_actor_init(struct page **,
+	int, int, void (*)(struct page **, int, int));
+extern void squashfs_page_actor_free(struct squashfs_page_actor *, int);
+
+extern void squashfs_actor_to_buf(struct squashfs_page_actor *, void *, int);
+extern void squashfs_buf_to_actor(void *, struct squashfs_page_actor *, int);
+extern void squashfs_bh_to_actor(struct buffer_head **, int,
+	struct squashfs_page_actor *, int, int, int);
+extern void squashfs_bh_to_buf(struct buffer_head **, int, void *, int, int,
+	int);
+
+/*
+ * Calling code should avoid sleeping between calls to squashfs_first_page()
+ * and squashfs_finish_page().
+ */
 static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
 {
-	return actor->squashfs_first_page(actor);
+	actor->next_page = 1;
+	return actor->pageaddr = actor->page[0] ? kmap_atomic(actor->page[0])
+						: NULL;
 }
+
 static inline void *squashfs_next_page(struct squashfs_page_actor *actor)
 {
-	return actor->squashfs_next_page(actor);
+	if (!IS_ERR_OR_NULL(actor->pageaddr))
+		kunmap_atomic(actor->pageaddr);
+
+	if (actor->next_page == actor->pages)
+		return actor->pageaddr = ERR_PTR(-ENODATA);
+
+	actor->pageaddr = actor->page[actor->next_page] ?
+	    kmap_atomic(actor->page[actor->next_page]) : NULL;
+	++actor->next_page;
+	return actor->pageaddr;
 }
+
 static inline void squashfs_finish_page(struct squashfs_page_actor *actor)
 {
-	actor->squashfs_finish_page(actor);
+	if (!IS_ERR_OR_NULL(actor->pageaddr))
+		kunmap_atomic(actor->pageaddr);
 }
 
+extern struct page **alloc_page_array(int, int);
+extern void free_page_array(struct page **, int);
+
 #endif
diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h
index 1da565cb50c3..8a6995de0277 100644
--- a/fs/squashfs/squashfs_fs_sb.h
+++ b/fs/squashfs/squashfs_fs_sb.h
@@ -49,7 +49,7 @@ struct squashfs_cache_entry {
 	int			num_waiters;
 	wait_queue_head_t	wait_queue;
 	struct squashfs_cache	*cache;
-	void			**data;
+	struct page		**page;
 	struct squashfs_page_actor	*actor;
 };
 
diff --git a/fs/squashfs/xz_wrapper.c b/fs/squashfs/xz_wrapper.c
index 6bfaef73d065..2f7be1fb167c 100644
--- a/fs/squashfs/xz_wrapper.c
+++ b/fs/squashfs/xz_wrapper.c
@@ -55,7 +55,7 @@ static void *squashfs_xz_comp_opts(struct squashfs_sb_info *msblk,
 	struct comp_opts *opts;
 	int err = 0, n;
 
-	opts = kmalloc(sizeof(*opts), GFP_KERNEL);
+	opts = kmalloc(sizeof(*opts), GFP_ATOMIC);
 	if (opts == NULL) {
 		err = -ENOMEM;
 		goto out2;
@@ -136,6 +136,7 @@ static int squashfs_xz_uncompress(struct squashfs_sb_info *msblk, void *strm,
 	enum xz_ret xz_err;
 	int avail, total = 0, k = 0;
 	struct squashfs_xz *stream = strm;
+	void *buf = NULL;
 
 	xz_dec_reset(stream->state);
 	stream->buf.in_pos = 0;
@@ -156,12 +157,20 @@ static int squashfs_xz_uncompress(struct squashfs_sb_info *msblk, void *strm,
 
 		if (stream->buf.out_pos == stream->buf.out_size) {
 			stream->buf.out = squashfs_next_page(output);
-			if (stream->buf.out != NULL) {
+			if (!IS_ERR(stream->buf.out)) {
 				stream->buf.out_pos = 0;
 				total += PAGE_SIZE;
 			}
 		}
 
+		if (!stream->buf.out) {
+			if (!buf) {
+				buf = kmalloc(PAGE_SIZE, GFP_ATOMIC);
+				if (!buf)
+					goto out;
+			}
+			stream->buf.out = buf;
+		}
 		xz_err = xz_dec_run(stream->state, &stream->buf);
 
 		if (stream->buf.in_pos == stream->buf.in_size && k < b)
@@ -173,11 +182,13 @@ static int squashfs_xz_uncompress(struct squashfs_sb_info *msblk, void *strm,
 	if (xz_err != XZ_STREAM_END || k < b)
 		goto out;
 
+	kfree(buf);
 	return total + stream->buf.out_pos;
 
 out:
 	for (; k < b; k++)
 		put_bh(bh[k]);
+	kfree(buf);
 
 	return -EIO;
 }
diff --git a/fs/squashfs/zlib_wrapper.c b/fs/squashfs/zlib_wrapper.c
index 2ec24d128bce..d917c728422b 100644
--- a/fs/squashfs/zlib_wrapper.c
+++ b/fs/squashfs/zlib_wrapper.c
@@ -66,6 +66,7 @@ static int zlib_uncompress(struct squashfs_sb_info *msblk, void *strm,
 	struct buffer_head **bh, int b, int offset, int length,
 	struct squashfs_page_actor *output)
 {
+	void *buf = NULL;
 	int zlib_err, zlib_init = 0, k = 0;
 	z_stream *stream = strm;
 
@@ -84,10 +85,19 @@ static int zlib_uncompress(struct squashfs_sb_info *msblk, void *strm,
 
 		if (stream->avail_out == 0) {
 			stream->next_out = squashfs_next_page(output);
-			if (stream->next_out != NULL)
+			if (!IS_ERR(stream->next_out))
 				stream->avail_out = PAGE_SIZE;
 		}
 
+		if (!stream->next_out) {
+			if (!buf) {
+				buf = kmalloc(PAGE_SIZE, GFP_ATOMIC);
+				if (!buf)
+					goto out;
+			}
+			stream->next_out = buf;
+		}
+
 		if (!zlib_init) {
 			zlib_err = zlib_inflateInit(stream);
 			if (zlib_err != Z_OK) {
@@ -115,11 +125,13 @@ static int zlib_uncompress(struct squashfs_sb_info *msblk, void *strm,
 	if (k < b)
 		goto out;
 
+	kfree(buf);
 	return stream->total_out;
 
 out:
 	for (; k < b; k++)
 		put_bh(bh[k]);
+	kfree(buf);
 
 	return -EIO;
 }
-- 
2.14.1.821.g8fa685d3b7-goog

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] Squashfs: replace buffer_head with BIO
  2017-09-22 21:55 [PATCH 0/5] Squashfs Whitelist and Compression Threshold Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 1/5] Squashfs: remove the FILE_CACHE option Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 2/5] Squashfs: refactor page_actor Daniel Rosenberg
@ 2017-09-22 21:55 ` Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 4/5] Squashfs: implement .readpages() Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 5/5] Squashfs: optimize reading uncompressed data Daniel Rosenberg
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel Rosenberg @ 2017-09-22 21:55 UTC (permalink / raw)
  To: Phillip Lougher, linux-kernel
  Cc: adrien, Adrien Schildknecht, Daniel Rosenberg

From: Adrien Schildknecht <adriens@google.com>

The 'll_rw_block' has been deprecated and BIO is now the basic container
for block I/O within the kernel.

Switching to BIO offers 2 advantages:
  1/ It removes synchronous wait for the up-to-date buffers: SquashFS
     now deals with decompressions/copies asynchronously.
     Implementing an asynchronous mechanism to read data is needed to
     efficiently implement .readpages().
  2/ Prior to this patch, merging the read requests entirely depends on
     the IO scheduler. SquashFS has more information than the IO
     scheduler about what could be merged. Moreover, merging the reads
     at the FS level means that we rely less on the IO scheduler.

Signed-off-by: Adrien Schildknecht <adriens@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/squashfs/block.c       | 522 +++++++++++++++++++++++++++++++++-------------
 fs/squashfs/file_direct.c | 195 ++++++-----------
 fs/squashfs/squashfs.h    |   4 +
 fs/squashfs/super.c       |   7 +
 4 files changed, 456 insertions(+), 272 deletions(-)

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 2751476e6b6e..252dfc82ae72 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -32,6 +32,8 @@
 #include <linux/string.h>
 #include <linux/buffer_head.h>
 #include <linux/bio.h>
+#include <linux/pagemap.h>
+#include <linux/workqueue.h>
 
 #include "squashfs_fs.h"
 #include "squashfs_fs_sb.h"
@@ -39,45 +41,357 @@
 #include "decompressor.h"
 #include "page_actor.h"
 
-/*
- * Read the metadata block length, this is stored in the first two
- * bytes of the metadata block.
- */
-static struct buffer_head *get_block_length(struct super_block *sb,
-			u64 *cur_index, int *offset, int *length)
+static struct workqueue_struct *squashfs_read_wq;
+
+struct squashfs_read_request {
+	struct super_block *sb;
+	u64 index;
+	int length;
+	int compressed;
+	int offset;
+	u64 read_end;
+	struct squashfs_page_actor *output;
+	enum {
+		SQUASHFS_COPY,
+		SQUASHFS_DECOMPRESS,
+		SQUASHFS_METADATA,
+	} data_processing;
+	bool synchronous;
+
+	/*
+	 * If the read is synchronous, it is possible to retrieve information
+	 * about the request by setting these pointers.
+	 */
+	int *res;
+	int *bytes_read;
+	int *bytes_uncompressed;
+
+	int nr_buffers;
+	struct buffer_head **bh;
+	struct work_struct offload;
+};
+
+struct squashfs_bio_request {
+	struct buffer_head **bh;
+	int nr_buffers;
+};
+
+static int squashfs_bio_submit(struct squashfs_read_request *req);
+
+int squashfs_init_read_wq(void)
+{
+	squashfs_read_wq = create_workqueue("SquashFS read wq");
+	return !!squashfs_read_wq;
+}
+
+void squashfs_destroy_read_wq(void)
 {
-	struct squashfs_sb_info *msblk = sb->s_fs_info;
+	flush_workqueue(squashfs_read_wq);
+	destroy_workqueue(squashfs_read_wq);
+}
+
+static void free_read_request(struct squashfs_read_request *req, int error)
+{
+	if (!req->synchronous)
+		squashfs_page_actor_free(req->output, error);
+	if (req->res)
+		*(req->res) = error;
+	kfree(req->bh);
+	kfree(req);
+}
+
+static void squashfs_process_blocks(struct squashfs_read_request *req)
+{
+	int error = 0;
+	int bytes, i, length;
+	struct squashfs_sb_info *msblk = req->sb->s_fs_info;
+	struct squashfs_page_actor *actor = req->output;
+	struct buffer_head **bh = req->bh;
+	int nr_buffers = req->nr_buffers;
+
+	for (i = 0; i < nr_buffers; ++i) {
+		if (!bh[i])
+			continue;
+		wait_on_buffer(bh[i]);
+		if (!buffer_uptodate(bh[i]))
+			error = -EIO;
+	}
+	if (error)
+		goto cleanup;
+
+	if (req->data_processing == SQUASHFS_METADATA) {
+		/* Extract the length of the metadata block */
+		if (req->offset != msblk->devblksize - 1) {
+			length = le16_to_cpup((__le16 *)
+					(bh[0]->b_data + req->offset));
+		} else {
+			length = (unsigned char)bh[0]->b_data[req->offset];
+			length |= (unsigned char)bh[1]->b_data[0] << 8;
+		}
+		req->compressed = SQUASHFS_COMPRESSED(length);
+		req->data_processing = req->compressed ? SQUASHFS_DECOMPRESS
+						       : SQUASHFS_COPY;
+		length = SQUASHFS_COMPRESSED_SIZE(length);
+		if (req->index + length + 2 > req->read_end) {
+			for (i = 0; i < nr_buffers; ++i)
+				put_bh(bh[i]);
+			kfree(bh);
+			req->length = length;
+			req->index += 2;
+			squashfs_bio_submit(req);
+			return;
+		}
+		req->length = length;
+		req->offset = (req->offset + 2) % PAGE_SIZE;
+		if (req->offset < 2) {
+			put_bh(bh[0]);
+			++bh;
+			--nr_buffers;
+		}
+	}
+	if (req->bytes_read)
+		*(req->bytes_read) = req->length;
+
+	if (req->data_processing == SQUASHFS_COPY) {
+		squashfs_bh_to_actor(bh, nr_buffers, req->output, req->offset,
+			req->length, msblk->devblksize);
+	} else if (req->data_processing == SQUASHFS_DECOMPRESS) {
+		req->length = squashfs_decompress(msblk, bh, nr_buffers,
+			req->offset, req->length, actor);
+		if (req->length < 0) {
+			error = -EIO;
+			goto cleanup;
+		}
+	}
+
+	/* Last page may have trailing bytes not filled */
+	bytes = req->length % PAGE_SIZE;
+	if (bytes && actor->page[actor->pages - 1])
+		zero_user_segment(actor->page[actor->pages - 1], bytes,
+				  PAGE_SIZE);
+
+cleanup:
+	if (req->bytes_uncompressed)
+		*(req->bytes_uncompressed) = req->length;
+	if (error) {
+		for (i = 0; i < nr_buffers; ++i)
+			if (bh[i])
+				put_bh(bh[i]);
+	}
+	free_read_request(req, error);
+}
+
+static void read_wq_handler(struct work_struct *work)
+{
+	squashfs_process_blocks(container_of(work,
+		    struct squashfs_read_request, offload));
+}
+
+static void squashfs_bio_end_io(struct bio *bio)
+{
+	int i;
+	int error = bio->bi_status;
+	struct squashfs_bio_request *bio_req = bio->bi_private;
+
+	bio_put(bio);
+
+	for (i = 0; i < bio_req->nr_buffers; ++i) {
+		if (!bio_req->bh[i])
+			continue;
+		if (!error)
+			set_buffer_uptodate(bio_req->bh[i]);
+		else
+			clear_buffer_uptodate(bio_req->bh[i]);
+		unlock_buffer(bio_req->bh[i]);
+	}
+	kfree(bio_req);
+}
+
+static int actor_getblks(struct squashfs_read_request *req, u64 block)
+{
+	int i;
+
+	req->bh = kmalloc_array(req->nr_buffers, sizeof(*(req->bh)), GFP_NOIO);
+	if (!req->bh)
+		return -ENOMEM;
+
+	for (i = 0; i < req->nr_buffers; ++i) {
+		req->bh[i] = sb_getblk(req->sb, block + i);
+		if (!req->bh[i]) {
+			while (--i) {
+				if (req->bh[i])
+					put_bh(req->bh[i]);
+			}
+			return -1;
+		}
+	}
+	return 0;
+}
+
+static int squashfs_bio_submit(struct squashfs_read_request *req)
+{
+	struct bio *bio = NULL;
 	struct buffer_head *bh;
+	struct squashfs_bio_request *bio_req = NULL;
+	int b = 0, prev_block = 0;
+	struct squashfs_sb_info *msblk = req->sb->s_fs_info;
+
+	u64 read_start = round_down(req->index, msblk->devblksize);
+	u64 read_end = round_up(req->index + req->length, msblk->devblksize);
+	sector_t block = read_start >> msblk->devblksize_log2;
+	sector_t block_end = read_end >> msblk->devblksize_log2;
+	int offset = read_start - round_down(req->index, PAGE_SIZE);
+	int nr_buffers = block_end - block;
+	int blksz = msblk->devblksize;
+	int bio_max_pages = nr_buffers > BIO_MAX_PAGES ? BIO_MAX_PAGES
+						       : nr_buffers;
+
+	/* Setup the request */
+	req->read_end = read_end;
+	req->offset = req->index - read_start;
+	req->nr_buffers = nr_buffers;
+	if (actor_getblks(req, block) < 0)
+		goto getblk_failed;
 
-	bh = sb_bread(sb, *cur_index);
-	if (bh == NULL)
-		return NULL;
-
-	if (msblk->devblksize - *offset == 1) {
-		*length = (unsigned char) bh->b_data[*offset];
-		put_bh(bh);
-		bh = sb_bread(sb, ++(*cur_index));
-		if (bh == NULL)
-			return NULL;
-		*length |= (unsigned char) bh->b_data[0] << 8;
-		*offset = 1;
-	} else {
-		*length = (unsigned char) bh->b_data[*offset] |
-			(unsigned char) bh->b_data[*offset + 1] << 8;
-		*offset += 2;
-
-		if (*offset == msblk->devblksize) {
-			put_bh(bh);
-			bh = sb_bread(sb, ++(*cur_index));
-			if (bh == NULL)
-				return NULL;
-			*offset = 0;
+	/* Create and submit the BIOs */
+	for (b = 0; b < nr_buffers; ++b, offset += blksz) {
+		bh = req->bh[b];
+		if (!bh || !trylock_buffer(bh))
+			continue;
+		if (buffer_uptodate(bh)) {
+			unlock_buffer(bh);
+			continue;
 		}
+		offset %= PAGE_SIZE;
+
+		/* Append the buffer to the current BIO if it is contiguous */
+		if (bio && bio_req && prev_block + 1 == b) {
+			if (bio_add_page(bio, bh->b_page, blksz, offset)) {
+				bio_req->nr_buffers += 1;
+				prev_block = b;
+				continue;
+			}
+		}
+
+		/* Otherwise, submit the current BIO and create a new one */
+		if (bio)
+			submit_bio(bio);
+		bio_req = kcalloc(1, sizeof(struct squashfs_bio_request),
+				  GFP_NOIO);
+		if (!bio_req)
+			goto req_alloc_failed;
+		bio_req->bh = &req->bh[b];
+		bio = bio_alloc(GFP_NOIO, bio_max_pages);
+		if (!bio)
+			goto bio_alloc_failed;
+		bio_set_dev(bio, req->sb->s_bdev);
+		bio->bi_iter.bi_sector = (block + b)
+				       << (msblk->devblksize_log2 - 9);
+		bio_set_op_attrs(bio, REQ_OP_READ, 0);
+		bio->bi_private = bio_req;
+		bio->bi_end_io = squashfs_bio_end_io;
+
+		bio_add_page(bio, bh->b_page, blksz, offset);
+		bio_req->nr_buffers += 1;
+		prev_block = b;
 	}
+	if (bio)
+		submit_bio(bio);
 
-	return bh;
+	if (req->synchronous)
+		squashfs_process_blocks(req);
+	else {
+		INIT_WORK(&req->offload, read_wq_handler);
+		schedule_work(&req->offload);
+	}
+	return 0;
+
+bio_alloc_failed:
+	kfree(bio_req);
+req_alloc_failed:
+	unlock_buffer(bh);
+	while (--nr_buffers >= b)
+		if (req->bh[nr_buffers])
+			put_bh(req->bh[nr_buffers]);
+	while (--b >= 0)
+		if (req->bh[b])
+			wait_on_buffer(req->bh[b]);
+getblk_failed:
+	free_read_request(req, -ENOMEM);
+	return -ENOMEM;
+}
+
+static int read_metadata_block(struct squashfs_read_request *req,
+			       u64 *next_index)
+{
+	int ret, error, bytes_read = 0, bytes_uncompressed = 0;
+	struct squashfs_sb_info *msblk = req->sb->s_fs_info;
+
+	if (req->index + 2 > msblk->bytes_used) {
+		free_read_request(req, -EINVAL);
+		return -EINVAL;
+	}
+	req->length = 2;
+
+	/* Do not read beyond the end of the device */
+	if (req->index + req->length > msblk->bytes_used)
+		req->length = msblk->bytes_used - req->index;
+	req->data_processing = SQUASHFS_METADATA;
+
+	/*
+	 * Reading metadata is always synchronous because we don't know the
+	 * length in advance and the function is expected to update
+	 * 'next_index' and return the length.
+	 */
+	req->synchronous = true;
+	req->res = &error;
+	req->bytes_read = &bytes_read;
+	req->bytes_uncompressed = &bytes_uncompressed;
+
+	TRACE("Metadata block @ 0x%llx, %scompressed size %d, src size %d\n",
+	      req->index, req->compressed ? "" : "un", bytes_read,
+	      req->output->length);
+
+	ret = squashfs_bio_submit(req);
+	if (ret)
+		return ret;
+	if (error)
+		return error;
+	if (next_index)
+		*next_index += 2 + bytes_read;
+	return bytes_uncompressed;
 }
 
+static int read_data_block(struct squashfs_read_request *req, int length,
+			   u64 *next_index, bool synchronous)
+{
+	int ret, error = 0, bytes_uncompressed = 0, bytes_read = 0;
+
+	req->compressed = SQUASHFS_COMPRESSED_BLOCK(length);
+	req->length = length = SQUASHFS_COMPRESSED_SIZE_BLOCK(length);
+	req->data_processing = req->compressed ? SQUASHFS_DECOMPRESS
+					       : SQUASHFS_COPY;
+
+	req->synchronous = synchronous;
+	if (synchronous) {
+		req->res = &error;
+		req->bytes_read = &bytes_read;
+		req->bytes_uncompressed = &bytes_uncompressed;
+	}
+
+	TRACE("Data block @ 0x%llx, %scompressed size %d, src size %d\n",
+	      req->index, req->compressed ? "" : "un", req->length,
+	      req->output->length);
+
+	ret = squashfs_bio_submit(req);
+	if (ret)
+		return ret;
+	if (synchronous)
+		ret = error ? error : bytes_uncompressed;
+	if (next_index)
+		*next_index += length;
+	return ret;
+}
 
 /*
  * Read and decompress a metadata block or datablock.  Length is non-zero
@@ -88,128 +402,50 @@ static struct buffer_head *get_block_length(struct super_block *sb,
  * generated a larger block - this does occasionally happen with compression
  * algorithms).
  */
-int squashfs_read_data(struct super_block *sb, u64 index, int length,
-		u64 *next_index, struct squashfs_page_actor *output)
+static int __squashfs_read_data(struct super_block *sb, u64 index, int length,
+	u64 *next_index, struct squashfs_page_actor *output, bool sync)
 {
-	struct squashfs_sb_info *msblk = sb->s_fs_info;
-	struct buffer_head **bh;
-	int offset = index & ((1 << msblk->devblksize_log2) - 1);
-	u64 cur_index = index >> msblk->devblksize_log2;
-	int bytes, compressed, b = 0, k = 0, avail, i;
+	struct squashfs_read_request *req;
 
-	bh = kcalloc(((output->length + msblk->devblksize - 1)
-		>> msblk->devblksize_log2) + 1, sizeof(*bh), GFP_KERNEL);
-	if (bh == NULL)
+	req = kcalloc(1, sizeof(struct squashfs_read_request), GFP_KERNEL);
+	if (!req) {
+		if (!sync)
+			squashfs_page_actor_free(output, -ENOMEM);
 		return -ENOMEM;
+	}
 
-	if (length) {
-		/*
-		 * Datablock.
-		 */
-		bytes = -offset;
-		compressed = SQUASHFS_COMPRESSED_BLOCK(length);
-		length = SQUASHFS_COMPRESSED_SIZE_BLOCK(length);
-		if (next_index)
-			*next_index = index + length;
-
-		TRACE("Block @ 0x%llx, %scompressed size %d, src size %d\n",
-			index, compressed ? "" : "un", length, output->length);
-
-		if (length < 0 || length > output->length ||
-				(index + length) > msblk->bytes_used)
-			goto read_failure;
-
-		for (b = 0; bytes < length; b++, cur_index++) {
-			bh[b] = sb_getblk(sb, cur_index);
-			if (bh[b] == NULL)
-				goto block_release;
-			bytes += msblk->devblksize;
-		}
-		ll_rw_block(REQ_OP_READ, 0, b, bh);
-	} else {
-		/*
-		 * Metadata block.
-		 */
-		if ((index + 2) > msblk->bytes_used)
-			goto read_failure;
-
-		bh[0] = get_block_length(sb, &cur_index, &offset, &length);
-		if (bh[0] == NULL)
-			goto read_failure;
-		b = 1;
-
-		bytes = msblk->devblksize - offset;
-		compressed = SQUASHFS_COMPRESSED(length);
-		length = SQUASHFS_COMPRESSED_SIZE(length);
-		if (next_index)
-			*next_index = index + length + 2;
+	req->sb = sb;
+	req->index = index;
+	req->output = output;
 
-		TRACE("Block @ 0x%llx, %scompressed size %d\n", index,
-				compressed ? "" : "un", length);
+	if (next_index)
+		*next_index = index;
 
-		if (length < 0 || length > output->length ||
-					(index + length) > msblk->bytes_used)
-			goto block_release;
+	if (length)
+		length = read_data_block(req, length, next_index, sync);
+	else
+		length = read_metadata_block(req, next_index);
 
-		for (; bytes < length; b++) {
-			bh[b] = sb_getblk(sb, ++cur_index);
-			if (bh[b] == NULL)
-				goto block_release;
-			bytes += msblk->devblksize;
-		}
-		ll_rw_block(REQ_OP_READ, 0, b - 1, bh + 1);
+	if (length < 0) {
+		ERROR("squashfs_read_data failed to read block 0x%llx\n",
+		      (unsigned long long)index);
+		return -EIO;
 	}
 
-	for (i = 0; i < b; i++) {
-		wait_on_buffer(bh[i]);
-		if (!buffer_uptodate(bh[i]))
-			goto block_release;
-	}
-
-	if (compressed) {
-		length = squashfs_decompress(msblk, bh, b, offset, length,
-			output);
-		if (length < 0)
-			goto read_failure;
-	} else {
-		/*
-		 * Block is uncompressed.
-		 */
-		int in, pg_offset = 0;
-		void *data = squashfs_first_page(output);
-
-		for (bytes = length; k < b; k++) {
-			in = min(bytes, msblk->devblksize - offset);
-			bytes -= in;
-			while (in) {
-				if (pg_offset == PAGE_SIZE) {
-					data = squashfs_next_page(output);
-					pg_offset = 0;
-				}
-				avail = min_t(int, in, PAGE_SIZE -
-						pg_offset);
-				memcpy(data + pg_offset, bh[k]->b_data + offset,
-						avail);
-				in -= avail;
-				pg_offset += avail;
-				offset += avail;
-			}
-			offset = 0;
-			put_bh(bh[k]);
-		}
-		squashfs_finish_page(output);
-	}
-
-	kfree(bh);
 	return length;
+}
+
+int squashfs_read_data(struct super_block *sb, u64 index, int length,
+	u64 *next_index, struct squashfs_page_actor *output)
+{
+	return __squashfs_read_data(sb, index, length, next_index, output,
+				    true);
+}
 
-block_release:
-	for (; k < b; k++)
-		put_bh(bh[k]);
+int squashfs_read_data_async(struct super_block *sb, u64 index, int length,
+	u64 *next_index, struct squashfs_page_actor *output)
+{
 
-read_failure:
-	ERROR("squashfs_read_data failed to read block 0x%llx\n",
-					(unsigned long long) index);
-	kfree(bh);
-	return -EIO;
+	return __squashfs_read_data(sb, index, length, next_index, output,
+				    false);
 }
diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
index 4642b869d931..74b8f381320b 100644
--- a/fs/squashfs/file_direct.c
+++ b/fs/squashfs/file_direct.c
@@ -20,157 +20,94 @@
 #include "squashfs.h"
 #include "page_actor.h"
 
-static int squashfs_read_cache(struct page *target_page, u64 block, int bsize,
-	int pages, struct page **page);
+static void release_actor_pages(struct page **page, int pages, int error)
+{
+	int i;
 
-/* Read separately compressed datablock directly into page cache */
-int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
+	for (i = 0; i < pages; i++) {
+		if (!page[i])
+			continue;
+		flush_dcache_page(page[i]);
+		if (!error)
+			SetPageUptodate(page[i]);
+		else {
+			SetPageError(page[i]);
+			zero_user_segment(page[i], 0, PAGE_SIZE);
+		}
+		unlock_page(page[i]);
+		put_page(page[i]);
+	}
+	kfree(page);
+}
 
+/*
+ * Create a "page actor" which will kmap and kunmap the
+ * page cache pages appropriately within the decompressor
+ */
+static struct squashfs_page_actor *actor_from_page_cache(
+	struct page *target_page, int start_index, int nr_pages)
 {
-	struct inode *inode = target_page->mapping->host;
-	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
-
-	int file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
-	int mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
-	int start_index = target_page->index & ~mask;
-	int end_index = start_index | mask;
-	int i, n, pages, missing_pages, bytes, res = -ENOMEM;
+	int i, n;
 	struct page **page;
 	struct squashfs_page_actor *actor;
-	void *pageaddr;
-
-	if (end_index > file_end)
-		end_index = file_end;
 
-	pages = end_index - start_index + 1;
-
-	page = kmalloc_array(pages, sizeof(void *), GFP_KERNEL);
-	if (page == NULL)
-		return res;
-
-	/*
-	 * Create a "page actor" which will kmap and kunmap the
-	 * page cache pages appropriately within the decompressor
-	 */
-	actor = squashfs_page_actor_init(page, pages, 0, NULL);
-	if (actor == NULL)
-		goto out;
-
-	/* Try to grab all the pages covered by the Squashfs block */
-	for (missing_pages = 0, i = 0, n = start_index; i < pages; i++, n++) {
-		page[i] = (n == target_page->index) ? target_page :
-			grab_cache_page_nowait(target_page->mapping, n);
-
-		if (page[i] == NULL) {
-			missing_pages++;
-			continue;
+	page = kmalloc_array(nr_pages, sizeof(void *), GFP_KERNEL);
+	if (!page)
+		return NULL;
+
+	/* Try to grab all the pages covered by the SquashFS block */
+	for (i = 0, n = start_index; i < nr_pages; i++, n++) {
+		if (target_page->index == n) {
+			page[i] = target_page;
+		} else {
+			page[i] = grab_cache_page_nowait(target_page->mapping,
+							 n);
+			if (page[i] == NULL)
+				continue;
 		}
 
 		if (PageUptodate(page[i])) {
 			unlock_page(page[i]);
 			put_page(page[i]);
 			page[i] = NULL;
-			missing_pages++;
 		}
 	}
 
-	if (missing_pages) {
-		/*
-		 * Couldn't get one or more pages, this page has either
-		 * been VM reclaimed, but others are still in the page cache
-		 * and uptodate, or we're racing with another thread in
-		 * squashfs_readpage also trying to grab them.  Fall back to
-		 * using an intermediate buffer.
-		 */
-		res = squashfs_read_cache(target_page, block, bsize, pages,
-								page);
-		if (res < 0)
-			goto mark_errored;
-
-		goto out;
-	}
-
-	/* Decompress directly into the page cache buffers */
-	res = squashfs_read_data(inode->i_sb, block, bsize, NULL, actor);
-	if (res < 0)
-		goto mark_errored;
-
-	/* Last page may have trailing bytes not filled */
-	bytes = res % PAGE_SIZE;
-	if (bytes) {
-		pageaddr = kmap_atomic(page[pages - 1]);
-		memset(pageaddr + bytes, 0, PAGE_SIZE - bytes);
-		kunmap_atomic(pageaddr);
+	actor = squashfs_page_actor_init(page, nr_pages, 0,
+			release_actor_pages);
+	if (!actor) {
+		release_actor_pages(page, nr_pages, -ENOMEM);
+		kfree(page);
+		return NULL;
 	}
-
-	/* Mark pages as uptodate, unlock and release */
-	for (i = 0; i < pages; i++) {
-		flush_dcache_page(page[i]);
-		SetPageUptodate(page[i]);
-		unlock_page(page[i]);
-		if (page[i] != target_page)
-			put_page(page[i]);
-	}
-
-	kfree(actor);
-	kfree(page);
-
-	return 0;
-
-mark_errored:
-	/* Decompression failed, mark pages as errored.  Target_page is
-	 * dealt with by the caller
-	 */
-	for (i = 0; i < pages; i++) {
-		if (page[i] == NULL || page[i] == target_page)
-			continue;
-		flush_dcache_page(page[i]);
-		SetPageError(page[i]);
-		unlock_page(page[i]);
-		put_page(page[i]);
-	}
-
-out:
-	squashfs_page_actor_free(actor, 0);
-	kfree(page);
-	return res;
+	return actor;
 }
 
+/* Read separately compressed datablock directly into page cache */
+int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
 
-static int squashfs_read_cache(struct page *target_page, u64 block, int bsize,
-	int pages, struct page **page)
 {
-	struct inode *i = target_page->mapping->host;
-	struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
-						 block, bsize);
-	int bytes = buffer->length, res = buffer->error, n, offset = 0;
-	void *pageaddr;
-
-	if (res) {
-		ERROR("Unable to read page, block %llx, size %x\n", block,
-			bsize);
-		goto out;
-	}
+	struct inode *inode = target_page->mapping->host;
+	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
 
-	for (n = 0; n < pages && bytes > 0; n++,
-			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
-		int avail = min_t(int, bytes, PAGE_SIZE);
+	int file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
+	int mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
+	int start_index = target_page->index & ~mask;
+	int end_index = start_index | mask;
+	int pages, res = -ENOMEM;
+	struct squashfs_page_actor *actor;
 
-		if (page[n] == NULL)
-			continue;
+	if (end_index > file_end)
+		end_index = file_end;
+	pages = end_index - start_index + 1;
 
-		pageaddr = kmap_atomic(page[n]);
-		squashfs_copy_data(pageaddr, buffer, offset, avail);
-		memset(pageaddr + avail, 0, PAGE_SIZE - avail);
-		kunmap_atomic(pageaddr);
-		flush_dcache_page(page[n]);
-		SetPageUptodate(page[n]);
-		unlock_page(page[n]);
-		if (page[n] != target_page)
-			put_page(page[n]);
-	}
+	actor = actor_from_page_cache(target_page, start_index, pages);
+
+	if (!actor)
+		return -ENOMEM;
 
-out:
-	squashfs_cache_put(buffer);
-	return res;
+	get_page(target_page);
+	res = squashfs_read_data_async(inode->i_sb, block, bsize, NULL,
+				       actor);
+	return res < 0 ? res : 0;
 }
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 887d6d270080..66d7194563b6 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -28,8 +28,12 @@
 #define WARNING(s, args...)	pr_warn("SQUASHFS: "s, ## args)
 
 /* block.c */
+extern int squashfs_init_read_wq(void);
+extern void squashfs_destroy_read_wq(void);
 extern int squashfs_read_data(struct super_block *, u64, int, u64 *,
 				struct squashfs_page_actor *);
+extern int squashfs_read_data_async(struct super_block *, u64, int, u64 *,
+				struct squashfs_page_actor *);
 
 /* cache.c */
 extern struct squashfs_cache *squashfs_cache_init(char *, int, int);
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index cf01e15a7b16..e2a0a7342bf8 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -444,9 +444,15 @@ static int __init init_squashfs_fs(void)
 	if (err)
 		return err;
 
+	if (!squashfs_init_read_wq()) {
+		destroy_inodecache();
+		return -ENOMEM;
+	}
+
 	err = register_filesystem(&squashfs_fs_type);
 	if (err) {
 		destroy_inodecache();
+		squashfs_destroy_read_wq();
 		return err;
 	}
 
@@ -460,6 +466,7 @@ static void __exit exit_squashfs_fs(void)
 {
 	unregister_filesystem(&squashfs_fs_type);
 	destroy_inodecache();
+	squashfs_destroy_read_wq();
 }
 
 
-- 
2.14.1.821.g8fa685d3b7-goog

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] Squashfs: implement .readpages()
  2017-09-22 21:55 [PATCH 0/5] Squashfs Whitelist and Compression Threshold Daniel Rosenberg
                   ` (2 preceding siblings ...)
  2017-09-22 21:55 ` [PATCH 3/5] Squashfs: replace buffer_head with BIO Daniel Rosenberg
@ 2017-09-22 21:55 ` Daniel Rosenberg
  2017-09-22 21:55 ` [PATCH 5/5] Squashfs: optimize reading uncompressed data Daniel Rosenberg
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel Rosenberg @ 2017-09-22 21:55 UTC (permalink / raw)
  To: Phillip Lougher, linux-kernel
  Cc: adrien, Adrien Schildknecht, Adrien Schildknecht, Daniel Rosenberg

From: Adrien Schildknecht <adrien+dev@schischi.me>

Squashfs does not implement .readpages(), so the kernel just repeatedly
calls .readpage().

The readpages function tries to pack as much pages as possible in the
same page actor so that only 1 read request is issued.

Now that the read requests are asynchronous, the kernel can truly
prefetch pages using its readahead algorithm.

Signed-off-by: Adrien Schildknecht <adriens@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/squashfs/file.c        | 137 ++++++++++++++++++++++++++++++++++------------
 fs/squashfs/file_direct.c |  62 ++++++++++++++-------
 fs/squashfs/squashfs.h    |   5 +-
 3 files changed, 146 insertions(+), 58 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 13d80947bf9e..bb2e77ee4209 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -47,6 +47,7 @@
 #include <linux/string.h>
 #include <linux/pagemap.h>
 #include <linux/mutex.h>
+#include <linux/mm_inline.h>
 
 #include "squashfs_fs.h"
 #include "squashfs_fs_sb.h"
@@ -438,6 +439,21 @@ static int squashfs_readpage_fragment(struct page *page)
 	return res;
 }
 
+static int squashfs_readpages_fragment(struct page *page,
+	struct list_head *readahead_pages, struct address_space *mapping)
+{
+	if (!page) {
+		page = lru_to_page(readahead_pages);
+		list_del(&page->lru);
+		if (add_to_page_cache_lru(page, mapping, page->index,
+			mapping_gfp_constraint(mapping, GFP_KERNEL))) {
+			put_page(page);
+			return 0;
+		}
+	}
+	return squashfs_readpage_fragment(page);
+}
+
 static int squashfs_readpage_sparse(struct page *page, int index, int file_end)
 {
 	struct inode *inode = page->mapping->host;
@@ -450,54 +466,105 @@ static int squashfs_readpage_sparse(struct page *page, int index, int file_end)
 	return 0;
 }
 
-static int squashfs_readpage(struct file *file, struct page *page)
+static int squashfs_readpages_sparse(struct page *page,
+	struct list_head *readahead_pages, int index, int file_end,
+	struct address_space *mapping)
 {
-	struct inode *inode = page->mapping->host;
+	if (!page) {
+		page = lru_to_page(readahead_pages);
+		list_del(&page->lru);
+		if (add_to_page_cache_lru(page, mapping, page->index,
+			mapping_gfp_constraint(mapping, GFP_KERNEL))) {
+			put_page(page);
+			return 0;
+		}
+	}
+	return squashfs_readpage_sparse(page, index, file_end);
+}
+
+static int __squashfs_readpages(struct file *file, struct page *page,
+	struct list_head *readahead_pages, unsigned int nr_pages,
+	struct address_space *mapping)
+{
+	struct inode *inode = mapping->host;
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
-	int index = page->index >> (msblk->block_log - PAGE_SHIFT);
 	int file_end = i_size_read(inode) >> msblk->block_log;
 	int res;
-	void *pageaddr;
 
-	TRACE("Entered squashfs_readpage, page index %lx, start block %llx\n",
-				page->index, squashfs_i(inode)->start);
+	do {
+		struct page *cur_page = page ? page
+					     : lru_to_page(readahead_pages);
+		int page_index = cur_page->index;
+		int index = page_index >> (msblk->block_log - PAGE_SHIFT);
+
+		if (page_index >= ((i_size_read(inode) + PAGE_SIZE - 1) >>
+						PAGE_SHIFT))
+			return 1;
+
+		if (index < file_end || squashfs_i(inode)->fragment_block ==
+						SQUASHFS_INVALID_BLK) {
+			u64 block = 0;
+			int bsize = read_blocklist(inode, index, &block);
+
+			if (bsize < 0)
+				return -1;
+
+			if (bsize == 0) {
+				res = squashfs_readpages_sparse(page,
+					readahead_pages, index, file_end,
+					mapping);
+			} else {
+				res = squashfs_readpages_block(page,
+					readahead_pages, &nr_pages, mapping,
+					page_index, block, bsize);
+			}
+		} else {
+			res = squashfs_readpages_fragment(page,
+				readahead_pages, mapping);
+		}
+		if (res)
+			return 0;
+		page = NULL;
+	} while (readahead_pages && !list_empty(readahead_pages));
+
+	return 0;
+}
+
+static int squashfs_readpage(struct file *file, struct page *page)
+{
+	int ret;
 
-	if (page->index >= ((i_size_read(inode) + PAGE_SIZE - 1) >>
-					PAGE_SHIFT))
-		goto out;
+	TRACE("Entered squashfs_readpage, page index %lx, start block %llx\n",
+	      page->index, squashfs_i(page->mapping->host)->start);
 
-	if (index < file_end || squashfs_i(inode)->fragment_block ==
-					SQUASHFS_INVALID_BLK) {
-		u64 block = 0;
-		int bsize = read_blocklist(inode, index, &block);
-		if (bsize < 0)
-			goto error_out;
+	get_page(page);
 
-		if (bsize == 0)
-			res = squashfs_readpage_sparse(page, index, file_end);
+	ret = __squashfs_readpages(file, page, NULL, 1, page->mapping);
+	if (ret) {
+		flush_dcache_page(page);
+		if (ret < 0)
+			SetPageError(page);
 		else
-			res = squashfs_readpage_block(page, block, bsize);
-	} else
-		res = squashfs_readpage_fragment(page);
-
-	if (!res)
-		return 0;
-
-error_out:
-	SetPageError(page);
-out:
-	pageaddr = kmap_atomic(page);
-	memset(pageaddr, 0, PAGE_SIZE);
-	kunmap_atomic(pageaddr);
-	flush_dcache_page(page);
-	if (!PageError(page))
-		SetPageUptodate(page);
-	unlock_page(page);
+			SetPageUptodate(page);
+		zero_user_segment(page, 0, PAGE_SIZE);
+		unlock_page(page);
+		put_page(page);
+	}
 
 	return 0;
 }
 
+static int squashfs_readpages(struct file *file, struct address_space *mapping,
+			      struct list_head *pages, unsigned int nr_pages)
+{
+	TRACE("Entered squashfs_readpages, %u pages, first page index %lx\n",
+		nr_pages, lru_to_page(pages)->index);
+	__squashfs_readpages(file, NULL, pages, nr_pages, mapping);
+	return 0;
+}
+
 
 const struct address_space_operations squashfs_aops = {
-	.readpage = squashfs_readpage
+	.readpage = squashfs_readpage,
+	.readpages = squashfs_readpages,
 };
diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
index 74b8f381320b..a978811de327 100644
--- a/fs/squashfs/file_direct.c
+++ b/fs/squashfs/file_direct.c
@@ -13,6 +13,7 @@
 #include <linux/string.h>
 #include <linux/pagemap.h>
 #include <linux/mutex.h>
+#include <linux/mm_inline.h>
 
 #include "squashfs_fs.h"
 #include "squashfs_fs_sb.h"
@@ -45,23 +46,40 @@ static void release_actor_pages(struct page **page, int pages, int error)
  * page cache pages appropriately within the decompressor
  */
 static struct squashfs_page_actor *actor_from_page_cache(
-	struct page *target_page, int start_index, int nr_pages)
+	unsigned int actor_pages, struct page *target_page,
+	struct list_head *rpages, unsigned int *nr_pages, int start_index,
+	struct address_space *mapping)
 {
-	int i, n;
 	struct page **page;
 	struct squashfs_page_actor *actor;
+	int i, n;
+	gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
 
-	page = kmalloc_array(nr_pages, sizeof(void *), GFP_KERNEL);
+	page = kmalloc_array(actor_pages, sizeof(void *), GFP_KERNEL);
 	if (!page)
 		return NULL;
 
-	/* Try to grab all the pages covered by the SquashFS block */
-	for (i = 0, n = start_index; i < nr_pages; i++, n++) {
-		if (target_page->index == n) {
+	for (i = 0, n = start_index; i < actor_pages; i++, n++) {
+		if (target_page == NULL && rpages && !list_empty(rpages)) {
+			struct page *cur_page = lru_to_page(rpages);
+
+			if (cur_page->index < start_index + actor_pages) {
+				list_del(&cur_page->lru);
+				--(*nr_pages);
+				if (add_to_page_cache_lru(cur_page, mapping,
+							  cur_page->index, gfp))
+					put_page(cur_page);
+				else
+					target_page = cur_page;
+			} else
+				rpages = NULL;
+		}
+
+		if (target_page && target_page->index == n) {
 			page[i] = target_page;
+			target_page = NULL;
 		} else {
-			page[i] = grab_cache_page_nowait(target_page->mapping,
-							 n);
+			page[i] = grab_cache_page_nowait(mapping, n);
 			if (page[i] == NULL)
 				continue;
 		}
@@ -73,40 +91,42 @@ static struct squashfs_page_actor *actor_from_page_cache(
 		}
 	}
 
-	actor = squashfs_page_actor_init(page, nr_pages, 0,
+	actor = squashfs_page_actor_init(page, actor_pages, 0,
 			release_actor_pages);
 	if (!actor) {
-		release_actor_pages(page, nr_pages, -ENOMEM);
+		release_actor_pages(page, actor_pages, -ENOMEM);
 		kfree(page);
 		return NULL;
 	}
 	return actor;
 }
 
-/* Read separately compressed datablock directly into page cache */
-int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
+int squashfs_readpages_block(struct page *target_page,
+			     struct list_head *readahead_pages,
+			     unsigned int *nr_pages,
+			     struct address_space *mapping,
+			     int page_index, u64 block, int bsize)
 
 {
-	struct inode *inode = target_page->mapping->host;
+	struct squashfs_page_actor *actor;
+	struct inode *inode = mapping->host;
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
-
 	int file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
 	int mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
-	int start_index = target_page->index & ~mask;
+	int start_index = page_index & ~mask;
 	int end_index = start_index | mask;
-	int pages, res = -ENOMEM;
-	struct squashfs_page_actor *actor;
+	int actor_pages, res;
 
 	if (end_index > file_end)
 		end_index = file_end;
-	pages = end_index - start_index + 1;
-
-	actor = actor_from_page_cache(target_page, start_index, pages);
+	actor_pages = end_index - start_index + 1;
 
+	actor = actor_from_page_cache(actor_pages, target_page,
+				      readahead_pages, nr_pages, start_index,
+				      mapping);
 	if (!actor)
 		return -ENOMEM;
 
-	get_page(target_page);
 	res = squashfs_read_data_async(inode->i_sb, block, bsize, NULL,
 				       actor);
 	return res < 0 ? res : 0;
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 66d7194563b6..f4faab52a879 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -74,8 +74,9 @@ extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
 void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
 				int);
 
-/* file_xxx.c */
-extern int squashfs_readpage_block(struct page *, u64, int);
+/* file_direct.c */
+extern int squashfs_readpages_block(struct page *, struct list_head *,
+	unsigned int *, struct address_space *, int, u64, int);
 
 /* id.c */
 extern int squashfs_get_id(struct super_block *, unsigned int, unsigned int *);
-- 
2.14.1.821.g8fa685d3b7-goog

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] Squashfs: optimize reading uncompressed data
  2017-09-22 21:55 [PATCH 0/5] Squashfs Whitelist and Compression Threshold Daniel Rosenberg
                   ` (3 preceding siblings ...)
  2017-09-22 21:55 ` [PATCH 4/5] Squashfs: implement .readpages() Daniel Rosenberg
@ 2017-09-22 21:55 ` Daniel Rosenberg
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel Rosenberg @ 2017-09-22 21:55 UTC (permalink / raw)
  To: Phillip Lougher, linux-kernel
  Cc: adrien, Adrien Schildknecht, Daniel Rosenberg

From: Adrien Schildknecht <adriens@google.com>

When dealing with uncompressed data, there is no need to read a whole
block (default 128K) to get the desired page: the pages are
independent from each others.

This patch change the readpages logic so that reading uncompressed
data only read the number of pages advised by the readahead algorithm.

Moreover, if the page actor contains holes (i.e. pages that are already
up-to-date), squashfs skips the buffer_head associated to those pages.

This patch greatly improve the performance of random reads for
uncompressed files because squashfs only read what is needed. It also
reduces the number of unnecessary reads.

Signed-off-by: Adrien Schildknecht <adriens@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/squashfs/block.c       | 25 +++++++++++++++++++++++++
 fs/squashfs/file_direct.c | 37 ++++++++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 252dfc82ae72..37658b6e83ee 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -207,6 +207,22 @@ static void squashfs_bio_end_io(struct bio *bio)
 	kfree(bio_req);
 }
 
+static int bh_is_optional(struct squashfs_read_request *req, int idx)
+{
+	int start_idx, end_idx;
+	struct squashfs_sb_info *msblk = req->sb->s_fs_info;
+
+	start_idx = (idx * msblk->devblksize - req->offset) >> PAGE_SHIFT;
+	end_idx = ((idx + 1) * msblk->devblksize - req->offset + 1) >> PAGE_SHIFT;
+	if (start_idx >= req->output->pages)
+		return 1;
+	if (start_idx < 0)
+		start_idx = end_idx;
+	if (end_idx >= req->output->pages)
+		end_idx = start_idx;
+	return !req->output->page[start_idx] && !req->output->page[end_idx];
+}
+
 static int actor_getblks(struct squashfs_read_request *req, u64 block)
 {
 	int i;
@@ -216,6 +232,15 @@ static int actor_getblks(struct squashfs_read_request *req, u64 block)
 		return -ENOMEM;
 
 	for (i = 0; i < req->nr_buffers; ++i) {
+		/*
+		 * When dealing with an uncompressed block, the actor may
+		 * contains NULL pages. There's no need to read the buffers
+		 * associated with these pages.
+		 */
+		if (!req->compressed && bh_is_optional(req, i)) {
+			req->bh[i] = NULL;
+			continue;
+		}
 		req->bh[i] = sb_getblk(req->sb, block + i);
 		if (!req->bh[i]) {
 			while (--i) {
diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
index a978811de327..dc87f77ce11e 100644
--- a/fs/squashfs/file_direct.c
+++ b/fs/squashfs/file_direct.c
@@ -111,15 +111,38 @@ int squashfs_readpages_block(struct page *target_page,
 	struct squashfs_page_actor *actor;
 	struct inode *inode = mapping->host;
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
-	int file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
+	int start_index, end_index, file_end, actor_pages, res;
 	int mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
-	int start_index = page_index & ~mask;
-	int end_index = start_index | mask;
-	int actor_pages, res;
 
-	if (end_index > file_end)
-		end_index = file_end;
-	actor_pages = end_index - start_index + 1;
+	/*
+	 * If readpage() is called on an uncompressed datablock, we can just
+	 * read the pages instead of fetching the whole block.
+	 * This greatly improves the performance when a process keep doing
+	 * random reads because we only fetch the necessary data.
+	 * The readahead algorithm will take care of doing speculative reads
+	 * if necessary.
+	 * We can't read more than 1 block even if readahead provides use more
+	 * pages because we don't know yet if the next block is compressed or
+	 * not.
+	 */
+	if (bsize && !SQUASHFS_COMPRESSED_BLOCK(bsize)) {
+		u64 block_end = block + msblk->block_size;
+
+		block += (page_index & mask) * PAGE_SIZE;
+		actor_pages = (block_end - block) / PAGE_SIZE;
+		if (*nr_pages < actor_pages)
+			actor_pages = *nr_pages;
+		start_index = page_index;
+		bsize = min_t(int, bsize, (PAGE_SIZE * actor_pages)
+					  | SQUASHFS_COMPRESSED_BIT_BLOCK);
+	} else {
+		file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
+		start_index = page_index & ~mask;
+		end_index = start_index | mask;
+		if (end_index > file_end)
+			end_index = file_end;
+		actor_pages = end_index - start_index + 1;
+	}
 
 	actor = actor_from_page_cache(actor_pages, target_page,
 				      readahead_pages, nr_pages, start_index,
-- 
2.14.1.821.g8fa685d3b7-goog

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/5] Squashfs: remove the FILE_CACHE option
  2017-09-22 21:55 ` [PATCH 1/5] Squashfs: remove the FILE_CACHE option Daniel Rosenberg
@ 2017-09-23  2:01   ` Phillip Lougher
  0 siblings, 0 replies; 7+ messages in thread
From: Phillip Lougher @ 2017-09-23  2:01 UTC (permalink / raw)
  To: Daniel Rosenberg, LKML; +Cc: Phillip Lougher, adrien, Adrien Schildknecht

On Fri, Sep 22, 2017 at 10:55 PM, Daniel Rosenberg <drosen@google.com> wrote:
> From: Adrien Schildknecht <adriens@google.com>
>
> FILE_DIRECT is working fine and offers faster results and lower memory
> footprint.
>
> Removing FILE_CACHE makes our life easier because we don't have to
> maintain 2 differents function that does the same thing.

When I added FILE_DIRECT, I deliberately retained the original
FILE_CACHE behaviour and kept it as default, and I spent a lot of
effort  to do so.  It basically required complete rewriting to enable
FILE_CACHE and FILE_DIRECT to co-exist.  FILE_CACHE wasn't simply some
old cruft I left in there because I couldn't be bothered to remove it.

There is a good reason for keeping the FILE_CACHE behaviour.  While
FILE_DIRECT improves the performance of Squashfs by eliminating a
memcpy, and removing contention on a lock, it also increases the
amount of I/O and CPU Squashfs can use at any one time (by enabling
multiple Squashfs operations to run in parallel).   This changes
scheduling and can take resources away from other tasks.

Who knows how many low-end embedded systems are out there that rely on
the original slower behaviour of FILE_CACHE and will break if it is
removed.

I didn't want to remove the FILE_CACHE behaviour and get a lot of
people complaining (and quite rightly too) that I'd removed a feature
they were relying on, and I don't intend to now either.  You quite
simply don't remove a feature that people may be relying on.

Let's be clear.  Your reason for removing FILE_CACHE is simply because
when adding your new whizzo feature, you needed to update both the
FILE_CACHE and FILE_DIRECT code, and as *you* didn't use FILE_CACHE,
you couldn't be bothered to update it, and removed it instead.  This
is not a valid reason for removing it.

NACK

Phillip

>
> Signed-off-by: Adrien Schildknecht <adriens@google.com>
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> ---
>  fs/squashfs/Kconfig      | 28 ----------------------------
>  fs/squashfs/Makefile     |  3 +--
>  fs/squashfs/file_cache.c | 38 --------------------------------------
>  fs/squashfs/page_actor.h | 42 +-----------------------------------------
>  4 files changed, 2 insertions(+), 109 deletions(-)
>  delete mode 100644 fs/squashfs/file_cache.c
>
> diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
> index 1adb3346b9d6..6c81bf620067 100644
> --- a/fs/squashfs/Kconfig
> +++ b/fs/squashfs/Kconfig
> @@ -25,34 +25,6 @@ config SQUASHFS
>
>           If unsure, say N.
>
> -choice
> -       prompt "File decompression options"
> -       depends on SQUASHFS
> -       help
> -         Squashfs now supports two options for decompressing file
> -         data.  Traditionally Squashfs has decompressed into an
> -         intermediate buffer and then memcopied it into the page cache.
> -         Squashfs now supports the ability to decompress directly into
> -         the page cache.
> -
> -         If unsure, select "Decompress file data into an intermediate buffer"
> -
> -config SQUASHFS_FILE_CACHE
> -       bool "Decompress file data into an intermediate buffer"
> -       help
> -         Decompress file data into an intermediate buffer and then
> -         memcopy it into the page cache.
> -
> -config SQUASHFS_FILE_DIRECT
> -       bool "Decompress files directly into the page cache"
> -       help
> -         Directly decompress file data into the page cache.
> -         Doing so can significantly improve performance because
> -         it eliminates a memcpy and it also removes the lock contention
> -         on the single buffer.
> -
> -endchoice
> -
>  choice
>         prompt "Decompressor parallelisation options"
>         depends on SQUASHFS
> diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
> index 6655631c53ae..225330ab7723 100644
> --- a/fs/squashfs/Makefile
> +++ b/fs/squashfs/Makefile
> @@ -5,8 +5,7 @@
>  obj-$(CONFIG_SQUASHFS) += squashfs.o
>  squashfs-y += block.o cache.o dir.o export.o file.o fragment.o id.o inode.o
>  squashfs-y += namei.o super.o symlink.o decompressor.o
> -squashfs-$(CONFIG_SQUASHFS_FILE_CACHE) += file_cache.o
> -squashfs-$(CONFIG_SQUASHFS_FILE_DIRECT) += file_direct.o page_actor.o
> +squashfs-y += file_direct.o page_actor.o
>  squashfs-$(CONFIG_SQUASHFS_DECOMP_SINGLE) += decompressor_single.o
>  squashfs-$(CONFIG_SQUASHFS_DECOMP_MULTI) += decompressor_multi.o
>  squashfs-$(CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU) += decompressor_multi_percpu.o
> diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
> deleted file mode 100644
> index f2310d2a2019..000000000000
> --- a/fs/squashfs/file_cache.c
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -/*
> - * Copyright (c) 2013
> - * Phillip Lougher <phillip@squashfs.org.uk>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2. See
> - * the COPYING file in the top-level directory.
> - */
> -
> -#include <linux/fs.h>
> -#include <linux/vfs.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/string.h>
> -#include <linux/pagemap.h>
> -#include <linux/mutex.h>
> -
> -#include "squashfs_fs.h"
> -#include "squashfs_fs_sb.h"
> -#include "squashfs_fs_i.h"
> -#include "squashfs.h"
> -
> -/* Read separately compressed datablock and memcopy into page cache */
> -int squashfs_readpage_block(struct page *page, u64 block, int bsize)
> -{
> -       struct inode *i = page->mapping->host;
> -       struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
> -               block, bsize);
> -       int res = buffer->error;
> -
> -       if (res)
> -               ERROR("Unable to read page, block %llx, size %x\n", block,
> -                       bsize);
> -       else
> -               squashfs_copy_cache(page, buffer, buffer->length, 0);
> -
> -       squashfs_cache_put(buffer);
> -       return res;
> -}
> diff --git a/fs/squashfs/page_actor.h b/fs/squashfs/page_actor.h
> index 98537eab27e2..d2df0544e0df 100644
> --- a/fs/squashfs/page_actor.h
> +++ b/fs/squashfs/page_actor.h
> @@ -8,46 +8,6 @@
>   * the COPYING file in the top-level directory.
>   */
>
> -#ifndef CONFIG_SQUASHFS_FILE_DIRECT
> -struct squashfs_page_actor {
> -       void    **page;
> -       int     pages;
> -       int     length;
> -       int     next_page;
> -};
> -
> -static inline struct squashfs_page_actor *squashfs_page_actor_init(void **page,
> -       int pages, int length)
> -{
> -       struct squashfs_page_actor *actor = kmalloc(sizeof(*actor), GFP_KERNEL);
> -
> -       if (actor == NULL)
> -               return NULL;
> -
> -       actor->length = length ? : pages * PAGE_SIZE;
> -       actor->page = page;
> -       actor->pages = pages;
> -       actor->next_page = 0;
> -       return actor;
> -}
> -
> -static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
> -{
> -       actor->next_page = 1;
> -       return actor->page[0];
> -}
> -
> -static inline void *squashfs_next_page(struct squashfs_page_actor *actor)
> -{
> -       return actor->next_page == actor->pages ? NULL :
> -               actor->page[actor->next_page++];
> -}
> -
> -static inline void squashfs_finish_page(struct squashfs_page_actor *actor)
> -{
> -       /* empty */
> -}
> -#else
>  struct squashfs_page_actor {
>         union {
>                 void            **buffer;
> @@ -77,5 +37,5 @@ static inline void squashfs_finish_page(struct squashfs_page_actor *actor)
>  {
>         actor->squashfs_finish_page(actor);
>  }
> -#endif
> +
>  #endif
> --
> 2.14.1.821.g8fa685d3b7-goog
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-09-23  2:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22 21:55 [PATCH 0/5] Squashfs Whitelist and Compression Threshold Daniel Rosenberg
2017-09-22 21:55 ` [PATCH 1/5] Squashfs: remove the FILE_CACHE option Daniel Rosenberg
2017-09-23  2:01   ` Phillip Lougher
2017-09-22 21:55 ` [PATCH 2/5] Squashfs: refactor page_actor Daniel Rosenberg
2017-09-22 21:55 ` [PATCH 3/5] Squashfs: replace buffer_head with BIO Daniel Rosenberg
2017-09-22 21:55 ` [PATCH 4/5] Squashfs: implement .readpages() Daniel Rosenberg
2017-09-22 21:55 ` [PATCH 5/5] Squashfs: optimize reading uncompressed data Daniel Rosenberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.