Re: Readahead regressed with c1f6925e1091("mm: put readahead pages in cache earlier") on multicore arm64 platforms

[Hsin-Yi Wang <hsinyi@chromium.org>]

On Thu, Oct 7, 2021 at 9:46 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Thu, Oct 07, 2021 at 03:08:38PM +0800, Hsin-Yi Wang wrote:
> > This calls into squashfs_readpage().
>
> Aha!  I hadn't looked at squashfs before, and now that I do, I can
> see why this commit causes problems for squashfs.  (It would be
> helpful if your report included more detail about which paths inside
> squashfs were taken, but I think I can guess):
>
> squashfs_readpage()
>   squashfs_readpage_block()
>     squashfs_copy_cache()
>       grab_cache_page_nowait()
>
Right, before the patch, push_page won't be null but after the patch,
grab_cache_page_nowait() fails.


> Before this patch, readahead of 1MB would allocate 256x4kB pages,
> then add each one to the page cache and call ->readpage on it:
>
>         for (page_idx = 0; page_idx < readahead_count(rac); page_idx++) {
>                 struct page *page = lru_to_page(pages);
>                 list_del(&page->lru);
>                 if (!add_to_page_cache_lru(page, rac->mapping, page->index,
>                                gfp))
>                         aops->readpage(rac->file, page);
>
> When Squashfs sees it has more than 4kB of data, it calls
> grab_cache_page_nowait(), which allocates more memory (ignoring the
> other 255 pages which have been allocated, because they're not in the
> page cache yet).  Then this loop frees the pages that readahead
> allocated.
>
> After this patch, the pages are already in the page cache when
> ->readpage is called the first time.  So the call to
> grab_cache_page_nowait() fails and squashfs redoes the decompression for
> each page.
>
> Neither of these approaches are efficient.  Squashfs need to implement
> ->readahead.  Working on it now ...
>