* [PATCH 0/2] Implement readahead for squashfs
@ 2022-05-16 10:50 Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 1/2] Revert "squashfs: provide backing_dev_info in order to disable read-ahead" Hsin-Yi Wang
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-16 10:50 UTC (permalink / raw)
To: Phillip Lougher, Matthew Wilcox, Xiongwei Song
Cc: Zheng Liang, Zhang Yi, Hou Tao, Miao Xie, Andrew Morton,
linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Revert "squashfs: provide backing_dev_info in order to disable read-ahead"
2022-05-16 10:50 [PATCH 0/2] Implement readahead for squashfs Hsin-Yi Wang
@ 2022-05-16 10:51 ` Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 2/2] squashfs: implement readahead Hsin-Yi Wang
2022-05-17 3:35 ` [PATCH 3/2] squashfs: always build "file direct" version of page actor Phillip Lougher
2 siblings, 0 replies; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-16 10:51 UTC (permalink / raw)
To: Phillip Lougher, Matthew Wilcox, Xiongwei Song
Cc: Zheng Liang, Zhang Yi, Hou Tao, Miao Xie, Andrew Morton,
linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] squashfs: implement readahead
2022-05-16 10:50 [PATCH 0/2] Implement readahead for squashfs Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 1/2] Revert "squashfs: provide backing_dev_info in order to disable read-ahead" Hsin-Yi Wang
@ 2022-05-16 10:51 ` Hsin-Yi Wang
2022-05-16 11:04 ` Hsin-Yi Wang
` (2 more replies)
2022-05-17 3:35 ` [PATCH 3/2] squashfs: always build "file direct" version of page actor Phillip Lougher
2 siblings, 3 replies; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-16 10:51 UTC (permalink / raw)
To: Phillip Lougher, Matthew Wilcox, Xiongwei Song
Cc: Zheng Liang, Zhang Yi, Hou Tao, Miao Xie, Andrew Morton,
linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 10:51 ` [PATCH 2/2] squashfs: implement readahead Hsin-Yi Wang
@ 2022-05-16 11:04 ` Hsin-Yi Wang
2022-05-16 12:36 ` Matthew Wilcox
2022-05-16 14:21 ` kernel test robot
2022-05-16 16:01 ` kernel test robot
2 siblings, 1 reply; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-16 11:04 UTC (permalink / raw)
To: Phillip Lougher, Matthew Wilcox, Xiongwei Song
Cc: Zheng Liang, Zhang Yi, Hou Tao, Miao Xie, Andrew Morton,
linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 11:04 ` Hsin-Yi Wang
@ 2022-05-16 12:36 ` Matthew Wilcox
2022-05-16 12:47 ` Hsin-Yi Wang
0 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2022-05-16 12:36 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Phillip Lougher, Xiongwei Song, Zheng Liang, Zhang Yi, Hou Tao,
Miao Xie, Andrew Morton, linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 12:36 ` Matthew Wilcox
@ 2022-05-16 12:47 ` Hsin-Yi Wang
2022-05-16 12:55 ` Matthew Wilcox
0 siblings, 1 reply; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-16 12:47 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Phillip Lougher, Xiongwei Song, Zheng Liang, Zhang Yi, Hou Tao,
Miao Xie, Andrew Morton, linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 12:47 ` Hsin-Yi Wang
@ 2022-05-16 12:55 ` Matthew Wilcox
2022-05-16 12:57 ` Hsin-Yi Wang
0 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2022-05-16 12:55 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Phillip Lougher, Xiongwei Song, Zheng Liang, Zhang Yi, Hou Tao,
Miao Xie, Andrew Morton, linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 12:55 ` Matthew Wilcox
@ 2022-05-16 12:57 ` Hsin-Yi Wang
0 siblings, 0 replies; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-16 12:57 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Phillip Lougher, Xiongwei Song, Zheng Liang, Zhang Yi, Hou Tao,
Miao Xie, Andrew Morton, linux-mm @ kvack . org,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 10:51 ` [PATCH 2/2] squashfs: implement readahead Hsin-Yi Wang
2022-05-16 11:04 ` Hsin-Yi Wang
@ 2022-05-16 14:21 ` kernel test robot
2022-05-17 3:40 ` Phillip Lougher
2022-05-16 16:01 ` kernel test robot
2 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2022-05-16 14:21 UTC (permalink / raw)
To: Hsin-Yi Wang, Phillip Lougher, Matthew Wilcox, Xiongwei Song
Cc: kbuild-all, Zheng Liang, Zhang Yi, Hou Tao, Miao Xie,
Andrew Morton, Linux Memory Management List,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 10:51 ` [PATCH 2/2] squashfs: implement readahead Hsin-Yi Wang
2022-05-16 11:04 ` Hsin-Yi Wang
2022-05-16 14:21 ` kernel test robot
@ 2022-05-16 16:01 ` kernel test robot
2022-05-17 3:41 ` Phillip Lougher
2 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2022-05-16 16:01 UTC (permalink / raw)
To: Hsin-Yi Wang, Phillip Lougher, Matthew Wilcox, Xiongwei Song
Cc: kbuild-all, Zheng Liang, Zhang Yi, Hou Tao, Miao Xie,
Andrew Morton, Linux Memory Management List,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/2] squashfs: always build "file direct" version of page actor
2022-05-16 10:50 [PATCH 0/2] Implement readahead for squashfs Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 1/2] Revert "squashfs: provide backing_dev_info in order to disable read-ahead" Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 2/2] squashfs: implement readahead Hsin-Yi Wang
@ 2022-05-17 3:35 ` Phillip Lougher
2022-05-17 7:51 ` Hsin-Yi Wang
2 siblings, 1 reply; 14+ messages in thread
From: Phillip Lougher @ 2022-05-17 3:35 UTC (permalink / raw)
To: hsinyi
Cc: Xiongwei.Song, akpm, houtao1, linux-kernel, linux-mm, miaoxie,
phillip, squashfs-devel, willy, yi.zhang, zhengliang6,
kernel test robot
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 14:21 ` kernel test robot
@ 2022-05-17 3:40 ` Phillip Lougher
0 siblings, 0 replies; 14+ messages in thread
From: Phillip Lougher @ 2022-05-17 3:40 UTC (permalink / raw)
To: kernel test robot, Hsin-Yi Wang, Matthew Wilcox, Xiongwei Song
Cc: kbuild-all, Zheng Liang, Zhang Yi, Hou Tao, Miao Xie,
Andrew Morton, Linux Memory Management List,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] squashfs: implement readahead
2022-05-16 16:01 ` kernel test robot
@ 2022-05-17 3:41 ` Phillip Lougher
0 siblings, 0 replies; 14+ messages in thread
From: Phillip Lougher @ 2022-05-17 3:41 UTC (permalink / raw)
To: kernel test robot, Hsin-Yi Wang, Matthew Wilcox, Xiongwei Song
Cc: kbuild-all, Zheng Liang, Zhang Yi, Hou Tao, Miao Xie,
Andrew Morton, Linux Memory Management List,
squashfs-devel @ lists . sourceforge . net, linux-kernel
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/2] squashfs: always build "file direct" version of page actor
2022-05-17 3:35 ` [PATCH 3/2] squashfs: always build "file direct" version of page actor Phillip Lougher
@ 2022-05-17 7:51 ` Hsin-Yi Wang
0 siblings, 0 replies; 14+ messages in thread
From: Hsin-Yi Wang @ 2022-05-17 7:51 UTC (permalink / raw)
To: Phillip Lougher
Cc: Xiongwei.Song, akpm, houtao1, linux-kernel, linux-mm, miaoxie,
squashfs-devel, willy, yi.zhang, zhengliang6, kernel test robot
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-05-17 7:52 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16 10:50 [PATCH 0/2] Implement readahead for squashfs Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 1/2] Revert "squashfs: provide backing_dev_info in order to disable read-ahead" Hsin-Yi Wang
2022-05-16 10:51 ` [PATCH 2/2] squashfs: implement readahead Hsin-Yi Wang
2022-05-16 11:04 ` Hsin-Yi Wang
2022-05-16 12:36 ` Matthew Wilcox
2022-05-16 12:47 ` Hsin-Yi Wang
2022-05-16 12:55 ` Matthew Wilcox
2022-05-16 12:57 ` Hsin-Yi Wang
2022-05-16 14:21 ` kernel test robot
2022-05-17 3:40 ` Phillip Lougher
2022-05-16 16:01 ` kernel test robot
2022-05-17 3:41 ` Phillip Lougher
2022-05-17 3:35 ` [PATCH 3/2] squashfs: always build "file direct" version of page actor Phillip Lougher
2022-05-17 7:51 ` Hsin-Yi Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).