[PATCH 0/2] Implement readahead for squashfs

[PATCH 0/2] 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

 fs/squashfs/file.c  | 77 +++++++++++++++++++++++++++++++++++++++++++++
 fs/squashfs/super.c | 33 -------------------
 2 files changed, 77 insertions(+), 33 deletions(-)

-- 
2.36.0.550.gb090851708-goog

[PATCH 1/2] 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 <sxwjean@gmail.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.0.550.gb090851708-goog

[PATCH 2/2] 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.
Otherwise pages will be marked as uptodate. The unhandled pages will be
updated by readpage later.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
Note that this patch was not formally sent to the list before. It's
attached to email thread for discussion as it's still under development.

- v1:
The patch outline was suggested by Matthew. It went through a few
reviews by Matthew offline.

- v2:
https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m442435c149d411c5c9d8019cff5915419b04bf10
This is a resend of v1.

- v3:
https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m55a709e6ba5ec59fe95323a67a7f3d6b1953e470
Fix page actor size to avoid a crash from squashfs_decompress().
Suggested by Phillip Lougher[1]
[1] https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m687f82debb7667ff31982a05aef3eba081eb5039

- v4:
https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#mf93267690ec2e841dade6a494fe72c84b61328d9
Fix to free page after used. Suggested by Xiongwei Song[2]
Refactor the skip page logic to possible improve the performance.
Suggested by Phillip Lougher[3]
[2] https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m0e7b33d167b1ef0eb39b9f41c32ed3f80dfced18
[3] https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m1e0a8f8e4a98d79d14c81b66e197b6dc0a3b77a1
---
 fs/squashfs/file.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index a8e495d8eb86..91dfec792f4c 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,83 @@ 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;
+	size_t shift = msblk->block_log - PAGE_SHIFT;
+	loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
+	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;
+	u64 block = 0;
+	int bsize, res, i, index;
+	int file_end = i_size_read(inode) >> msblk->block_log;
+	unsigned int max_pages = 1UL << shift;
+
+	readahead_expand(ractl, start, (len | mask) + 1);
+
+	if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
+	    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 (;;) {
+		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;
+
+		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 >= 0)
+			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
+	.readahead = squashfs_readahead
 };
-- 
2.36.0.550.gb090851708-goog

Re: [PATCH 2/2] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, May 16, 2022 at 6:51 PM Hsin-Yi Wang <hsinyi@chromium.org> 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.
> 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>
> ---
> Note that this patch was not formally sent to the list before. It's
> attached to email thread for discussion as it's still under development.
>
> - v1:
> The patch outline was suggested by Matthew. It went through a few
> reviews by Matthew offline.
>
> - v2:
> https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m442435c149d411c5c9d8019cff5915419b04bf10
> This is a resend of v1.
>
> - v3:
> https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m55a709e6ba5ec59fe95323a67a7f3d6b1953e470
> Fix page actor size to avoid a crash from squashfs_decompress().
> Suggested by Phillip Lougher[1]
> [1] https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m687f82debb7667ff31982a05aef3eba081eb5039
>
> - v4:
> https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#mf93267690ec2e841dade6a494fe72c84b61328d9
> Fix to free page after used. Suggested by Xiongwei Song[2]
> Refactor the skip page logic to possible improve the performance.
> Suggested by Phillip Lougher[3]
> [2] https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m0e7b33d167b1ef0eb39b9f41c32ed3f80dfced18
> [3] https://lore.kernel.org/linux-mm/Yn5Yij9pRPCzDozt@casper.infradead.org/t/#m1e0a8f8e4a98d79d14c81b66e197b6dc0a3b77a1
> ---
>  fs/squashfs/file.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
>
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index a8e495d8eb86..91dfec792f4c 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,83 @@ 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;
> +       size_t shift = msblk->block_log - PAGE_SHIFT;
> +       loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
> +       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;
> +       u64 block = 0;
> +       int bsize, res, i, index;
> +       int file_end = i_size_read(inode) >> msblk->block_log;
> +       unsigned int max_pages = 1UL << shift;
> +
> +       readahead_expand(ractl, start, (len | mask) + 1);
> +
> +       if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
> +           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 (;;) {
> +               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;
> +
> +               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 >= 0)
> +                       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
> +       .readahead = squashfs_readahead
>  };
> --
> 2.36.0.550.gb090851708-goog
>

Re: [PATCH 2/2] squashfs: implement readahead

[Matthew Wilcox]

On Mon, May 16, 2022 at 07:04:08PM +0800, Hsin-Yi Wang wrote:
> > +       loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
> > +       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;
> > +       u64 block = 0;
> > +       int bsize, res, i, index;
> > +       int file_end = i_size_read(inode) >> msblk->block_log;
> > +       unsigned int max_pages = 1UL << shift;
> > +
> > +       readahead_expand(ractl, start, (len | mask) + 1);
> > +
> > +       if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
> > +           file_end == 0)
> > +               return;

What's the first half of this condition supposed to be checking for?
It seems to be checking whether readahead_expand() shrunk the range
covered by the ractl, but readahead_expand() never does that, so I'm
confused why you're checking for it.

Re: [PATCH 2/2] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, May 16, 2022 at 8:36 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, May 16, 2022 at 07:04:08PM +0800, Hsin-Yi Wang wrote:
> > > +       loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
> > > +       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;
> > > +       u64 block = 0;
> > > +       int bsize, res, i, index;
> > > +       int file_end = i_size_read(inode) >> msblk->block_log;
> > > +       unsigned int max_pages = 1UL << shift;
> > > +
> > > +       readahead_expand(ractl, start, (len | mask) + 1);
> > > +
> > > +       if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
> > > +           file_end == 0)
> > > +               return;
>
> What's the first half of this condition supposed to be checking for?
> It seems to be checking whether readahead_expand() shrunk the range
> covered by the ractl, but readahead_expand() never does that, so I'm
> confused why you're checking for it.

hi Matthew,

This is to check if readahead_expand() expands as much as it's requested.
I didn't encounter the mismatch so far in my testing. If this check is
not necessary, it can be removed.

Re: [PATCH 2/2] squashfs: implement readahead

[Matthew Wilcox]

On Mon, May 16, 2022 at 08:47:52PM +0800, Hsin-Yi Wang wrote:
> On Mon, May 16, 2022 at 8:36 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Mon, May 16, 2022 at 07:04:08PM +0800, Hsin-Yi Wang wrote:
> > > > +       loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
> > > > +       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;
> > > > +       u64 block = 0;
> > > > +       int bsize, res, i, index;
> > > > +       int file_end = i_size_read(inode) >> msblk->block_log;
> > > > +       unsigned int max_pages = 1UL << shift;
> > > > +
> > > > +       readahead_expand(ractl, start, (len | mask) + 1);
> > > > +
> > > > +       if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
> > > > +           file_end == 0)
> > > > +               return;
> >
> > What's the first half of this condition supposed to be checking for?
> > It seems to be checking whether readahead_expand() shrunk the range
> > covered by the ractl, but readahead_expand() never does that, so I'm
> > confused why you're checking for it.
> 
> hi Matthew,
> 
> This is to check if readahead_expand() expands as much as it's requested.
> I didn't encounter the mismatch so far in my testing. If this check is
> not necessary, it can be removed.

Then I think req_end is miscalculated?  It should surely be:

	req_end = start + (len | mask) + 1;

But I'm not sure that we should be failing under such circumstances.
For example, we may have been asked to read 1.5MB, attempt to round up
to 2MB, and fail.  But we can still submit a readahead for the first 1MB,
before leaving the second 512kB for readpage to handle.

So maybe we should just remove this check entirely.

Re: [PATCH 2/2] squashfs: implement readahead

[Hsin-Yi Wang]

On Mon, May 16, 2022 at 8:55 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, May 16, 2022 at 08:47:52PM +0800, Hsin-Yi Wang wrote:
> > On Mon, May 16, 2022 at 8:36 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Mon, May 16, 2022 at 07:04:08PM +0800, Hsin-Yi Wang wrote:
> > > > > +       loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
> > > > > +       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;
> > > > > +       u64 block = 0;
> > > > > +       int bsize, res, i, index;
> > > > > +       int file_end = i_size_read(inode) >> msblk->block_log;
> > > > > +       unsigned int max_pages = 1UL << shift;
> > > > > +
> > > > > +       readahead_expand(ractl, start, (len | mask) + 1);
> > > > > +
> > > > > +       if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
> > > > > +           file_end == 0)
> > > > > +               return;
> > >
> > > What's the first half of this condition supposed to be checking for?
> > > It seems to be checking whether readahead_expand() shrunk the range
> > > covered by the ractl, but readahead_expand() never does that, so I'm
> > > confused why you're checking for it.
> >
> > hi Matthew,
> >
> > This is to check if readahead_expand() expands as much as it's requested.
> > I didn't encounter the mismatch so far in my testing. If this check is
> > not necessary, it can be removed.
>
> Then I think req_end is miscalculated?  It should surely be:
>
>         req_end = start + (len | mask) + 1;
>
> But I'm not sure that we should be failing under such circumstances.
> For example, we may have been asked to read 1.5MB, attempt to round up
> to 2MB, and fail.  But we can still submit a readahead for the first 1MB,
> before leaving the second 512kB for readpage to handle.
>
> So maybe we should just remove this check entirely.

I'll remove this in the next version.

Re: [PATCH 2/2] squashfs: implement readahead

[kernel test robot]

Hi Hsin-Yi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20220513]
[cannot apply to akpm-mm/mm-everything v5.18-rc7 v5.18-rc6 v5.18-rc5 v5.18-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
base:    1e1b28b936aed946122b4e0991e7144fdbbfd77e
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220516/202205162245.LVgJF5HH-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
        git checkout 573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash fs/squashfs/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/squashfs/file.c: In function 'squashfs_readahead':
   fs/squashfs/file.c:526:17: error: implicit declaration of function 'squashfs_page_actor_init_special'; did you mean 'squashfs_page_actor_init'? [-Werror=implicit-function-declaration]
     526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 squashfs_page_actor_init
>> fs/squashfs/file.c:526:15: warning: assignment to 'struct squashfs_page_actor *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
         |               ^
   fs/squashfs/file.c: At top level:
   fs/squashfs/file.c:577:9: error: request for member 'readahead' in something not a structure or union
     577 |         .readahead = squashfs_readahead
         |         ^
   cc1: some warnings being treated as errors


vim +526 fs/squashfs/file.c

   498	
   499	static void squashfs_readahead(struct readahead_control *ractl)
   500	{
   501		struct inode *inode = ractl->mapping->host;
   502		struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
   503		size_t mask = (1UL << msblk->block_log) - 1;
   504		size_t shift = msblk->block_log - PAGE_SHIFT;
   505		loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
   506		loff_t start = readahead_pos(ractl) &~ mask;
   507		size_t len = readahead_length(ractl) + readahead_pos(ractl) - start;
   508		struct squashfs_page_actor *actor;
   509		unsigned int nr_pages = 0;
   510		struct page **pages;
   511		u64 block = 0;
   512		int bsize, res, i, index;
   513		int file_end = i_size_read(inode) >> msblk->block_log;
   514		unsigned int max_pages = 1UL << shift;
   515	
   516		readahead_expand(ractl, start, (len | mask) + 1);
   517	
   518		if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
   519		    file_end == 0)
   520			return;
   521	
   522		pages = kmalloc_array(max_pages, sizeof(void *), GFP_KERNEL);
   523		if (!pages)
   524			return;
   525	
 > 526		actor = squashfs_page_actor_init_special(pages, max_pages, 0);
   527		if (!actor)
   528			goto out;
   529	
   530		for (;;) {
   531			nr_pages = __readahead_batch(ractl, pages, max_pages);
   532			if (!nr_pages)
   533				break;
   534	
   535			if (readahead_pos(ractl) >= i_size_read(inode) ||
   536			    nr_pages < max_pages)
   537				goto skip_pages;
   538	
   539			index = pages[0]->index >> shift;
   540			if ((pages[nr_pages - 1]->index >> shift) != index)
   541				goto skip_pages;
   542	
   543			bsize = read_blocklist(inode, index, &block);
   544			if (bsize == 0)
   545				goto skip_pages;
   546	
   547			res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
   548						 actor);
   549	
   550			if (res >= 0)
   551				for (i = 0; i < nr_pages; i++)
   552					SetPageUptodate(pages[i]);
   553	
   554			for (i = 0; i < nr_pages; i++) {
   555				unlock_page(pages[i]);
   556				put_page(pages[i]);
   557			}
   558		}
   559	
   560		kfree(actor);
   561		kfree(pages);
   562		return;
   563	
   564	skip_pages:
   565		for (i = 0; i < nr_pages; i++) {
   566			unlock_page(pages[i]);
   567			put_page(pages[i]);
   568		}
   569	
   570		kfree(actor);
   571	out:
   572		kfree(pages);
   573	}
   574	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/2] squashfs: implement readahead

[kernel test robot]

Hi Hsin-Yi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220513]
[cannot apply to akpm-mm/mm-everything v5.18-rc7 v5.18-rc6 v5.18-rc5 v5.18-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
base:    1e1b28b936aed946122b4e0991e7144fdbbfd77e
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220516/202205162301.zVca8ZpM-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
        git checkout 573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/squashfs/file.c: In function 'squashfs_readahead':
>> fs/squashfs/file.c:526:17: error: implicit declaration of function 'squashfs_page_actor_init_special'; did you mean 'squashfs_page_actor_init'? [-Werror=implicit-function-declaration]
     526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 squashfs_page_actor_init
   fs/squashfs/file.c:526:15: warning: assignment to 'struct squashfs_page_actor *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
         |               ^
   fs/squashfs/file.c: At top level:
>> fs/squashfs/file.c:577:9: error: request for member 'readahead' in something not a structure or union
     577 |         .readahead = squashfs_readahead
         |         ^
   cc1: some warnings being treated as errors


vim +526 fs/squashfs/file.c

   498	
   499	static void squashfs_readahead(struct readahead_control *ractl)
   500	{
   501		struct inode *inode = ractl->mapping->host;
   502		struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
   503		size_t mask = (1UL << msblk->block_log) - 1;
   504		size_t shift = msblk->block_log - PAGE_SHIFT;
   505		loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
   506		loff_t start = readahead_pos(ractl) &~ mask;
   507		size_t len = readahead_length(ractl) + readahead_pos(ractl) - start;
   508		struct squashfs_page_actor *actor;
   509		unsigned int nr_pages = 0;
   510		struct page **pages;
   511		u64 block = 0;
   512		int bsize, res, i, index;
   513		int file_end = i_size_read(inode) >> msblk->block_log;
   514		unsigned int max_pages = 1UL << shift;
   515	
   516		readahead_expand(ractl, start, (len | mask) + 1);
   517	
   518		if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
   519		    file_end == 0)
   520			return;
   521	
   522		pages = kmalloc_array(max_pages, sizeof(void *), GFP_KERNEL);
   523		if (!pages)
   524			return;
   525	
 > 526		actor = squashfs_page_actor_init_special(pages, max_pages, 0);
   527		if (!actor)
   528			goto out;
   529	
   530		for (;;) {
   531			nr_pages = __readahead_batch(ractl, pages, max_pages);
   532			if (!nr_pages)
   533				break;
   534	
   535			if (readahead_pos(ractl) >= i_size_read(inode) ||
   536			    nr_pages < max_pages)
   537				goto skip_pages;
   538	
   539			index = pages[0]->index >> shift;
   540			if ((pages[nr_pages - 1]->index >> shift) != index)
   541				goto skip_pages;
   542	
   543			bsize = read_blocklist(inode, index, &block);
   544			if (bsize == 0)
   545				goto skip_pages;
   546	
   547			res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
   548						 actor);
   549	
   550			if (res >= 0)
   551				for (i = 0; i < nr_pages; i++)
   552					SetPageUptodate(pages[i]);
   553	
   554			for (i = 0; i < nr_pages; i++) {
   555				unlock_page(pages[i]);
   556				put_page(pages[i]);
   557			}
   558		}
   559	
   560		kfree(actor);
   561		kfree(pages);
   562		return;
   563	
   564	skip_pages:
   565		for (i = 0; i < nr_pages; i++) {
   566			unlock_page(pages[i]);
   567			put_page(pages[i]);
   568		}
   569	
   570		kfree(actor);
   571	out:
   572		kfree(pages);
   573	}
   574	
   575	const struct address_space_operations squashfs_aops = {
   576		.read_folio = squashfs_read_folio
 > 577		.readahead = squashfs_readahead

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Phillip Lougher]

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>
---
 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.34.1

Re: [PATCH 2/2] squashfs: implement readahead

[Phillip Lougher]

On 16/05/2022 15:21, kernel test robot wrote:
> Hi Hsin-Yi,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on next-20220513]
> [cannot apply to akpm-mm/mm-everything v5.18-rc7 v5.18-rc6 v5.18-rc5 v5.18-rc7]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 

This is fixed by

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

Which I have just sent.

Phillip

> url:    https://github.com/intel-lab-lkp/linux/commits/Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
> base:    1e1b28b936aed946122b4e0991e7144fdbbfd77e
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220516/202205162245.LVgJF5HH-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.3.0
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://github.com/intel-lab-lkp/linux/commit/573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
>          git checkout 573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash fs/squashfs/
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>     fs/squashfs/file.c: In function 'squashfs_readahead':
>     fs/squashfs/file.c:526:17: error: implicit declaration of function 'squashfs_page_actor_init_special'; did you mean 'squashfs_page_actor_init'? [-Werror=implicit-function-declaration]
>       526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           |                 squashfs_page_actor_init
>>> fs/squashfs/file.c:526:15: warning: assignment to 'struct squashfs_page_actor *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>       526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>           |               ^
>     fs/squashfs/file.c: At top level:
>     fs/squashfs/file.c:577:9: error: request for member 'readahead' in something not a structure or union
>       577 |         .readahead = squashfs_readahead
>           |         ^
>     cc1: some warnings being treated as errors
> 
> 
> vim +526 fs/squashfs/file.c
> 
>     498	
>     499	static void squashfs_readahead(struct readahead_control *ractl)
>     500	{
>     501		struct inode *inode = ractl->mapping->host;
>     502		struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
>     503		size_t mask = (1UL << msblk->block_log) - 1;
>     504		size_t shift = msblk->block_log - PAGE_SHIFT;
>     505		loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
>     506		loff_t start = readahead_pos(ractl) &~ mask;
>     507		size_t len = readahead_length(ractl) + readahead_pos(ractl) - start;
>     508		struct squashfs_page_actor *actor;
>     509		unsigned int nr_pages = 0;
>     510		struct page **pages;
>     511		u64 block = 0;
>     512		int bsize, res, i, index;
>     513		int file_end = i_size_read(inode) >> msblk->block_log;
>     514		unsigned int max_pages = 1UL << shift;
>     515	
>     516		readahead_expand(ractl, start, (len | mask) + 1);
>     517	
>     518		if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
>     519		    file_end == 0)
>     520			return;
>     521	
>     522		pages = kmalloc_array(max_pages, sizeof(void *), GFP_KERNEL);
>     523		if (!pages)
>     524			return;
>     525	
>   > 526		actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>     527		if (!actor)
>     528			goto out;
>     529	
>     530		for (;;) {
>     531			nr_pages = __readahead_batch(ractl, pages, max_pages);
>     532			if (!nr_pages)
>     533				break;
>     534	
>     535			if (readahead_pos(ractl) >= i_size_read(inode) ||
>     536			    nr_pages < max_pages)
>     537				goto skip_pages;
>     538	
>     539			index = pages[0]->index >> shift;
>     540			if ((pages[nr_pages - 1]->index >> shift) != index)
>     541				goto skip_pages;
>     542	
>     543			bsize = read_blocklist(inode, index, &block);
>     544			if (bsize == 0)
>     545				goto skip_pages;
>     546	
>     547			res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
>     548						 actor);
>     549	
>     550			if (res >= 0)
>     551				for (i = 0; i < nr_pages; i++)
>     552					SetPageUptodate(pages[i]);
>     553	
>     554			for (i = 0; i < nr_pages; i++) {
>     555				unlock_page(pages[i]);
>     556				put_page(pages[i]);
>     557			}
>     558		}
>     559	
>     560		kfree(actor);
>     561		kfree(pages);
>     562		return;
>     563	
>     564	skip_pages:
>     565		for (i = 0; i < nr_pages; i++) {
>     566			unlock_page(pages[i]);
>     567			put_page(pages[i]);
>     568		}
>     569	
>     570		kfree(actor);
>     571	out:
>     572		kfree(pages);
>     573	}
>     574	
> 

Re: [PATCH 2/2] squashfs: implement readahead

[Phillip Lougher]

On 16/05/2022 17:01, kernel test robot wrote:
> Hi Hsin-Yi,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on next-20220513]
> [cannot apply to akpm-mm/mm-everything v5.18-rc7 v5.18-rc6 v5.18-rc5 v5.18-rc7]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]

This is fixed by

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

Which I have just sent.

Phillip

> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
> base:    1e1b28b936aed946122b4e0991e7144fdbbfd77e
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220516/202205162301.zVca8ZpM-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.3.0
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://github.com/intel-lab-lkp/linux/commit/573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review Hsin-Yi-Wang/Implement-readahead-for-squashfs/20220516-185438
>          git checkout 573e1f2ced0df097c30c595d5bf5a9e7a5fcb8d5
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>     fs/squashfs/file.c: In function 'squashfs_readahead':
>>> fs/squashfs/file.c:526:17: error: implicit declaration of function 'squashfs_page_actor_init_special'; did you mean 'squashfs_page_actor_init'? [-Werror=implicit-function-declaration]
>       526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           |                 squashfs_page_actor_init
>     fs/squashfs/file.c:526:15: warning: assignment to 'struct squashfs_page_actor *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>       526 |         actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>           |               ^
>     fs/squashfs/file.c: At top level:
>>> fs/squashfs/file.c:577:9: error: request for member 'readahead' in something not a structure or union
>       577 |         .readahead = squashfs_readahead
>           |         ^
>     cc1: some warnings being treated as errors
> 
> 
> vim +526 fs/squashfs/file.c
> 
>     498	
>     499	static void squashfs_readahead(struct readahead_control *ractl)
>     500	{
>     501		struct inode *inode = ractl->mapping->host;
>     502		struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
>     503		size_t mask = (1UL << msblk->block_log) - 1;
>     504		size_t shift = msblk->block_log - PAGE_SHIFT;
>     505		loff_t req_end = readahead_pos(ractl) + readahead_length(ractl);
>     506		loff_t start = readahead_pos(ractl) &~ mask;
>     507		size_t len = readahead_length(ractl) + readahead_pos(ractl) - start;
>     508		struct squashfs_page_actor *actor;
>     509		unsigned int nr_pages = 0;
>     510		struct page **pages;
>     511		u64 block = 0;
>     512		int bsize, res, i, index;
>     513		int file_end = i_size_read(inode) >> msblk->block_log;
>     514		unsigned int max_pages = 1UL << shift;
>     515	
>     516		readahead_expand(ractl, start, (len | mask) + 1);
>     517	
>     518		if (readahead_pos(ractl) + readahead_length(ractl) < req_end ||
>     519		    file_end == 0)
>     520			return;
>     521	
>     522		pages = kmalloc_array(max_pages, sizeof(void *), GFP_KERNEL);
>     523		if (!pages)
>     524			return;
>     525	
>   > 526		actor = squashfs_page_actor_init_special(pages, max_pages, 0);
>     527		if (!actor)
>     528			goto out;
>     529	
>     530		for (;;) {
>     531			nr_pages = __readahead_batch(ractl, pages, max_pages);
>     532			if (!nr_pages)
>     533				break;
>     534	
>     535			if (readahead_pos(ractl) >= i_size_read(inode) ||
>     536			    nr_pages < max_pages)
>     537				goto skip_pages;
>     538	
>     539			index = pages[0]->index >> shift;
>     540			if ((pages[nr_pages - 1]->index >> shift) != index)
>     541				goto skip_pages;
>     542	
>     543			bsize = read_blocklist(inode, index, &block);
>     544			if (bsize == 0)
>     545				goto skip_pages;
>     546	
>     547			res = squashfs_read_data(inode->i_sb, block, bsize, NULL,
>     548						 actor);
>     549	
>     550			if (res >= 0)
>     551				for (i = 0; i < nr_pages; i++)
>     552					SetPageUptodate(pages[i]);
>     553	
>     554			for (i = 0; i < nr_pages; i++) {
>     555				unlock_page(pages[i]);
>     556				put_page(pages[i]);
>     557			}
>     558		}
>     559	
>     560		kfree(actor);
>     561		kfree(pages);
>     562		return;
>     563	
>     564	skip_pages:
>     565		for (i = 0; i < nr_pages; i++) {
>     566			unlock_page(pages[i]);
>     567			put_page(pages[i]);
>     568		}
>     569	
>     570		kfree(actor);
>     571	out:
>     572		kfree(pages);
>     573	}
>     574	
>     575	const struct address_space_operations squashfs_aops = {
>     576		.read_folio = squashfs_read_folio
>   > 577		.readahead = squashfs_readahead
> 

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

[Hsin-Yi Wang]

On Tue, May 17, 2022 at 11:36 AM Phillip Lougher
<phillip@squashfs.org.uk> wrote:
>
> 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>
Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

Tested with CONFIG_SQUASHFS_FILE_DIRECT unselected. I'll pick this
patch to the series. Thanks

> ---
>  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.34.1
>