2022-06-17 08:42:06

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v7 0/4] Implement readahead for squashfs

Commit 9eec1d897139("squashfs: provide backing_dev_info in order to
disable read-ahead") mitigates the performance drop issue for squashfs
by closing readahead for it.

This series implements readahead callback for squashfs. The previous
discussions are in [1] and [2].

[1]
https://lore.kernel.org/all/CAJMQK-g9G6KQmH-V=BRGX0swZji9Wxe_2c7ht-MMAapdFy2pXw@mail.gmail.com/T/
[2]
https://lore.kernel.org/linux-mm/[email protected]/t/#m4af4473b94f98a4996cb11756b633a07e5e059d1

v7 adds the patch to support reading fragments in readahead call[3]. No
changes on other patches since v6, which is included in next-20220617.

[3]
https://patchwork.kernel.org/project/linux-mm/patch/[email protected]/

Hsin-Yi Wang (2):
Revert "squashfs: provide backing_dev_info in order to disable
read-ahead"
squashfs: implement readahead

Phillip Lougher (2):
squashfs: always build "file direct" version of page actor
squashfs: support reading fragments in readahead call

fs/squashfs/Makefile | 4 +-
fs/squashfs/file.c | 133 ++++++++++++++++++++++++++++++++++++++-
fs/squashfs/page_actor.h | 46 --------------
fs/squashfs/super.c | 33 ----------
4 files changed, 134 insertions(+), 82 deletions(-)

--
2.36.1.476.g0c4daa206d-goog


2022-06-17 08:42:30

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v7 1/4] Revert "squashfs: provide backing_dev_info in order to disable read-ahead"

This reverts commit 9eec1d897139e5de287af5d559a02b811b844d82.

Revert closing the readahead to squashfs since the readahead callback
for squashfs is implemented.

Suggested-by: Xiongwei Song <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
fs/squashfs/super.c | 33 ---------------------------------
1 file changed, 33 deletions(-)

diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index 6d594ba2ed28f..32565dafa7f3b 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -29,7 +29,6 @@
#include <linux/module.h>
#include <linux/magic.h>
#include <linux/xattr.h>
-#include <linux/backing-dev.h>

#include "squashfs_fs.h"
#include "squashfs_fs_sb.h"
@@ -113,24 +112,6 @@ static const struct squashfs_decompressor *supported_squashfs_filesystem(
return decompressor;
}

-static int squashfs_bdi_init(struct super_block *sb)
-{
- int err;
- unsigned int major = MAJOR(sb->s_dev);
- unsigned int minor = MINOR(sb->s_dev);
-
- bdi_put(sb->s_bdi);
- sb->s_bdi = &noop_backing_dev_info;
-
- err = super_setup_bdi_name(sb, "squashfs_%u_%u", major, minor);
- if (err)
- return err;
-
- sb->s_bdi->ra_pages = 0;
- sb->s_bdi->io_pages = 0;
-
- return 0;
-}

static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
@@ -146,20 +127,6 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)

TRACE("Entered squashfs_fill_superblock\n");

- /*
- * squashfs provides 'backing_dev_info' in order to disable read-ahead. For
- * squashfs, I/O is not deferred, it is done immediately in read_folio,
- * which means the user would always have to wait their own I/O. So the effect
- * of readahead is very weak for squashfs. squashfs_bdi_init will set
- * sb->s_bdi->ra_pages and sb->s_bdi->io_pages to 0 and close readahead for
- * squashfs.
- */
- err = squashfs_bdi_init(sb);
- if (err) {
- errorf(fc, "squashfs init bdi failed");
- return err;
- }
-
sb->s_fs_info = kzalloc(sizeof(*msblk), GFP_KERNEL);
if (sb->s_fs_info == NULL) {
ERROR("Failed to allocate squashfs_sb_info\n");
--
2.36.1.476.g0c4daa206d-goog

2022-06-17 08:43:19

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v7 3/4] squashfs: implement readahead

Implement readahead callback for squashfs. It will read datablocks
which cover pages in readahead request. For a few cases it will
not mark page as uptodate, including:
- file end is 0.
- zero filled blocks.
- current batch of pages isn't in the same datablock.
- decompressor error.
Otherwise pages will be marked as uptodate. The unhandled pages will be
updated by readpage later.

Suggested-by: Matthew Wilcox <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
Reported-by: Matthew Wilcox <[email protected]>
Reported-by: Phillip Lougher <[email protected]>
Reported-by: Xiongwei Song <[email protected]>
Reported-by: Andrew Morton <[email protected]>
---
fs/squashfs/file.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 7f0904b203294..f0c64ee272d5d 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -39,6 +39,7 @@
#include "squashfs_fs_sb.h"
#include "squashfs_fs_i.h"
#include "squashfs.h"
+#include "page_actor.h"

/*
* Locate cache slot in range [offset, index] for specified inode. If
@@ -496,7 +497,96 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
return res;
}

+static void squashfs_readahead(struct readahead_control *ractl)
+{
+ struct inode *inode = ractl->mapping->host;
+ struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
+ size_t mask = (1UL << msblk->block_log) - 1;
+ unsigned short shift = msblk->block_log - PAGE_SHIFT;
+ loff_t start = readahead_pos(ractl) & ~mask;
+ size_t len = readahead_length(ractl) + readahead_pos(ractl) - start;
+ struct squashfs_page_actor *actor;
+ unsigned int nr_pages = 0;
+ struct page **pages;
+ int i, file_end = i_size_read(inode) >> msblk->block_log;
+ unsigned int max_pages = 1UL << shift;
+
+ readahead_expand(ractl, start, (len | mask) + 1);
+
+ if (file_end == 0)
+ return;
+
+ pages = kmalloc_array(max_pages, sizeof(void *), GFP_KERNEL);
+ if (!pages)
+ return;
+
+ for (;;) {
+ pgoff_t index;
+ int res, bsize;
+ u64 block = 0;
+ unsigned int expected;
+
+ nr_pages = __readahead_batch(ractl, pages, max_pages);
+ if (!nr_pages)
+ break;
+
+ if (readahead_pos(ractl) >= i_size_read(inode))
+ goto skip_pages;
+
+ index = pages[0]->index >> shift;
+ if ((pages[nr_pages - 1]->index >> shift) != index)
+ goto skip_pages;
+
+ expected = index == file_end ?
+ (i_size_read(inode) & (msblk->block_size - 1)) :
+ msblk->block_size;
+
+ bsize = read_blocklist(inode, index, &block);
+ if (bsize == 0)
+ goto skip_pages;
+
+ actor = squashfs_page_actor_init_special(msblk, pages, nr_pages,
+ expected);
+ if (!actor)
+ goto skip_pages;
+
+ res = squashfs_read_data(inode->i_sb, block, bsize, NULL, actor);
+
+ kfree(actor);
+
+ if (res == expected) {
+ int bytes;
+
+ /* Last page (if present) may have trailing bytes not filled */
+ bytes = res % PAGE_SIZE;
+ if (pages[nr_pages - 1]->index == file_end && bytes)
+ memzero_page(pages[nr_pages - 1], bytes,
+ PAGE_SIZE - bytes);
+
+ for (i = 0; i < nr_pages; i++) {
+ flush_dcache_page(pages[i]);
+ SetPageUptodate(pages[i]);
+ }
+ }
+
+ for (i = 0; i < nr_pages; i++) {
+ unlock_page(pages[i]);
+ put_page(pages[i]);
+ }
+ }
+
+ kfree(pages);
+ return;
+
+skip_pages:
+ for (i = 0; i < nr_pages; i++) {
+ unlock_page(pages[i]);
+ put_page(pages[i]);
+ }
+ kfree(pages);
+}

const struct address_space_operations squashfs_aops = {
- .read_folio = squashfs_read_folio
+ .read_folio = squashfs_read_folio,
+ .readahead = squashfs_readahead
};
--
2.36.1.476.g0c4daa206d-goog

2022-06-17 09:15:35

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v7 4/4] squashfs: support reading fragments in readahead call

From: Phillip Lougher <[email protected]>

This patch adds a function which can be used to read fragments in the
readahead call.

This function is necessary because filesystems built with the -tailends
(or -always-use-fragments) option may have fragments present which
cannot be currently handled.

Signed-off-by: Phillip Lougher <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
fs/squashfs/file.c | 47 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index f0c64ee272d5d..98e64fec75b77 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -497,6 +497,41 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
return res;
}

+static int squashfs_readahead_fragment(struct page **page,
+ unsigned int pages, unsigned int expected)
+{
+ struct inode *inode = page[0]->mapping->host;
+ struct squashfs_cache_entry *buffer = squashfs_get_fragment(inode->i_sb,
+ squashfs_i(inode)->fragment_block,
+ squashfs_i(inode)->fragment_size);
+ struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
+ unsigned int n, mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
+
+ if (buffer->error)
+ goto out;
+
+ expected += squashfs_i(inode)->fragment_offset;
+
+ for (n = 0; n < pages; n++) {
+ unsigned int base = (page[n]->index & mask) << PAGE_SHIFT;
+ unsigned int offset = base + squashfs_i(inode)->fragment_offset;
+
+ if (expected > offset) {
+ unsigned int avail = min_t(unsigned int, expected -
+ offset, PAGE_SIZE);
+
+ squashfs_fill_page(page[n], buffer, offset, avail);
+ }
+
+ unlock_page(page[n]);
+ put_page(page[n]);
+ }
+
+out:
+ squashfs_cache_put(buffer);
+ return buffer->error;
+}
+
static void squashfs_readahead(struct readahead_control *ractl)
{
struct inode *inode = ractl->mapping->host;
@@ -513,9 +548,6 @@ static void squashfs_readahead(struct readahead_control *ractl)

readahead_expand(ractl, start, (len | mask) + 1);

- if (file_end == 0)
- return;
-
pages = kmalloc_array(max_pages, sizeof(void *), GFP_KERNEL);
if (!pages)
return;
@@ -541,6 +573,15 @@ static void squashfs_readahead(struct readahead_control *ractl)
(i_size_read(inode) & (msblk->block_size - 1)) :
msblk->block_size;

+ if (index == file_end && squashfs_i(inode)->fragment_block !=
+ SQUASHFS_INVALID_BLK) {
+ res = squashfs_readahead_fragment(pages, nr_pages,
+ expected);
+ if (res)
+ goto skip_pages;
+ continue;
+ }
+
bsize = read_blocklist(inode, index, &block);
if (bsize == 0)
goto skip_pages;
--
2.36.1.476.g0c4daa206d-goog

2022-06-17 09:32:06

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v7 2/4] squashfs: always build "file direct" version of page actor

From: Phillip Lougher <[email protected]>

Squashfs_readahead uses the "file direct" version of the page
actor, and so build it unconditionally.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Phillip Lougher <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
fs/squashfs/Makefile | 4 ++--
fs/squashfs/page_actor.h | 46 ----------------------------------------
2 files changed, 2 insertions(+), 48 deletions(-)

diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 7bd9b8b856d0b..477c89a519ee8 100644
--- a/fs/squashfs/Makefile
+++ b/fs/squashfs/Makefile
@@ -5,9 +5,9 @@

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-y += namei.o super.o symlink.o decompressor.o page_actor.o
squashfs-$(CONFIG_SQUASHFS_FILE_CACHE) += file_cache.o
-squashfs-$(CONFIG_SQUASHFS_FILE_DIRECT) += file_direct.o page_actor.o
+squashfs-$(CONFIG_SQUASHFS_FILE_DIRECT) += file_direct.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/page_actor.h b/fs/squashfs/page_actor.h
index 37523c54256fa..24841d28bc0fb 100644
--- a/fs/squashfs/page_actor.h
+++ b/fs/squashfs/page_actor.h
@@ -6,51 +6,6 @@
* Phillip Lougher <[email protected]>
*/

-#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 */
-}
-
-static inline void squashfs_actor_nobuff(struct squashfs_page_actor *actor)
-{
- /* empty */
-}
-#else
struct squashfs_page_actor {
union {
void **buffer;
@@ -91,4 +46,3 @@ static inline void squashfs_actor_nobuff(struct squashfs_page_actor *actor)
actor->alloc_buffer = 0;
}
#endif
-#endif
--
2.36.1.476.g0c4daa206d-goog

2022-06-21 02:57:58

by Phillip Lougher

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] Implement readahead for squashfs

On 17/06/2022 09:38, Hsin-Yi Wang wrote:
> Commit 9eec1d897139("squashfs: provide backing_dev_info in order to
> disable read-ahead") mitigates the performance drop issue for squashfs
> by closing readahead for it.
>
> This series implements readahead callback for squashfs. The previous
> discussions are in [1] and [2].

Patch series looks OK to me.

I will be offline at the Glastonbury festival from Tuesday 21 June to
Monday 27 June.

Thanks

Phillip

>
> [1]
> https://lore.kernel.org/all/CAJMQK-g9G6KQmH-V=BRGX0swZji9Wxe_2c7ht-MMAapdFy2pXw@mail.gmail.com/T/
> [2]
> https://lore.kernel.org/linux-mm/[email protected]/t/#m4af4473b94f98a4996cb11756b633a07e5e059d1
>
> v7 adds the patch to support reading fragments in readahead call[3]. No
> changes on other patches since v6, which is included in next-20220617.
>
> [3]
> https://patchwork.kernel.org/project/linux-mm/patch/[email protected]/
>
> Hsin-Yi Wang (2):
> Revert "squashfs: provide backing_dev_info in order to disable
> read-ahead"
> squashfs: implement readahead
>
> Phillip Lougher (2):
> squashfs: always build "file direct" version of page actor
> squashfs: support reading fragments in readahead call
>
> fs/squashfs/Makefile | 4 +-
> fs/squashfs/file.c | 133 ++++++++++++++++++++++++++++++++++++++-
> fs/squashfs/page_actor.h | 46 --------------
> fs/squashfs/super.c | 33 ----------
> 4 files changed, 134 insertions(+), 82 deletions(-)
>