* 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 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 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
* 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