[PATCH v4 0/3] Implement readahead for squashfs

[PATCH v4 0/3] Implement readahead for squashfs

[Hsin-Yi Wang]

Commit c1f6925e1091("mm: put readahead pages in cache earlier") requires
fs to implement readahead callback. Otherwise there will be a
performance regression.

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
discussion 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/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m4af4473b94f98a4996cb11756b633a07e5e059d1

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

Phillip Lougher (1):
  squashfs: always build "file direct" version of page actor

 fs/squashfs/Makefile     |  4 +-
 fs/squashfs/file.c       | 97 +++++++++++++++++++++++++++++++++++++++-
 fs/squashfs/page_actor.h | 41 -----------------
 fs/squashfs/super.c      | 33 --------------
 4 files changed, 98 insertions(+), 77 deletions(-)

-- 
2.36.1.255.ge46751e96f-goog

[PATCH v4 1/3] Revert "squashfs: provide backing_dev_info in order to disable read-ahead"

[Hsin-Yi Wang]

This reverts commit 9eec1d897139e5de287af5d559a02b811b844d82.

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

Suggested-by: Xiongwei Song <Xiongwei.Song@windriver.com>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 fs/squashfs/super.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index 6d594ba2ed28..32565dafa7f3 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.255.ge46751e96f-goog

[PATCH v4 2/3] squashfs: always build "file direct" version of page actor

[Hsin-Yi Wang]

From: Phillip Lougher <phillip@squashfs.org.uk>

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

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 fs/squashfs/Makefile     |  4 ++--
 fs/squashfs/page_actor.h | 41 ----------------------------------------
 2 files changed, 2 insertions(+), 43 deletions(-)

diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 7bd9b8b856d0..477c89a519ee 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 2e3073ace009..26e07373af8a 100644
--- a/fs/squashfs/page_actor.h
+++ b/fs/squashfs/page_actor.h
@@ -6,46 +6,6 @@
  * Phillip Lougher <phillip@squashfs.org.uk>
  */
 
-#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;
@@ -76,4 +36,3 @@ static inline void squashfs_finish_page(struct squashfs_page_actor *actor)
 	actor->squashfs_finish_page(actor);
 }
 #endif
-#endif
-- 
2.36.1.255.ge46751e96f-goog

[PATCH v4 3/3] squashfs: implement readahead

[Hsin-Yi Wang]

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 or not enough in a
  datablock.
- decompressor error.
Otherwise pages will be marked as uptodate. The unhandled pages will be
updated by readpage later.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Reported-by: Matthew Wilcox <willy@infradead.org>
Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
---
v3->v4: Fix a few variable type and their locations.
v3: https://lore.kernel.org/lkml/20220523065909.883444-4-hsinyi@chromium.org/
v2: https://lore.kernel.org/lkml/20220517082650.2005840-4-hsinyi@chromium.org/
v1: https://lore.kernel.org/lkml/20220516105100.1412740-3-hsinyi@chromium.org/
---
 fs/squashfs/file.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index a8e495d8eb86..df7ad4b3e99c 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
@@ -495,7 +496,101 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 	return 0;
 }
 
+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;
+
+	actor = squashfs_page_actor_init_special(pages, max_pages, 0);
+	if (!actor)
+		goto out;
+
+	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) ||
+		    nr_pages < max_pages)
+			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;
+
+		res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
+					 actor);
+
+		if (res == expected) {
+			int bytes;
+
+			/* Last page may have trailing bytes not filled */
+			bytes = res % PAGE_SIZE;
+			if (bytes) {
+				void *pageaddr;
+
+				pageaddr = kmap_atomic(pages[nr_pages - 1]);
+				memset(pageaddr + bytes, 0, PAGE_SIZE - bytes);
+				kunmap_atomic(pageaddr);
+			}
+
+			for (i = 0; i < nr_pages; i++)
+				SetPageUptodate(pages[i]);
+		}
+
+		for (i = 0; i < nr_pages; i++) {
+			unlock_page(pages[i]);
+			put_page(pages[i]);
+		}
+	}
+
+	kfree(actor);
+	kfree(pages);
+	return;
+
+skip_pages:
+	for (i = 0; i < nr_pages; i++) {
+		unlock_page(pages[i]);
+		put_page(pages[i]);
+	}
+
+	kfree(actor);
+out:
+	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.255.ge46751e96f-goog

Re: [PATCH v4 3/3] squashfs: implement readahead

[Marek Szyprowski]

Hi,

On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> 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 or not enough in a
>    datablock.
> - decompressor error.
> Otherwise pages will be marked as uptodate. The unhandled pages will be
> updated by readpage later.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Reported-by: Matthew Wilcox <willy@infradead.org>
> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> ---

This patch landed recently in linux-next as commit 95f7a26191de 
("squashfs: implement readahead"). I've noticed that it causes serious 
issues on my test systems (various ARM 32bit and 64bit based boards). 
The easiest way to observe is udev timeout 'waiting for /dev to be fully 
populated' and prolonged booting time. I'm using squashfs for deploying 
kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on 
top of the next-20220603 fixes the issue.

Let me know how I can help debugging this issue. There is no hurry 
though, because the next week I will be on holidays.

> v3->v4: Fix a few variable type and their locations.
> v3: https://lore.kernel.org/lkml/20220523065909.883444-4-hsinyi@chromium.org/
> v2: https://lore.kernel.org/lkml/20220517082650.2005840-4-hsinyi@chromium.org/
> v1: https://lore.kernel.org/lkml/20220516105100.1412740-3-hsinyi@chromium.org/
> ---
>   fs/squashfs/file.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index a8e495d8eb86..df7ad4b3e99c 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
> @@ -495,7 +496,101 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   	return 0;
>   }
>   
> +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;
> +
> +	actor = squashfs_page_actor_init_special(pages, max_pages, 0);
> +	if (!actor)
> +		goto out;
> +
> +	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) ||
> +		    nr_pages < max_pages)
> +			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;
> +
> +		res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
> +					 actor);
> +
> +		if (res == expected) {
> +			int bytes;
> +
> +			/* Last page may have trailing bytes not filled */
> +			bytes = res % PAGE_SIZE;
> +			if (bytes) {
> +				void *pageaddr;
> +
> +				pageaddr = kmap_atomic(pages[nr_pages - 1]);
> +				memset(pageaddr + bytes, 0, PAGE_SIZE - bytes);
> +				kunmap_atomic(pageaddr);
> +			}
> +
> +			for (i = 0; i < nr_pages; i++)
> +				SetPageUptodate(pages[i]);
> +		}
> +
> +		for (i = 0; i < nr_pages; i++) {
> +			unlock_page(pages[i]);
> +			put_page(pages[i]);
> +		}
> +	}
> +
> +	kfree(actor);
> +	kfree(pages);
> +	return;
> +
> +skip_pages:
> +	for (i = 0; i < nr_pages; i++) {
> +		unlock_page(pages[i]);
> +		put_page(pages[i]);
> +	}
> +
> +	kfree(actor);
> +out:
> +	kfree(pages);
> +}
>   
>   const struct address_space_operations squashfs_aops = {
> -	.read_folio = squashfs_read_folio
> +	.read_folio = squashfs_read_folio,
> +	.readahead = squashfs_readahead
>   };

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 3/3] squashfs: implement readahead

[Matthew Wilcox]

On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
> Hi,
> 
> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> > 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 or not enough in a
> >    datablock.
> > - decompressor error.
> > Otherwise pages will be marked as uptodate. The unhandled pages will be
> > updated by readpage later.
> >
> > Suggested-by: Matthew Wilcox <willy@infradead.org>
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Reported-by: Matthew Wilcox <willy@infradead.org>
> > Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> > Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> > ---
> 
> This patch landed recently in linux-next as commit 95f7a26191de 
> ("squashfs: implement readahead"). I've noticed that it causes serious 
> issues on my test systems (various ARM 32bit and 64bit based boards). 
> The easiest way to observe is udev timeout 'waiting for /dev to be fully 
> populated' and prolonged booting time. I'm using squashfs for deploying 
> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on 
> top of the next-20220603 fixes the issue.

How large are these files?  Just a few kilobytes?

Re: [PATCH v4 3/3] squashfs: implement readahead

[Marek Szyprowski]

Hi Matthew,

On 03.06.2022 14:59, Matthew Wilcox wrote:
> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
>>> 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 or not enough in a
>>>     datablock.
>>> - decompressor error.
>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
>>> updated by readpage later.
>>>
>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>>> Reported-by: Matthew Wilcox <willy@infradead.org>
>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
>>> ---
>> This patch landed recently in linux-next as commit 95f7a26191de
>> ("squashfs: implement readahead"). I've noticed that it causes serious
>> issues on my test systems (various ARM 32bit and 64bit based boards).
>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
>> populated' and prolonged booting time. I'm using squashfs for deploying
>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
>> top of the next-20220603 fixes the issue.
> How large are these files?  Just a few kilobytes?

Yes, they are small, most of them are smaller than 16KB, some about 
128KB and a few about 256KB. I've sent a detailed list in private mail.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 3/3] squashfs: implement readahead

[Hsin-Yi Wang]

On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> Hi Matthew,
>
> On 03.06.2022 14:59, Matthew Wilcox wrote:
> > On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
> >> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> >>> 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 or not enough in a
> >>>     datablock.
> >>> - decompressor error.
> >>> Otherwise pages will be marked as uptodate. The unhandled pages will be
> >>> updated by readpage later.
> >>>
> >>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> >>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> >>> Reported-by: Matthew Wilcox <willy@infradead.org>
> >>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> >>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> >>> ---
> >> This patch landed recently in linux-next as commit 95f7a26191de
> >> ("squashfs: implement readahead"). I've noticed that it causes serious
> >> issues on my test systems (various ARM 32bit and 64bit based boards).
> >> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> >> populated' and prolonged booting time. I'm using squashfs for deploying
> >> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> >> top of the next-20220603 fixes the issue.
> > How large are these files?  Just a few kilobytes?
>
> Yes, they are small, most of them are smaller than 16KB, some about
> 128KB and a few about 256KB. I've sent a detailed list in private mail.
>

Hi Marek,

Are there any obvious squashfs errors in dmesg? Did you enable
CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?

Thanks

> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Re: [PATCH v4 3/3] squashfs: implement readahead

[Marek Szyprowski]

Hi,

On 03.06.2022 16:55, Hsin-Yi Wang wrote:
> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> On 03.06.2022 14:59, Matthew Wilcox wrote:
>>> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
>>>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
>>>>> 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 or not enough in a
>>>>>      datablock.
>>>>> - decompressor error.
>>>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
>>>>> updated by readpage later.
>>>>>
>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>>>>> Reported-by: Matthew Wilcox <willy@infradead.org>
>>>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
>>>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
>>>>> ---
>>>> This patch landed recently in linux-next as commit 95f7a26191de
>>>> ("squashfs: implement readahead"). I've noticed that it causes serious
>>>> issues on my test systems (various ARM 32bit and 64bit based boards).
>>>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
>>>> populated' and prolonged booting time. I'm using squashfs for deploying
>>>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
>>>> top of the next-20220603 fixes the issue.
>>> How large are these files?  Just a few kilobytes?
>> Yes, they are small, most of them are smaller than 16KB, some about
>> 128KB and a few about 256KB. I've sent a detailed list in private mail.
>>
> Hi Marek,
>
> Are there any obvious squashfs errors in dmesg? Did you enable
> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?

Here are related config entries from my .config:

CONFIG_SQUASHFS=y
CONFIG_SQUASHFS_FILE_CACHE=y
# CONFIG_SQUASHFS_FILE_DIRECT is not set
CONFIG_SQUASHFS_DECOMP_SINGLE=y
# CONFIG_SQUASHFS_DECOMP_MULTI is not set
# CONFIG_SQUASHFS_DECOMP_MULTI_PERCPU is not set
# CONFIG_SQUASHFS_XATTR is not set
CONFIG_SQUASHFS_ZLIB=y
# CONFIG_SQUASHFS_LZ4 is not set
CONFIG_SQUASHFS_LZO=y
CONFIG_SQUASHFS_XZ=y
# CONFIG_SQUASHFS_ZSTD is not set
# CONFIG_SQUASHFS_4K_DEVBLK_SIZE is not set
# CONFIG_SQUASHFS_EMBEDDED is not set
CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE=3

(I've used standard ARM 32bit multi_v7_defconfig)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 3/3] squashfs: implement readahead

[Matthew Wilcox]

On Fri, Jun 03, 2022 at 10:55:01PM +0800, Hsin-Yi Wang wrote:
> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
> >
> > Hi Matthew,
> >
> > On 03.06.2022 14:59, Matthew Wilcox wrote:
> > > On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
> > >> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> > >>> 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 or not enough in a
> > >>>     datablock.
> > >>> - decompressor error.
> > >>> Otherwise pages will be marked as uptodate. The unhandled pages will be
> > >>> updated by readpage later.
> > >>>
> > >>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> > >>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > >>> Reported-by: Matthew Wilcox <willy@infradead.org>
> > >>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> > >>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> > >>> ---
> > >> This patch landed recently in linux-next as commit 95f7a26191de
> > >> ("squashfs: implement readahead"). I've noticed that it causes serious
> > >> issues on my test systems (various ARM 32bit and 64bit based boards).
> > >> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> > >> populated' and prolonged booting time. I'm using squashfs for deploying
> > >> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> > >> top of the next-20220603 fixes the issue.
> > > How large are these files?  Just a few kilobytes?
> >
> > Yes, they are small, most of them are smaller than 16KB, some about
> > 128KB and a few about 256KB. I've sent a detailed list in private mail.
> >
> 
> Hi Marek,
> 
> Are there any obvious squashfs errors in dmesg? Did you enable
> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?

I don't think it's an error problem.  I think it's a short file problem.

As I understand the current code (and apologies for not keeping up
to date with how the patch is progressing), if the file is less than
msblk->block_size bytes, we'll leave all the pages as !uptodate, leaving
them to be brough uptodate by squashfs_read_folio().  So Marek is hitting
the worst case scenario where we re-read the entire block for each page
in it.  I think we have to handle this tail case in ->readahead().

Re: [PATCH v4 3/3] squashfs: implement readahead

[Marek Szyprowski]

Hi Matthew,

On 03.06.2022 17:29, Matthew Wilcox wrote:
> On Fri, Jun 03, 2022 at 10:55:01PM +0800, Hsin-Yi Wang wrote:
>> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>> Hi Matthew,
>>>
>>> On 03.06.2022 14:59, Matthew Wilcox wrote:
>>>> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
>>>>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
>>>>>> 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 or not enough in a
>>>>>>      datablock.
>>>>>> - decompressor error.
>>>>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
>>>>>> updated by readpage later.
>>>>>>
>>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>>>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>>>>>> Reported-by: Matthew Wilcox <willy@infradead.org>
>>>>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
>>>>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
>>>>>> ---
>>>>> This patch landed recently in linux-next as commit 95f7a26191de
>>>>> ("squashfs: implement readahead"). I've noticed that it causes serious
>>>>> issues on my test systems (various ARM 32bit and 64bit based boards).
>>>>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
>>>>> populated' and prolonged booting time. I'm using squashfs for deploying
>>>>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
>>>>> top of the next-20220603 fixes the issue.
>>>> How large are these files?  Just a few kilobytes?
>>> Yes, they are small, most of them are smaller than 16KB, some about
>>> 128KB and a few about 256KB. I've sent a detailed list in private mail.
>>>
>> Hi Marek,
>>
>> Are there any obvious squashfs errors in dmesg? Did you enable
>> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?
> I don't think it's an error problem.  I think it's a short file problem.
>
> As I understand the current code (and apologies for not keeping up
> to date with how the patch is progressing), if the file is less than
> msblk->block_size bytes, we'll leave all the pages as !uptodate, leaving
> them to be brough uptodate by squashfs_read_folio().  So Marek is hitting
> the worst case scenario where we re-read the entire block for each page
> in it.  I think we have to handle this tail case in ->readahead().

I'm not sure if this is related to reading of small files. There are 
only 50 modules being loaded from squashfs volume. I did a quick test of 
reading the files.

Simple file read with this patch:

root@target:~# time find /initrd/ -type f | while read f; do cat $f 
 >/dev/null; done

real    0m5.865s
user    0m2.362s
sys     0m3.844s

Without:

root@target:~# time find /initrd/ -type f | while read f; do cat $f 
 >/dev/null; done

real    0m6.619s
user    0m2.112s
sys     0m4.827s

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 3/3] squashfs: implement readahead

[Phillip Lougher]

On 03/06/2022 16:58, Marek Szyprowski wrote:
> Hi Matthew,
> 
> On 03.06.2022 17:29, Matthew Wilcox wrote:
>> On Fri, Jun 03, 2022 at 10:55:01PM +0800, Hsin-Yi Wang wrote:
>>> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
>>> <m.szyprowski@samsung.com> wrote:
>>>> Hi Matthew,
>>>>
>>>> On 03.06.2022 14:59, Matthew Wilcox wrote:
>>>>> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
>>>>>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
>>>>>>> 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 or not enough in a
>>>>>>>       datablock.
>>>>>>> - decompressor error.
>>>>>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
>>>>>>> updated by readpage later.
>>>>>>>
>>>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>>>>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>>>>>>> Reported-by: Matthew Wilcox <willy@infradead.org>
>>>>>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
>>>>>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
>>>>>>> ---
>>>>>> This patch landed recently in linux-next as commit 95f7a26191de
>>>>>> ("squashfs: implement readahead"). I've noticed that it causes serious
>>>>>> issues on my test systems (various ARM 32bit and 64bit based boards).
>>>>>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
>>>>>> populated' and prolonged booting time. I'm using squashfs for deploying
>>>>>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
>>>>>> top of the next-20220603 fixes the issue.
>>>>> How large are these files?  Just a few kilobytes?
>>>> Yes, they are small, most of them are smaller than 16KB, some about
>>>> 128KB and a few about 256KB. I've sent a detailed list in private mail.
>>>>
>>> Hi Marek,
>>>
>>> Are there any obvious squashfs errors in dmesg? Did you enable
>>> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?
>> I don't think it's an error problem.  I think it's a short file problem.
>>
>> As I understand the current code (and apologies for not keeping up
>> to date with how the patch is progressing), if the file is less than
>> msblk->block_size bytes, we'll leave all the pages as !uptodate, leaving
>> them to be brough uptodate by squashfs_read_folio().  So Marek is hitting
>> the worst case scenario where we re-read the entire block for each page
>> in it.  I think we have to handle this tail case in ->readahead().
> 
> I'm not sure if this is related to reading of small files. There are
> only 50 modules being loaded from squashfs volume. I did a quick test of
> reading the files.
> 
> Simple file read with this patch:
> 
> root@target:~# time find /initrd/ -type f | while read f; do cat $f
>   >/dev/null; done
> 
> real    0m5.865s
> user    0m2.362s
> sys     0m3.844s
> 
> Without:
> 
> root@target:~# time find /initrd/ -type f | while read f; do cat $f
>   >/dev/null; done
> 
> real    0m6.619s
> user    0m2.112s
> sys     0m4.827s
> 

It has been a four day holiday in the UK (Queen's Platinum Jubilee),
hence the delay in responding.

The above read use-case is sequential (only one thread/process),
whereas the use-case where the slow-down is observed may be
parallel (multiple threads/processes entering Squashfs).

The above sequential use-case if the small files are held in
fragments, will be exhibiting caching behaviour that will
ameliorate the case where the same block is being repeatedly
re-read for each page in it.  Because each time
Squashfs is re-entered handling only a single page, the
decompressed block will be found in the fragment
cache, eliminating a block decompression for each page.

In a parallel use-case the decompressed fragment block
may be being eliminated from the cache (by other reading
processes), hence forcing the block to be repeatedly
decompressed.

Hence the slow-down will be much more noticable with a
parallel use-case than a sequential use-case.  It also may
be why this slipped through testing, if the test cases
are purely sequential in nature.

So Matthew's previous comment is still the most likely
explanation for the slow-down.

Phillip

> Best regards

Re: [PATCH v4 3/3] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, Jun 6, 2022 at 11:54 AM Phillip Lougher <phillip@squashfs.org.uk> wrote:
>
> On 03/06/2022 16:58, Marek Szyprowski wrote:
> > Hi Matthew,
> >
> > On 03.06.2022 17:29, Matthew Wilcox wrote:
> >> On Fri, Jun 03, 2022 at 10:55:01PM +0800, Hsin-Yi Wang wrote:
> >>> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
> >>> <m.szyprowski@samsung.com> wrote:
> >>>> Hi Matthew,
> >>>>
> >>>> On 03.06.2022 14:59, Matthew Wilcox wrote:
> >>>>> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
> >>>>>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> >>>>>>> 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 or not enough in a
> >>>>>>>       datablock.
> >>>>>>> - decompressor error.
> >>>>>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
> >>>>>>> updated by readpage later.
> >>>>>>>
> >>>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> >>>>>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> >>>>>>> Reported-by: Matthew Wilcox <willy@infradead.org>
> >>>>>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> >>>>>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> >>>>>>> ---
> >>>>>> This patch landed recently in linux-next as commit 95f7a26191de
> >>>>>> ("squashfs: implement readahead"). I've noticed that it causes serious
> >>>>>> issues on my test systems (various ARM 32bit and 64bit based boards).
> >>>>>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> >>>>>> populated' and prolonged booting time. I'm using squashfs for deploying
> >>>>>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> >>>>>> top of the next-20220603 fixes the issue.
> >>>>> How large are these files?  Just a few kilobytes?
> >>>> Yes, they are small, most of them are smaller than 16KB, some about
> >>>> 128KB and a few about 256KB. I've sent a detailed list in private mail.
> >>>>
> >>> Hi Marek,
> >>>
> >>> Are there any obvious squashfs errors in dmesg? Did you enable
> >>> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?
> >> I don't think it's an error problem.  I think it's a short file problem.
> >>
> >> As I understand the current code (and apologies for not keeping up
> >> to date with how the patch is progressing), if the file is less than
> >> msblk->block_size bytes, we'll leave all the pages as !uptodate, leaving
> >> them to be brough uptodate by squashfs_read_folio().  So Marek is hitting
> >> the worst case scenario where we re-read the entire block for each page
> >> in it.  I think we have to handle this tail case in ->readahead().
> >
> > I'm not sure if this is related to reading of small files. There are
> > only 50 modules being loaded from squashfs volume. I did a quick test of
> > reading the files.
> >
> > Simple file read with this patch:
> >
> > root@target:~# time find /initrd/ -type f | while read f; do cat $f
> >   >/dev/null; done
> >
> > real    0m5.865s
> > user    0m2.362s
> > sys     0m3.844s
> >
> > Without:
> >
> > root@target:~# time find /initrd/ -type f | while read f; do cat $f
> >   >/dev/null; done
> >
> > real    0m6.619s
> > user    0m2.112s
> > sys     0m4.827s
> >
>
> It has been a four day holiday in the UK (Queen's Platinum Jubilee),
> hence the delay in responding.
>
> The above read use-case is sequential (only one thread/process),
> whereas the use-case where the slow-down is observed may be
> parallel (multiple threads/processes entering Squashfs).
>
> The above sequential use-case if the small files are held in
> fragments, will be exhibiting caching behaviour that will
> ameliorate the case where the same block is being repeatedly
> re-read for each page in it.  Because each time
> Squashfs is re-entered handling only a single page, the
> decompressed block will be found in the fragment
> cache, eliminating a block decompression for each page.
>
> In a parallel use-case the decompressed fragment block
> may be being eliminated from the cache (by other reading
> processes), hence forcing the block to be repeatedly
> decompressed.
>
> Hence the slow-down will be much more noticable with a
> parallel use-case than a sequential use-case.  It also may
> be why this slipped through testing, if the test cases
> are purely sequential in nature.
>
> So Matthew's previous comment is still the most likely
> explanation for the slow-down.
>
Thanks for the pointers. To deal with short file cases (nr_pages <
max_pages), Can we refer to squashfs_fill_page() used in
squashfs_read_cache(), similar to the case where there are missing
pages on the block?

Directly calling squashfs_read_data() on short files will lead to crash:

Unable to handle kernel paging request at virtual address:
[   19.244654]  zlib_inflate+0xba4/0x10c8
[   19.244658]  zlib_uncompress+0x150/0x1bc
[   19.244662]  squashfs_decompress+0x6c/0xb4
[   19.244669]  squashfs_read_data+0x1a8/0x298
[   19.244673]  squashfs_readahead+0x2cc/0x4cc

I also noticed that the function didn't set flush_dcache_page() with
SetPageUptodate() previously.

Put these 2 issues together:

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 658fb98af0cd..27519f1f9045 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -532,8 +532,7 @@ static void squashfs_readahead(struct
readahead_control *ractl)
                if (!nr_pages)
                        break;

-               if (readahead_pos(ractl) >= i_size_read(inode) ||
-                   nr_pages < max_pages)
+               if (readahead_pos(ractl) >= i_size_read(inode))
                        goto skip_pages;

                index = pages[0]->index >> shift;
@@ -548,6 +547,23 @@ static void squashfs_readahead(struct
readahead_control *ractl)
                if (bsize == 0)
                        goto skip_pages;

+               if (nr_pages < max_pages) {
+                       struct squashfs_cache_entry *buffer;
+
+                       buffer = squashfs_get_datablock(inode->i_sb, block,
+                                                       bsize);
+                       if (!buffer->error) {
+                               for (i = 0; i < nr_pages && expected > 0; i++,
+                                                       expected -= PAGE_SIZE) {
+                                       int avail = min_t(int,
expected, PAGE_SIZE);
+
+                                       squashfs_fill_page(pages[i],
buffer, i * PAGE_SIZE, avail);
+                               }
+                       }
+                       squashfs_cache_put(buffer);
+                       goto skip_pages;
+               }
+
                res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
                                         actor);

@@ -564,8 +580,10 @@ static void squashfs_readahead(struct
readahead_control *ractl)
                                kunmap_atomic(pageaddr);
                        }

-                       for (i = 0; i < nr_pages; i++)
+                       for (i = 0; i < nr_pages; i++) {
+                               flush_dcache_page(pages[i]);
                                SetPageUptodate(pages[i]);
+                       }
                }


> Phillip
>
> > Best regards
>

Re: [PATCH v4 3/3] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, Jun 6, 2022 at 5:55 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> On Mon, Jun 6, 2022 at 11:54 AM Phillip Lougher <phillip@squashfs.org.uk> wrote:
> >
> > On 03/06/2022 16:58, Marek Szyprowski wrote:
> > > Hi Matthew,
> > >
> > > On 03.06.2022 17:29, Matthew Wilcox wrote:
> > >> On Fri, Jun 03, 2022 at 10:55:01PM +0800, Hsin-Yi Wang wrote:
> > >>> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
> > >>> <m.szyprowski@samsung.com> wrote:
> > >>>> Hi Matthew,
> > >>>>
> > >>>> On 03.06.2022 14:59, Matthew Wilcox wrote:
> > >>>>> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
> > >>>>>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> > >>>>>>> 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 or not enough in a
> > >>>>>>>       datablock.
> > >>>>>>> - decompressor error.
> > >>>>>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
> > >>>>>>> updated by readpage later.
> > >>>>>>>
> > >>>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> > >>>>>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > >>>>>>> Reported-by: Matthew Wilcox <willy@infradead.org>
> > >>>>>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> > >>>>>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> > >>>>>>> ---
> > >>>>>> This patch landed recently in linux-next as commit 95f7a26191de
> > >>>>>> ("squashfs: implement readahead"). I've noticed that it causes serious
> > >>>>>> issues on my test systems (various ARM 32bit and 64bit based boards).
> > >>>>>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> > >>>>>> populated' and prolonged booting time. I'm using squashfs for deploying
> > >>>>>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> > >>>>>> top of the next-20220603 fixes the issue.
> > >>>>> How large are these files?  Just a few kilobytes?
> > >>>> Yes, they are small, most of them are smaller than 16KB, some about
> > >>>> 128KB and a few about 256KB. I've sent a detailed list in private mail.
> > >>>>
> > >>> Hi Marek,
> > >>>
> > >>> Are there any obvious squashfs errors in dmesg? Did you enable
> > >>> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?
> > >> I don't think it's an error problem.  I think it's a short file problem.
> > >>
> > >> As I understand the current code (and apologies for not keeping up
> > >> to date with how the patch is progressing), if the file is less than
> > >> msblk->block_size bytes, we'll leave all the pages as !uptodate, leaving
> > >> them to be brough uptodate by squashfs_read_folio().  So Marek is hitting
> > >> the worst case scenario where we re-read the entire block for each page
> > >> in it.  I think we have to handle this tail case in ->readahead().
> > >
> > > I'm not sure if this is related to reading of small files. There are
> > > only 50 modules being loaded from squashfs volume. I did a quick test of
> > > reading the files.
> > >
> > > Simple file read with this patch:
> > >
> > > root@target:~# time find /initrd/ -type f | while read f; do cat $f
> > >   >/dev/null; done
> > >
> > > real    0m5.865s
> > > user    0m2.362s
> > > sys     0m3.844s
> > >
> > > Without:
> > >
> > > root@target:~# time find /initrd/ -type f | while read f; do cat $f
> > >   >/dev/null; done
> > >
> > > real    0m6.619s
> > > user    0m2.112s
> > > sys     0m4.827s
> > >
> >
> > It has been a four day holiday in the UK (Queen's Platinum Jubilee),
> > hence the delay in responding.
> >
> > The above read use-case is sequential (only one thread/process),
> > whereas the use-case where the slow-down is observed may be
> > parallel (multiple threads/processes entering Squashfs).
> >
> > The above sequential use-case if the small files are held in
> > fragments, will be exhibiting caching behaviour that will
> > ameliorate the case where the same block is being repeatedly
> > re-read for each page in it.  Because each time
> > Squashfs is re-entered handling only a single page, the
> > decompressed block will be found in the fragment
> > cache, eliminating a block decompression for each page.
> >
> > In a parallel use-case the decompressed fragment block
> > may be being eliminated from the cache (by other reading
> > processes), hence forcing the block to be repeatedly
> > decompressed.
> >
> > Hence the slow-down will be much more noticable with a
> > parallel use-case than a sequential use-case.  It also may
> > be why this slipped through testing, if the test cases
> > are purely sequential in nature.
> >
> > So Matthew's previous comment is still the most likely
> > explanation for the slow-down.
> >
> Thanks for the pointers. To deal with short file cases (nr_pages <
> max_pages), Can we refer to squashfs_fill_page() used in
> squashfs_read_cache(), similar to the case where there are missing
> pages on the block?
>
> Directly calling squashfs_read_data() on short files will lead to crash:
>
> Unable to handle kernel paging request at virtual address:
> [   19.244654]  zlib_inflate+0xba4/0x10c8
> [   19.244658]  zlib_uncompress+0x150/0x1bc
> [   19.244662]  squashfs_decompress+0x6c/0xb4
> [   19.244669]  squashfs_read_data+0x1a8/0x298
> [   19.244673]  squashfs_readahead+0x2cc/0x4cc
>
> I also noticed that the function didn't set flush_dcache_page() with
> SetPageUptodate() previously.
>
> Put these 2 issues together:
>

The patch here is not correct. Please ignore it for now. Sorry for the noice.

> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 658fb98af0cd..27519f1f9045 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -532,8 +532,7 @@ static void squashfs_readahead(struct
> readahead_control *ractl)
>                 if (!nr_pages)
>                         break;
>
> -               if (readahead_pos(ractl) >= i_size_read(inode) ||
> -                   nr_pages < max_pages)
> +               if (readahead_pos(ractl) >= i_size_read(inode))
>                         goto skip_pages;
>
>                 index = pages[0]->index >> shift;
> @@ -548,6 +547,23 @@ static void squashfs_readahead(struct
> readahead_control *ractl)
>                 if (bsize == 0)
>                         goto skip_pages;
>
> +               if (nr_pages < max_pages) {
> +                       struct squashfs_cache_entry *buffer;
> +
> +                       buffer = squashfs_get_datablock(inode->i_sb, block,
> +                                                       bsize);
> +                       if (!buffer->error) {
> +                               for (i = 0; i < nr_pages && expected > 0; i++,
> +                                                       expected -= PAGE_SIZE) {
> +                                       int avail = min_t(int,
> expected, PAGE_SIZE);
> +
> +                                       squashfs_fill_page(pages[i],
> buffer, i * PAGE_SIZE, avail);
> +                               }
> +                       }
> +                       squashfs_cache_put(buffer);
> +                       goto skip_pages;
> +               }
> +
>                 res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
>                                          actor);
>
> @@ -564,8 +580,10 @@ static void squashfs_readahead(struct
> readahead_control *ractl)
>                                 kunmap_atomic(pageaddr);
>                         }
>
> -                       for (i = 0; i < nr_pages; i++)
> +                       for (i = 0; i < nr_pages; i++) {
> +                               flush_dcache_page(pages[i]);
>                                 SetPageUptodate(pages[i]);
> +                       }
>                 }
>
>
> > Phillip
> >
> > > Best regards
> >

Re: [PATCH v4 3/3] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, Jun 6, 2022 at 7:09 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> On Mon, Jun 6, 2022 at 5:55 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> >
> > On Mon, Jun 6, 2022 at 11:54 AM Phillip Lougher <phillip@squashfs.org.uk> wrote:
> > >
> > > On 03/06/2022 16:58, Marek Szyprowski wrote:
> > > > Hi Matthew,
> > > >
> > > > On 03.06.2022 17:29, Matthew Wilcox wrote:
> > > >> On Fri, Jun 03, 2022 at 10:55:01PM +0800, Hsin-Yi Wang wrote:
> > > >>> On Fri, Jun 3, 2022 at 10:10 PM Marek Szyprowski
> > > >>> <m.szyprowski@samsung.com> wrote:
> > > >>>> Hi Matthew,
> > > >>>>
> > > >>>> On 03.06.2022 14:59, Matthew Wilcox wrote:
> > > >>>>> On Fri, Jun 03, 2022 at 02:54:21PM +0200, Marek Szyprowski wrote:
> > > >>>>>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> > > >>>>>>> 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 or not enough in a
> > > >>>>>>>       datablock.
> > > >>>>>>> - decompressor error.
> > > >>>>>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
> > > >>>>>>> updated by readpage later.
> > > >>>>>>>
> > > >>>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> > > >>>>>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > >>>>>>> Reported-by: Matthew Wilcox <willy@infradead.org>
> > > >>>>>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> > > >>>>>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> > > >>>>>>> ---
> > > >>>>>> This patch landed recently in linux-next as commit 95f7a26191de
> > > >>>>>> ("squashfs: implement readahead"). I've noticed that it causes serious
> > > >>>>>> issues on my test systems (various ARM 32bit and 64bit based boards).
> > > >>>>>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> > > >>>>>> populated' and prolonged booting time. I'm using squashfs for deploying
> > > >>>>>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> > > >>>>>> top of the next-20220603 fixes the issue.
> > > >>>>> How large are these files?  Just a few kilobytes?
> > > >>>> Yes, they are small, most of them are smaller than 16KB, some about
> > > >>>> 128KB and a few about 256KB. I've sent a detailed list in private mail.
> > > >>>>
> > > >>> Hi Marek,
> > > >>>
> > > >>> Are there any obvious squashfs errors in dmesg? Did you enable
> > > >>> CONFIG_SQUASHFS_FILE_DIRECT or CONFIG_SQUASHFS_FILE_CACHE?
> > > >> I don't think it's an error problem.  I think it's a short file problem.
> > > >>
> > > >> As I understand the current code (and apologies for not keeping up
> > > >> to date with how the patch is progressing), if the file is less than
> > > >> msblk->block_size bytes, we'll leave all the pages as !uptodate, leaving
> > > >> them to be brough uptodate by squashfs_read_folio().  So Marek is hitting
> > > >> the worst case scenario where we re-read the entire block for each page
> > > >> in it.  I think we have to handle this tail case in ->readahead().
> > > >
> > > > I'm not sure if this is related to reading of small files. There are
> > > > only 50 modules being loaded from squashfs volume. I did a quick test of
> > > > reading the files.
> > > >
> > > > Simple file read with this patch:
> > > >
> > > > root@target:~# time find /initrd/ -type f | while read f; do cat $f
> > > >   >/dev/null; done
> > > >
> > > > real    0m5.865s
> > > > user    0m2.362s
> > > > sys     0m3.844s
> > > >
> > > > Without:
> > > >
> > > > root@target:~# time find /initrd/ -type f | while read f; do cat $f
> > > >   >/dev/null; done
> > > >
> > > > real    0m6.619s
> > > > user    0m2.112s
> > > > sys     0m4.827s
> > > >
> > >
> > > It has been a four day holiday in the UK (Queen's Platinum Jubilee),
> > > hence the delay in responding.
> > >
> > > The above read use-case is sequential (only one thread/process),
> > > whereas the use-case where the slow-down is observed may be
> > > parallel (multiple threads/processes entering Squashfs).
> > >
> > > The above sequential use-case if the small files are held in
> > > fragments, will be exhibiting caching behaviour that will
> > > ameliorate the case where the same block is being repeatedly
> > > re-read for each page in it.  Because each time
> > > Squashfs is re-entered handling only a single page, the
> > > decompressed block will be found in the fragment
> > > cache, eliminating a block decompression for each page.
> > >
> > > In a parallel use-case the decompressed fragment block
> > > may be being eliminated from the cache (by other reading
> > > processes), hence forcing the block to be repeatedly
> > > decompressed.
> > >
> > > Hence the slow-down will be much more noticable with a
> > > parallel use-case than a sequential use-case.  It also may
> > > be why this slipped through testing, if the test cases
> > > are purely sequential in nature.
> > >
> > > So Matthew's previous comment is still the most likely
> > > explanation for the slow-down.
> > >
> > Thanks for the pointers. To deal with short file cases (nr_pages <
> > max_pages), Can we refer to squashfs_fill_page() used in
> > squashfs_read_cache(), similar to the case where there are missing
> > pages on the block?
> >
> > Directly calling squashfs_read_data() on short files will lead to crash:
> >
> > Unable to handle kernel paging request at virtual address:
> > [   19.244654]  zlib_inflate+0xba4/0x10c8
> > [   19.244658]  zlib_uncompress+0x150/0x1bc
> > [   19.244662]  squashfs_decompress+0x6c/0xb4
> > [   19.244669]  squashfs_read_data+0x1a8/0x298
> > [   19.244673]  squashfs_readahead+0x2cc/0x4cc
> >
> > I also noticed that the function didn't set flush_dcache_page() with
> > SetPageUptodate() previously.
> >
> > Put these 2 issues together:
> >
>
> The patch here is not correct. Please ignore it for now. Sorry for the noice.
>
Hi all,

The correct version is sent as v5:
https://lore.kernel.org/lkml/20220606150305.1883410-1-hsinyi@chromium.org/T/#t

Note that this is based on next-20220513, which doesn't have v4 applied.
I also squashed a fix to a checkpatch error in this version.

Thanks


> > diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> > index 658fb98af0cd..27519f1f9045 100644
> > --- a/fs/squashfs/file.c
> > +++ b/fs/squashfs/file.c
> > @@ -532,8 +532,7 @@ static void squashfs_readahead(struct
> > readahead_control *ractl)
> >                 if (!nr_pages)
> >                         break;
> >
> > -               if (readahead_pos(ractl) >= i_size_read(inode) ||
> > -                   nr_pages < max_pages)
> > +               if (readahead_pos(ractl) >= i_size_read(inode))
> >                         goto skip_pages;
> >
> >                 index = pages[0]->index >> shift;
> > @@ -548,6 +547,23 @@ static void squashfs_readahead(struct
> > readahead_control *ractl)
> >                 if (bsize == 0)
> >                         goto skip_pages;
> >
> > +               if (nr_pages < max_pages) {
> > +                       struct squashfs_cache_entry *buffer;
> > +
> > +                       buffer = squashfs_get_datablock(inode->i_sb, block,
> > +                                                       bsize);
> > +                       if (!buffer->error) {
> > +                               for (i = 0; i < nr_pages && expected > 0; i++,
> > +                                                       expected -= PAGE_SIZE) {
> > +                                       int avail = min_t(int,
> > expected, PAGE_SIZE);
> > +
> > +                                       squashfs_fill_page(pages[i],
> > buffer, i * PAGE_SIZE, avail);
> > +                               }
> > +                       }
> > +                       squashfs_cache_put(buffer);
> > +                       goto skip_pages;
> > +               }
> > +
> >                 res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
> >                                          actor);
> >
> > @@ -564,8 +580,10 @@ static void squashfs_readahead(struct
> > readahead_control *ractl)
> >                                 kunmap_atomic(pageaddr);
> >                         }
> >
> > -                       for (i = 0; i < nr_pages; i++)
> > +                       for (i = 0; i < nr_pages; i++) {
> > +                               flush_dcache_page(pages[i]);
> >                                 SetPageUptodate(pages[i]);
> > +                       }
> >                 }
> >
> >
> > > Phillip
> > >
> > > > Best regards
> > >

Re: [PATCH v4 3/3] squashfs: implement readahead

[Phillip Lougher]

On 03/06/2022 13:54, Marek Szyprowski wrote:
> Hi,
> 
> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
>> 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 or not enough in a
>>     datablock.
>> - decompressor error.
>> Otherwise pages will be marked as uptodate. The unhandled pages will be
>> updated by readpage later.
>>
>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>> Reported-by: Matthew Wilcox <willy@infradead.org>
>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
>> ---
> 
> This patch landed recently in linux-next as commit 95f7a26191de
> ("squashfs: implement readahead"). I've noticed that it causes serious
> issues on my test systems (various ARM 32bit and 64bit based boards).
> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> populated' and prolonged booting time. I'm using squashfs for deploying
> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> top of the next-20220603 fixes the issue.
> 
> Let me know how I can help debugging this issue. There is no hurry
> though, because the next week I will be on holidays.

Hi Marek,

Can you supply an example Squashfs filesystem and script that
reproduces the slow-down?  Failing that, can you supply a copy
of your initrd/root-filesystem that can be run under emulation
to reproduce the issue? (I don't have any modern ARM embedded
systems).

Again failing that, are you happy to test some debug code?

Thanks

Phillip (Squashfs maintainer and author).

> 
>> v3->v4: Fix a few variable type and their locations.
>> v3: https://lore.kernel.org/lkml/20220523065909.883444-4-hsinyi@chromium.org/
>> v2: https://lore.kernel.org/lkml/20220517082650.2005840-4-hsinyi@chromium.org/
>> v1: https://lore.kernel.org/lkml/20220516105100.1412740-3-hsinyi@chromium.org/
>> ---
>>    fs/squashfs/file.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-
>>    1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
>> index a8e495d8eb86..df7ad4b3e99c 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
>> @@ -495,7 +496,101 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>>    	return 0;
>>    }
>>    
>> +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;
>> +
>> +	actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>> +	if (!actor)
>> +		goto out;
>> +
>> +	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) ||
>> +		    nr_pages < max_pages)
>> +			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;
>> +
>> +		res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
>> +					 actor);
>> +
>> +		if (res == expected) {
>> +			int bytes;
>> +
>> +			/* Last page may have trailing bytes not filled */
>> +			bytes = res % PAGE_SIZE;
>> +			if (bytes) {
>> +				void *pageaddr;
>> +
>> +				pageaddr = kmap_atomic(pages[nr_pages - 1]);
>> +				memset(pageaddr + bytes, 0, PAGE_SIZE - bytes);
>> +				kunmap_atomic(pageaddr);
>> +			}
>> +
>> +			for (i = 0; i < nr_pages; i++)
>> +				SetPageUptodate(pages[i]);
>> +		}
>> +
>> +		for (i = 0; i < nr_pages; i++) {
>> +			unlock_page(pages[i]);
>> +			put_page(pages[i]);
>> +		}
>> +	}
>> +
>> +	kfree(actor);
>> +	kfree(pages);
>> +	return;
>> +
>> +skip_pages:
>> +	for (i = 0; i < nr_pages; i++) {
>> +		unlock_page(pages[i]);
>> +		put_page(pages[i]);
>> +	}
>> +
>> +	kfree(actor);
>> +out:
>> +	kfree(pages);
>> +}
>>    
>>    const struct address_space_operations squashfs_aops = {
>> -	.read_folio = squashfs_read_folio
>> +	.read_folio = squashfs_read_folio,
>> +	.readahead = squashfs_readahead
>>    };
> 
> Best regards

Re: [PATCH v4 3/3] squashfs: implement readahead

[Marek Szyprowski]

Hi Phillip,

On 07.06.2022 09:35, Phillip Lougher wrote:
> On 03/06/2022 13:54, Marek Szyprowski wrote:
>> Hi,
>>
>> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
>>> 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 or not enough in a
>>>     datablock.
>>> - decompressor error.
>>> Otherwise pages will be marked as uptodate. The unhandled pages will be
>>> updated by readpage later.
>>>
>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>>> Reported-by: Matthew Wilcox <willy@infradead.org>
>>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
>>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
>>> ---
>>
>> This patch landed recently in linux-next as commit 95f7a26191de
>> ("squashfs: implement readahead"). I've noticed that it causes serious
>> issues on my test systems (various ARM 32bit and 64bit based boards).
>> The easiest way to observe is udev timeout 'waiting for /dev to be fully
>> populated' and prolonged booting time. I'm using squashfs for deploying
>> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
>> top of the next-20220603 fixes the issue.
>>
>> Let me know how I can help debugging this issue. There is no hurry
>> though, because the next week I will be on holidays.
>
> Hi Marek,
>
> Can you supply an example Squashfs filesystem and script that
> reproduces the slow-down?  Failing that, can you supply a copy
> of your initrd/root-filesystem that can be run under emulation
> to reproduce the issue? (I don't have any modern ARM embedded
> systems).
>
> Again failing that, are you happy to test some debug code?
>
> Thanks
>
> Phillip (Squashfs maintainer and author).

I've just got back from my holidays.

Is this still relevant? I've noticed that v6 has been posted, but I 
failed to apply it on top of next-20220610 as mentioned in the 
cover-letter to test. I've also tried also to apply the mentioned 
'Squashfs: handle missing pages decompressing into page cache' patchset. 
On the other hand, next-20220610 seems to be working fine on my setup now.


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 3/3] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, Jun 13, 2022 at 8:08 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> Hi Phillip,
>
> On 07.06.2022 09:35, Phillip Lougher wrote:
> > On 03/06/2022 13:54, Marek Szyprowski wrote:
> >> Hi,
> >>
> >> On 01.06.2022 12:39, Hsin-Yi Wang wrote:
> >>> 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 or not enough in a
> >>>     datablock.
> >>> - decompressor error.
> >>> Otherwise pages will be marked as uptodate. The unhandled pages will be
> >>> updated by readpage later.
> >>>
> >>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> >>> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> >>> Reported-by: Matthew Wilcox <willy@infradead.org>
> >>> Reported-by: Phillip Lougher <phillip@squashfs.org.uk>
> >>> Reported-by: Xiongwei Song <Xiongwei.Song@windriver.com>
> >>> ---
> >>
> >> This patch landed recently in linux-next as commit 95f7a26191de
> >> ("squashfs: implement readahead"). I've noticed that it causes serious
> >> issues on my test systems (various ARM 32bit and 64bit based boards).
> >> The easiest way to observe is udev timeout 'waiting for /dev to be fully
> >> populated' and prolonged booting time. I'm using squashfs for deploying
> >> kernel modules via initrd. Reverting aeefca9dfae7 & 95f7a26191deon on
> >> top of the next-20220603 fixes the issue.
> >>
> >> Let me know how I can help debugging this issue. There is no hurry
> >> though, because the next week I will be on holidays.
> >
> > Hi Marek,
> >
> > Can you supply an example Squashfs filesystem and script that
> > reproduces the slow-down?  Failing that, can you supply a copy
> > of your initrd/root-filesystem that can be run under emulation
> > to reproduce the issue? (I don't have any modern ARM embedded
> > systems).
> >
> > Again failing that, are you happy to test some debug code?
> >
> > Thanks
> >
> > Phillip (Squashfs maintainer and author).
>
> I've just got back from my holidays.
>
> Is this still relevant? I've noticed that v6 has been posted, but I
> failed to apply it on top of next-20220610 as mentioned in the
> cover-letter to test. I've also tried also to apply the mentioned
> 'Squashfs: handle missing pages decompressing into page cache' patchset.
> On the other hand, next-20220610 seems to be working fine on my setup now.
>

hi Marek,

next-20220610 contains v5 of the series. To apply v6, you need to
revert ca1505bf4805 ("squashfs: implement readahead") and 9d58b94aa73a
("squashfs: always build "file direct" version of page actor") first,
then apply  'Squashfs: handle missing pages decompressing into page
cache' patchset, then finally apply v6, since v6 is dependent on the
patchset.

Thanks.

>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>