From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA5C9C433F5 for ; Sun, 15 May 2022 00:05:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CFB6B6B0073; Sat, 14 May 2022 20:05:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CA9816B0075; Sat, 14 May 2022 20:05:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B4AF06B0078; Sat, 14 May 2022 20:05:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id A6B0D6B0073 for ; Sat, 14 May 2022 20:05:37 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id E269160BA5 for ; Sun, 15 May 2022 00:05:34 +0000 (UTC) X-FDA: 79466033388.27.18B2355 Received: from p3plwbeout23-06.prod.phx3.secureserver.net (p3plsmtp23-06-2.prod.phx3.secureserver.net [68.178.252.172]) by imf26.hostedemail.com (Postfix) with ESMTP id 10EDD1400B6 for ; Sun, 15 May 2022 00:05:30 +0000 (UTC) Received: from mailex.mailcore.me ([94.136.40.145]) by :WBEOUT: with ESMTP id q1lMnfMRzcabFq1lMn5ljd; Sat, 14 May 2022 17:05:33 -0700 X-CMAE-Analysis: v=2.4 cv=Y/g9DjSN c=1 sm=1 tr=0 ts=628043cd a=7e6w4QD8YWtpVJ/7+iiidw==:117 a=84ok6UeoqCVsigPHarzEiQ==:17 a=ggZhUymU-5wA:10 a=IkcTkHD0fZMA:10 a=oZkIemNP1mAA:10 a=fLXdSNe98ZZkRG-PP7kA:9 a=QEXdDO2ut3YA:10 X-SECURESERVER-ACCT: phillip@squashfs.org.uk X-SID: q1lMnfMRzcabF Received: from 82-69-79-175.dsl.in-addr.zen.co.uk ([82.69.79.175] helo=[192.168.178.33]) by smtp10.mailcore.me with esmtpa (Exim 4.94.2) (envelope-from ) id 1nq1lL-000809-Dx; Sun, 15 May 2022 01:05:31 +0100 Message-ID: <41f94eab-2231-def6-f269-b35c0977fced@squashfs.org.uk> Date: Sun, 15 May 2022 01:05:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: squashfs performance regression and readahea To: Matthew Wilcox Cc: Hsin-Yi Wang , Xiongwei Song , Zheng Liang , Zhang Yi , Hou Tao , Miao Xie , Andrew Morton , Linus Torvalds , "Song, Xiongwei" , "linux-mm@kvack.org" , "squashfs-devel@lists.sourceforge.net" References: <1f8a8009-1c05-d55c-08bd-89c5916e5240@squashfs.org.uk> From: Phillip Lougher In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailcore-Auth: 439999529 X-Mailcore-Domain: 1394945 X-123-reg-Authenticated: phillip@squashfs.org.uk X-Originating-IP: 82.69.79.175 X-CMAE-Envelope: MS4xfFPYgbEdXSxRE00VTXdFsjAn6S7w2Qlv5GIg9nHGKUKUapi6wEgzoj6ilAfZTvvCSmma/MZ6WLr7YtAKUMxzOHrJOURjSZ69D7ZG+fS9x1VGYs/1MgER X4u1mB5F1HiwR/32okYdVxN0Lt019L/XORzaTf2uFVUTW1VVtcWXry/W4m91dsTqbKr2i0pD7/uE8ymlQHMUM5L7H6zD5lCz4dY= X-Stat-Signature: 3ygbug36c1r1zd3sa1wjd1498r4eqaqp Authentication-Results: imf26.hostedemail.com; dkim=none; spf=none (imf26.hostedemail.com: domain of phillip@squashfs.org.uk has no SPF policy when checking 68.178.252.172) smtp.mailfrom=phillip@squashfs.org.uk; dmarc=none X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 10EDD1400B6 X-HE-Tag: 1652573130-863199 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 13/05/2022 14:09, Matthew Wilcox wrote: > On Fri, May 13, 2022 at 06:33:21AM +0100, Phillip Lougher wrote: >> Looking at the new patch, I have a couple of questions which is worth >> clarifying to have a fuller understanding of the readahead behaviour. >> In otherwords I'm deducing the behaviour of the readahead calls >> from context, and I want to make sure they're doing what I think >> they're doing. > > I did write quite a lot of documentation as part of the readahead > revision, and filesystem authors are the target audience, so this is > somewhat disheartening to read. What could I have done better to make > the readahead documentation obvious for you to find? That wasn't a criticism about your documentation or how hard it is to find. Please don't take it that way. It was just quicker (for me) to understand the patch from a Squashfs point of view. Phillip > >> + nr_pages = min(readahead_count(ractl), max_pages); >> >> As I understand it, this will always produce nr_pages which will >> cover the entirety of the block to be decompressed? That is if >> a block is block_size, it will return the number of pages necessary >> to decompress the entire block? It will never return less than the >> necessary pages, i.e. if the block_size was 128K, it will never >> return less than 32 pages? > > readahead_count() returns the number of pages that the page cache is > asking the filesystem for. It may be any number from 1 to whatever > the current readahead window is. It's possible to ask the page > cache to expand the readahead request to be aligned to a decompression > boundary, but that may not be possible. For example, we may be in a > situation where we read pages 32-63 from the file previously, then > the page cache chose to discard pages 33, 35, 37, .., 63 under memory > pressure, and now the file is being re-read. This isn't a likely > usage pattern, of course, but it's a situation we have to cope with. > >> + nr_pages = __readahead_batch(ractl, pages, max_pages) >> >> My understanding is that this call will fully populate the >> pages array with page references without any holes. That >> is none of the pages array entries will be NULL, meaning >> there isn't a page for that entry. In other words, if the >> pages array has 32 pages, each of the 32 entries will >> reference a page. > > That is correct, a readahead request is always for a contiguous range of > the file. The pages are allocated before calling ->readahead, so > there's no opportunity for failure; they exist and they're already in > the page cache, waiting for the FS to tell the pagecache that they're > uptodate and unlock them. >