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 08386C433EF for ; Mon, 16 May 2022 12:55:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9356F6B0071; Mon, 16 May 2022 08:55:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8BEFB6B0072; Mon, 16 May 2022 08:55:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 785446B0073; Mon, 16 May 2022 08:55:36 -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 666E36B0071 for ; Mon, 16 May 2022 08:55:36 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 4271214B9 for ; Mon, 16 May 2022 12:55:36 +0000 (UTC) X-FDA: 79471602672.08.37921AB Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf17.hostedemail.com (Postfix) with ESMTP id 8F32C400C2 for ; Mon, 16 May 2022 12:55:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=beBrIscqgy/0uR/o+DxDTi2opTq7h59zNaTA5248BaE=; b=cO6FBmkT8y7IKKung2vpOoIQ5k 7oOqh/CYnFPlnrU1EfIvPKMMRYCfHUNrp+7WbU3R72a4bEDHzoZlyVdwo8TF+7+VIz/8amLZBTKGV QeMJJwfl5Y5/9RDq362odnkReBqFLN8g8Yi4qs7+NBJdR3DbbowtGGj73pAq+hZP6IzTisv/seGF9 VYEjTKO/fJ6jX9hpEHz6iMwSxpwQ9NdL3QgV27N4fQ4O8BIkeE3Hx7/8EybgwipWcPjrUklCEu86N MPPSZz1FSuISFDZYMmi2zwZNQDhzLaImM0nX+2F9auL5AwD3M7Wli7Dyz6RURKO4RR550eZ101FAe iFv1tLlA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nqaFx-009tNO-NU; Mon, 16 May 2022 12:55:25 +0000 Date: Mon, 16 May 2022 13:55:25 +0100 From: Matthew Wilcox 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@vger.kernel.org Subject: Re: [PATCH 2/2] squashfs: implement readahead Message-ID: References: <20220516105100.1412740-1-hsinyi@chromium.org> <20220516105100.1412740-3-hsinyi@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 8F32C400C2 X-Stat-Signature: pp6x3xpi59a941jekammknfjecr34bru X-Rspam-User: Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=cO6FBmkT; spf=none (imf17.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none X-HE-Tag: 1652705712-902648 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 Mon, May 16, 2022 at 08:47:52PM +0800, Hsin-Yi Wang wrote: > On Mon, May 16, 2022 at 8:36 PM Matthew Wilcox 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.