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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22FFAC433FE for ; Mon, 16 May 2022 12:56:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243626AbiEPM4v (ORCPT ); Mon, 16 May 2022 08:56:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43238 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243891AbiEPM4E (ORCPT ); Mon, 16 May 2022 08:56:04 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7EB8C38BE4 for ; Mon, 16 May 2022 05:55:51 -0700 (PDT) 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: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.