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 6DA29C433F5 for ; Thu, 19 May 2022 08:09:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D27E66B0072; Thu, 19 May 2022 04:09:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CD71F6B0073; Thu, 19 May 2022 04:09:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B9EF56B0074; Thu, 19 May 2022 04:09:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id A7FDA6B0072 for ; Thu, 19 May 2022 04:09:49 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 78BD89F8 for ; Thu, 19 May 2022 08:09:49 +0000 (UTC) X-FDA: 79481768898.28.0672349 Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) by imf25.hostedemail.com (Postfix) with ESMTP id 89980A00C8 for ; Thu, 19 May 2022 08:09:24 +0000 (UTC) Received: by mail-vs1-f46.google.com with SMTP id a12so2616355vsp.5 for ; Thu, 19 May 2022 01:09:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6CHqFUc2pzrLwDPfZGytZQ03T9cBPjhPzdLcOw4RoFA=; b=UbduS0B4a9VwGpxuxp3hYbv9y3h6h5rQFZpKQZ8qHUt7CDX0YahUxcwgoVq6NXDprY BMn44pHe9GlexYKSvuqDN6IC9F8Jr4J8DnB3t4ugT/7J7B2mxwncPcv3L+i8olECR3lD kuCHUqj7f7h8CGhjhfzNXibGCmlgzSvbntMys= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=6CHqFUc2pzrLwDPfZGytZQ03T9cBPjhPzdLcOw4RoFA=; b=e8Zt5UwCsk9Kg7J1kHWvpFR8x64N175QVCaOS0Pg1t3LSE+C/4SQ/RGVeVb9sS+6eY ieGIQROnSR6gdLP3I6DGkz2XLvRiCeqK3hrmyETTi+nQhoxO9+vljvTuvNTaMCovqj0i 2sIxoWKyG2iMS4uOadZjTNp0OZu3jRzw6To1CuYMnaMmTnTVwftRevQtqJw71KhR8Fzo uRQ3ZbP+lZMa1vLjw6u89nx2H/QEDAIodQ5a3P6TrdIgB4Yet9jv9eLd4Ju5kRG6N13W yRm6lkD422MtzF3R4k3Ox8t8U0Bp6FSJoqA92GLby3pF2yMz/t9zTFK5iFjtNoKdB3Uz Xa1A== X-Gm-Message-State: AOAM532GxTSt0rpRT2W/bOuXAPZx130IAYbT7z9u7fZoH/aeG7oEAaSB fjWb3+gQVJnXQ9/CxFbFZ+Rgx1EXU24FmsKL/6QsyQ== X-Google-Smtp-Source: ABdhPJyR0wfOdCkx9Ln3PXfB2tqqXTqSE4JnMb4pcTmgGOXyrijxP0mjju1tZU9SOZGyIg55wKrOSMCUloO1vRhUDvw= X-Received: by 2002:a05:6102:3a76:b0:32c:e483:3e45 with SMTP id bf22-20020a0561023a7600b0032ce4833e45mr1650908vsb.19.1652947787618; Thu, 19 May 2022 01:09:47 -0700 (PDT) MIME-Version: 1.0 References: <20220517082650.2005840-1-hsinyi@chromium.org> <20220517082650.2005840-4-hsinyi@chromium.org> In-Reply-To: <20220517082650.2005840-4-hsinyi@chromium.org> From: Hsin-Yi Wang Date: Thu, 19 May 2022 16:09:21 +0800 Message-ID: Subject: Re: [PATCH v2 3/3] squashfs: implement readahead 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@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 89980A00C8 X-Stat-Signature: cepi8exa5ry8km6t1zt6xzrb371ffrst X-Rspam-User: Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=UbduS0B4; spf=pass (imf25.hostedemail.com: domain of hsinyi@chromium.org designates 209.85.217.46 as permitted sender) smtp.mailfrom=hsinyi@chromium.org; dmarc=pass (policy=none) header.from=chromium.org X-HE-Tag: 1652947764-271733 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 Tue, May 17, 2022 at 4:28 PM Hsin-Yi Wang 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 > Signed-off-by: Hsin-Yi Wang > Reported-by: Matthew Wilcox > Reported-by: Phillip Lougher > Reported-by: Xiongwei Song > --- > v1->v2: remove unused check on readahead_expand(). > v1: https://lore.kernel.org/lkml/20220516105100.1412740-3-hsinyi@chromium.org/ > --- Hi Phillip and Matthew, Regarding the performance issue of this patch, I saw a possible performance gain if we only read the first block instead of reading until nr_pages == 0. To be more clear, apply the following diff (Please ignore the skipping of nr_pages check first. This is a demonstration of "only read and update the first block per readahead call"): diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c index aad6823f0615..c52f7c4a7cfe 100644 --- a/fs/squashfs/file.c +++ b/fs/squashfs/file.c @@ -524,10 +524,8 @@ static void squashfs_readahead(struct readahead_control *ractl) 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) All the performance numbers: 1. original: 39s 2. revert "mm: put readahead pages in cache earlier": 2.8s 3. v2 of this patch: 2.7s 4. v2 of this patch and apply the diff: 1.8s In my testing data, normally it reads and updates 1~2 blocks per readahead call. The change might not make sense since the performance improvement may only happen in certain cases. What do you think? Or is the performance of the current patch considered reasonable? Thanks. testing env: - arm64 on kernel 5.10 - data: ~ 300K pack file contains some android files > fs/squashfs/file.c | 77 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 76 insertions(+), 1 deletion(-) > > diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c > index a8e495d8eb86..e10a55c5b1eb 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,81 @@ 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 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 (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 > + .read_folio = squashfs_read_folio, > + .readahead = squashfs_readahead > }; > -- > 2.36.0.550.gb090851708-goog >