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 X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ECEA2C3B194 for ; Fri, 14 Feb 2020 03:36:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B9906206ED for ; Fri, 14 Feb 2020 03:36:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="TZf2FK1b" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728513AbgBNDgk (ORCPT ); Thu, 13 Feb 2020 22:36:40 -0500 Received: from hqnvemgate26.nvidia.com ([216.228.121.65]:8151 "EHLO hqnvemgate26.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728052AbgBNDgk (ORCPT ); Thu, 13 Feb 2020 22:36:40 -0500 Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Thu, 13 Feb 2020 19:36:25 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Thu, 13 Feb 2020 19:36:39 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Thu, 13 Feb 2020 19:36:39 -0800 Received: from [10.110.48.28] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 14 Feb 2020 03:36:39 +0000 Subject: Re: [PATCH v5 03/13] mm: Put readahead pages in cache earlier To: Matthew Wilcox , CC: , , , , , , , , References: <20200211010348.6872-1-willy@infradead.org> <20200211010348.6872-4-willy@infradead.org> X-Nvconfidentiality: public From: John Hubbard Message-ID: Date: Thu, 13 Feb 2020 19:36:38 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200211010348.6872-4-willy@infradead.org> X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To HQMAIL107.nvidia.com (172.20.187.13) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1581651385; bh=jodX0tIvBxH/fc59T73o9SPk6S3e+yDpKWnaNwpeJKk=; h=X-PGP-Universal:Subject:To:CC:References:X-Nvconfidentiality:From: Message-ID:Date:User-Agent:MIME-Version:In-Reply-To: X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=TZf2FK1bESe06GQrr3QWPstwI5CI6ADgd3ekfkEEBCxsznAzp/43OrXvLIdUwk6v9 4PvhXV9KA2GzBIO5FVXywx6Yx4JZowfMDze3Z/5j54bStQRS61QiScPwZCgRgQ9xzt 7ai4uLewCJr+qbdYGw8j6AC1KIuz1sHeXguO+TyPFfs/5Cwmjn/6ETP0ytHeXfze8x aZXVC6Py5XJwaXp8pJ7Om4/J3JPHPE/2VYV1PRSOmD236D2IEEGhBl9TBFg0OLQxCY GWBBoVBH3FzkluFiC7gp+exF/Tlgdi2RCth1YbNLQQ91aeVldSsZAblIBpXwiH2ihX ePKdIST/oKi4w== Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On 2/10/20 5:03 PM, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" > > At allocation time, put the pages in the cache unless we're using > ->readpages. > > Signed-off-by: Matthew Wilcox (Oracle) > --- > mm/readahead.c | 66 ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 24 deletions(-) > > diff --git a/mm/readahead.c b/mm/readahead.c > index fc77d13af556..96c6ca68a174 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -114,10 +114,10 @@ int read_cache_pages(struct address_space *mapping, struct list_head *pages, > EXPORT_SYMBOL(read_cache_pages); > > static void read_pages(struct address_space *mapping, struct file *filp, > - struct list_head *pages, unsigned int nr_pages, gfp_t gfp) > + struct list_head *pages, pgoff_t start, > + unsigned int nr_pages) > { > struct blk_plug plug; > - unsigned page_idx; > > blk_start_plug(&plug); > > @@ -125,18 +125,17 @@ static void read_pages(struct address_space *mapping, struct file *filp, > mapping->a_ops->readpages(filp, mapping, pages, nr_pages); > /* Clean up the remaining pages */ > put_pages_list(pages); > - goto out; > - } > + } else { > + struct page *page; > + unsigned long index; > > - for (page_idx = 0; page_idx < nr_pages; page_idx++) { > - struct page *page = lru_to_page(pages); > - list_del(&page->lru); > - if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) > + xa_for_each_range(&mapping->i_pages, index, page, start, > + start + nr_pages - 1) { > mapping->a_ops->readpage(filp, page); > - put_page(page); > + put_page(page); > + } > } > > -out: > blk_finish_plug(&plug); > } > > @@ -149,17 +148,18 @@ static void read_pages(struct address_space *mapping, struct file *filp, > * Returns the number of pages requested, or the maximum amount of I/O allowed. > */ > unsigned long __do_page_cache_readahead(struct address_space *mapping, > - struct file *filp, pgoff_t offset, unsigned long nr_to_read, > + struct file *filp, pgoff_t start, unsigned long nr_to_read, > unsigned long lookahead_size) > { > struct inode *inode = mapping->host; > - struct page *page; > unsigned long end_index; /* The last page we want to read */ > LIST_HEAD(page_pool); > int page_idx; > + pgoff_t page_offset = start; > unsigned long nr_pages = 0; > loff_t isize = i_size_read(inode); > gfp_t gfp_mask = readahead_gfp_mask(mapping); > + bool use_list = mapping->a_ops->readpages; > > if (isize == 0) > goto out; > @@ -170,7 +170,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * Preallocate as many pages as we will need. > */ > for (page_idx = 0; page_idx < nr_to_read; page_idx++) { > - pgoff_t page_offset = offset + page_idx; > + struct page *page; I see two distinct things happening in this patch, and I think they want to each be in their own patch: 1) A significant refactoring of the page loop, and 2) Changing the place where the page is added to the page cache. (Only this one is mentioned in the commit description.) We'll be more likely to spot any errors if these are teased apart. thanks, -- John Hubbard NVIDIA > > if (page_offset > end_index) > break; > @@ -178,25 +178,43 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > page = xa_load(&mapping->i_pages, page_offset); > if (page && !xa_is_value(page)) { > /* > - * Page already present? Kick off the current batch of > - * contiguous pages before continuing with the next > - * batch. > + * Page already present? Kick off the current batch > + * of contiguous pages before continuing with the > + * next batch. > + * It's possible this page is the page we should > + * be marking with PageReadahead. However, we > + * don't have a stable ref to this page so it might > + * be reallocated to another user before we can set > + * the bit. There's probably another page in the > + * cache marked with PageReadahead from the other > + * process which accessed this file. > */ > - if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, > - gfp_mask); > - nr_pages = 0; > - continue; > + goto skip; > } > > page = __page_cache_alloc(gfp_mask); > if (!page) > break; > - page->index = page_offset; > - list_add(&page->lru, &page_pool); > + if (use_list) { > + page->index = page_offset; > + list_add(&page->lru, &page_pool); > + } else if (add_to_page_cache_lru(page, mapping, page_offset, > + gfp_mask) < 0) { > + put_page(page); > + goto skip; > + } > + > if (page_idx == nr_to_read - lookahead_size) > SetPageReadahead(page); > nr_pages++; > + page_offset++; > + continue; > +skip: > + if (nr_pages) > + read_pages(mapping, filp, &page_pool, start, nr_pages); > + nr_pages = 0; > + page_offset++; > + start = page_offset; > } > > /* > @@ -205,7 +223,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * will then handle the error. > */ > if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, gfp_mask); > + read_pages(mapping, filp, &page_pool, start, nr_pages); > BUG_ON(!list_empty(&page_pool)); > out: > return nr_pages; > From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Hubbard Date: Thu, 13 Feb 2020 19:36:38 -0800 Subject: [Ocfs2-devel] [PATCH v5 03/13] mm: Put readahead pages in cache earlier In-Reply-To: <20200211010348.6872-4-willy@infradead.org> References: <20200211010348.6872-1-willy@infradead.org> <20200211010348.6872-4-willy@infradead.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ocfs2-devel@oss.oracle.com On 2/10/20 5:03 PM, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" > > At allocation time, put the pages in the cache unless we're using > ->readpages. > > Signed-off-by: Matthew Wilcox (Oracle) > --- > mm/readahead.c | 66 ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 24 deletions(-) > > diff --git a/mm/readahead.c b/mm/readahead.c > index fc77d13af556..96c6ca68a174 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -114,10 +114,10 @@ int read_cache_pages(struct address_space *mapping, struct list_head *pages, > EXPORT_SYMBOL(read_cache_pages); > > static void read_pages(struct address_space *mapping, struct file *filp, > - struct list_head *pages, unsigned int nr_pages, gfp_t gfp) > + struct list_head *pages, pgoff_t start, > + unsigned int nr_pages) > { > struct blk_plug plug; > - unsigned page_idx; > > blk_start_plug(&plug); > > @@ -125,18 +125,17 @@ static void read_pages(struct address_space *mapping, struct file *filp, > mapping->a_ops->readpages(filp, mapping, pages, nr_pages); > /* Clean up the remaining pages */ > put_pages_list(pages); > - goto out; > - } > + } else { > + struct page *page; > + unsigned long index; > > - for (page_idx = 0; page_idx < nr_pages; page_idx++) { > - struct page *page = lru_to_page(pages); > - list_del(&page->lru); > - if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) > + xa_for_each_range(&mapping->i_pages, index, page, start, > + start + nr_pages - 1) { > mapping->a_ops->readpage(filp, page); > - put_page(page); > + put_page(page); > + } > } > > -out: > blk_finish_plug(&plug); > } > > @@ -149,17 +148,18 @@ static void read_pages(struct address_space *mapping, struct file *filp, > * Returns the number of pages requested, or the maximum amount of I/O allowed. > */ > unsigned long __do_page_cache_readahead(struct address_space *mapping, > - struct file *filp, pgoff_t offset, unsigned long nr_to_read, > + struct file *filp, pgoff_t start, unsigned long nr_to_read, > unsigned long lookahead_size) > { > struct inode *inode = mapping->host; > - struct page *page; > unsigned long end_index; /* The last page we want to read */ > LIST_HEAD(page_pool); > int page_idx; > + pgoff_t page_offset = start; > unsigned long nr_pages = 0; > loff_t isize = i_size_read(inode); > gfp_t gfp_mask = readahead_gfp_mask(mapping); > + bool use_list = mapping->a_ops->readpages; > > if (isize == 0) > goto out; > @@ -170,7 +170,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * Preallocate as many pages as we will need. > */ > for (page_idx = 0; page_idx < nr_to_read; page_idx++) { > - pgoff_t page_offset = offset + page_idx; > + struct page *page; I see two distinct things happening in this patch, and I think they want to each be in their own patch: 1) A significant refactoring of the page loop, and 2) Changing the place where the page is added to the page cache. (Only this one is mentioned in the commit description.) We'll be more likely to spot any errors if these are teased apart. thanks, -- John Hubbard NVIDIA > > if (page_offset > end_index) > break; > @@ -178,25 +178,43 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > page = xa_load(&mapping->i_pages, page_offset); > if (page && !xa_is_value(page)) { > /* > - * Page already present? Kick off the current batch of > - * contiguous pages before continuing with the next > - * batch. > + * Page already present? Kick off the current batch > + * of contiguous pages before continuing with the > + * next batch. > + * It's possible this page is the page we should > + * be marking with PageReadahead. However, we > + * don't have a stable ref to this page so it might > + * be reallocated to another user before we can set > + * the bit. There's probably another page in the > + * cache marked with PageReadahead from the other > + * process which accessed this file. > */ > - if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, > - gfp_mask); > - nr_pages = 0; > - continue; > + goto skip; > } > > page = __page_cache_alloc(gfp_mask); > if (!page) > break; > - page->index = page_offset; > - list_add(&page->lru, &page_pool); > + if (use_list) { > + page->index = page_offset; > + list_add(&page->lru, &page_pool); > + } else if (add_to_page_cache_lru(page, mapping, page_offset, > + gfp_mask) < 0) { > + put_page(page); > + goto skip; > + } > + > if (page_idx == nr_to_read - lookahead_size) > SetPageReadahead(page); > nr_pages++; > + page_offset++; > + continue; > +skip: > + if (nr_pages) > + read_pages(mapping, filp, &page_pool, start, nr_pages); > + nr_pages = 0; > + page_offset++; > + start = page_offset; > } > > /* > @@ -205,7 +223,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * will then handle the error. > */ > if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, gfp_mask); > + read_pages(mapping, filp, &page_pool, start, nr_pages); > BUG_ON(!list_empty(&page_pool)); > out: > return nr_pages; > 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 X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8BC1AC35242 for ; Fri, 14 Feb 2020 03:36:49 +0000 (UTC) Received: from lists.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 58B0D206ED; Fri, 14 Feb 2020 03:36:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=sourceforge.net header.i=@sourceforge.net header.b="MfD/LjU8"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=sf.net header.i=@sf.net header.b="hAm3l3Yg"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="TZf2FK1b" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 58B0D206ED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nvidia.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-f2fs-devel-bounces@lists.sourceforge.net Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1j2Rma-000739-Hj; Fri, 14 Feb 2020 03:36:48 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1j2RmY-00072s-BP for linux-f2fs-devel@lists.sourceforge.net; Fri, 14 Feb 2020 03:36:46 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To: MIME-Version:Date:Message-ID:From:References:CC:To:Subject:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=jodX0tIvBxH/fc59T73o9SPk6S3e+yDpKWnaNwpeJKk=; b=MfD/LjU8iPB3A0pFMUldaUoxcw AiKoyDF20B7ANjAzY3tBFxNRE0umXx4CajIIQLMjZpJ8yzQ+CD2WT+QR7KDysHCpVogV42kcbgNJK axkZmibWNNjZk9JDUbF0B42pQT+ou4sS6dHxTnfvUu5XTa9HrMLAaE3yMepTNt3YGrcg=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version:Date: Message-ID:From:References:CC:To:Subject:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=jodX0tIvBxH/fc59T73o9SPk6S3e+yDpKWnaNwpeJKk=; b=hAm3l3YgkqdNUbLITVGSFdJ/I4 cbUBa2CaVGrNjV2O36AORWrTUByJ/ZQ2F55HC2qgh6VzvwbF/12hc7eanY0tC3IOZDR+FSD5dfYiR DcyFtawDxPpW1FktXJnWb19Rqj1dvxok13wbGTrmhHOeNjh7l3paYb/VAIakh/pmBr4c=; Received: from hqnvemgate26.nvidia.com ([216.228.121.65]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1j2RmW-003duO-Sg for linux-f2fs-devel@lists.sourceforge.net; Fri, 14 Feb 2020 03:36:46 +0000 Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Thu, 13 Feb 2020 19:36:25 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Thu, 13 Feb 2020 19:36:39 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Thu, 13 Feb 2020 19:36:39 -0800 Received: from [10.110.48.28] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 14 Feb 2020 03:36:39 +0000 To: Matthew Wilcox , References: <20200211010348.6872-1-willy@infradead.org> <20200211010348.6872-4-willy@infradead.org> X-Nvconfidentiality: public From: John Hubbard Message-ID: Date: Thu, 13 Feb 2020 19:36:38 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200211010348.6872-4-willy@infradead.org> X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To HQMAIL107.nvidia.com (172.20.187.13) Content-Language: en-US DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1581651385; bh=jodX0tIvBxH/fc59T73o9SPk6S3e+yDpKWnaNwpeJKk=; h=X-PGP-Universal:Subject:To:CC:References:X-Nvconfidentiality:From: Message-ID:Date:User-Agent:MIME-Version:In-Reply-To: X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=TZf2FK1bESe06GQrr3QWPstwI5CI6ADgd3ekfkEEBCxsznAzp/43OrXvLIdUwk6v9 4PvhXV9KA2GzBIO5FVXywx6Yx4JZowfMDze3Z/5j54bStQRS61QiScPwZCgRgQ9xzt 7ai4uLewCJr+qbdYGw8j6AC1KIuz1sHeXguO+TyPFfs/5Cwmjn/6ETP0ytHeXfze8x aZXVC6Py5XJwaXp8pJ7Om4/J3JPHPE/2VYV1PRSOmD236D2IEEGhBl9TBFg0OLQxCY GWBBoVBH3FzkluFiC7gp+exF/Tlgdi2RCth1YbNLQQ91aeVldSsZAblIBpXwiH2ihX ePKdIST/oKi4w== X-Headers-End: 1j2RmW-003duO-Sg Subject: Re: [f2fs-dev] [PATCH v5 03/13] mm: Put readahead pages in cache earlier X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-mm@kvack.org, ocfs2-devel@oss.oracle.com, linux-ext4@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-btrfs@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On 2/10/20 5:03 PM, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" > > At allocation time, put the pages in the cache unless we're using > ->readpages. > > Signed-off-by: Matthew Wilcox (Oracle) > --- > mm/readahead.c | 66 ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 24 deletions(-) > > diff --git a/mm/readahead.c b/mm/readahead.c > index fc77d13af556..96c6ca68a174 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -114,10 +114,10 @@ int read_cache_pages(struct address_space *mapping, struct list_head *pages, > EXPORT_SYMBOL(read_cache_pages); > > static void read_pages(struct address_space *mapping, struct file *filp, > - struct list_head *pages, unsigned int nr_pages, gfp_t gfp) > + struct list_head *pages, pgoff_t start, > + unsigned int nr_pages) > { > struct blk_plug plug; > - unsigned page_idx; > > blk_start_plug(&plug); > > @@ -125,18 +125,17 @@ static void read_pages(struct address_space *mapping, struct file *filp, > mapping->a_ops->readpages(filp, mapping, pages, nr_pages); > /* Clean up the remaining pages */ > put_pages_list(pages); > - goto out; > - } > + } else { > + struct page *page; > + unsigned long index; > > - for (page_idx = 0; page_idx < nr_pages; page_idx++) { > - struct page *page = lru_to_page(pages); > - list_del(&page->lru); > - if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) > + xa_for_each_range(&mapping->i_pages, index, page, start, > + start + nr_pages - 1) { > mapping->a_ops->readpage(filp, page); > - put_page(page); > + put_page(page); > + } > } > > -out: > blk_finish_plug(&plug); > } > > @@ -149,17 +148,18 @@ static void read_pages(struct address_space *mapping, struct file *filp, > * Returns the number of pages requested, or the maximum amount of I/O allowed. > */ > unsigned long __do_page_cache_readahead(struct address_space *mapping, > - struct file *filp, pgoff_t offset, unsigned long nr_to_read, > + struct file *filp, pgoff_t start, unsigned long nr_to_read, > unsigned long lookahead_size) > { > struct inode *inode = mapping->host; > - struct page *page; > unsigned long end_index; /* The last page we want to read */ > LIST_HEAD(page_pool); > int page_idx; > + pgoff_t page_offset = start; > unsigned long nr_pages = 0; > loff_t isize = i_size_read(inode); > gfp_t gfp_mask = readahead_gfp_mask(mapping); > + bool use_list = mapping->a_ops->readpages; > > if (isize == 0) > goto out; > @@ -170,7 +170,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * Preallocate as many pages as we will need. > */ > for (page_idx = 0; page_idx < nr_to_read; page_idx++) { > - pgoff_t page_offset = offset + page_idx; > + struct page *page; I see two distinct things happening in this patch, and I think they want to each be in their own patch: 1) A significant refactoring of the page loop, and 2) Changing the place where the page is added to the page cache. (Only this one is mentioned in the commit description.) We'll be more likely to spot any errors if these are teased apart. thanks, -- John Hubbard NVIDIA > > if (page_offset > end_index) > break; > @@ -178,25 +178,43 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > page = xa_load(&mapping->i_pages, page_offset); > if (page && !xa_is_value(page)) { > /* > - * Page already present? Kick off the current batch of > - * contiguous pages before continuing with the next > - * batch. > + * Page already present? Kick off the current batch > + * of contiguous pages before continuing with the > + * next batch. > + * It's possible this page is the page we should > + * be marking with PageReadahead. However, we > + * don't have a stable ref to this page so it might > + * be reallocated to another user before we can set > + * the bit. There's probably another page in the > + * cache marked with PageReadahead from the other > + * process which accessed this file. > */ > - if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, > - gfp_mask); > - nr_pages = 0; > - continue; > + goto skip; > } > > page = __page_cache_alloc(gfp_mask); > if (!page) > break; > - page->index = page_offset; > - list_add(&page->lru, &page_pool); > + if (use_list) { > + page->index = page_offset; > + list_add(&page->lru, &page_pool); > + } else if (add_to_page_cache_lru(page, mapping, page_offset, > + gfp_mask) < 0) { > + put_page(page); > + goto skip; > + } > + > if (page_idx == nr_to_read - lookahead_size) > SetPageReadahead(page); > nr_pages++; > + page_offset++; > + continue; > +skip: > + if (nr_pages) > + read_pages(mapping, filp, &page_pool, start, nr_pages); > + nr_pages = 0; > + page_offset++; > + start = page_offset; > } > > /* > @@ -205,7 +223,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * will then handle the error. > */ > if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, gfp_mask); > + read_pages(mapping, filp, &page_pool, start, nr_pages); > BUG_ON(!list_empty(&page_pool)); > out: > return nr_pages; > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel 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 X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 84F90C35242 for ; Fri, 14 Feb 2020 03:36:51 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 361EE206ED for ; Fri, 14 Feb 2020 03:36:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="TZf2FK1b" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 361EE206ED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nvidia.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-erofs-bounces+linux-erofs=archiver.kernel.org@lists.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 48JfFF2QYTzDqXl for ; Fri, 14 Feb 2020 14:36:49 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nvidia.com (client-ip=216.228.121.65; helo=hqnvemgate26.nvidia.com; envelope-from=jhubbard@nvidia.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=nvidia.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=nvidia.com header.i=@nvidia.com header.a=rsa-sha256 header.s=n1 header.b=TZf2FK1b; dkim-atps=neutral Received: from hqnvemgate26.nvidia.com (hqnvemgate26.nvidia.com [216.228.121.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 48JfF80dhRzDqX9 for ; Fri, 14 Feb 2020 14:36:43 +1100 (AEDT) Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Thu, 13 Feb 2020 19:36:25 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Thu, 13 Feb 2020 19:36:39 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Thu, 13 Feb 2020 19:36:39 -0800 Received: from [10.110.48.28] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 14 Feb 2020 03:36:39 +0000 Subject: Re: [PATCH v5 03/13] mm: Put readahead pages in cache earlier To: Matthew Wilcox , References: <20200211010348.6872-1-willy@infradead.org> <20200211010348.6872-4-willy@infradead.org> X-Nvconfidentiality: public From: John Hubbard Message-ID: Date: Thu, 13 Feb 2020 19:36:38 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200211010348.6872-4-willy@infradead.org> X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To HQMAIL107.nvidia.com (172.20.187.13) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1581651385; bh=jodX0tIvBxH/fc59T73o9SPk6S3e+yDpKWnaNwpeJKk=; h=X-PGP-Universal:Subject:To:CC:References:X-Nvconfidentiality:From: Message-ID:Date:User-Agent:MIME-Version:In-Reply-To: X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=TZf2FK1bESe06GQrr3QWPstwI5CI6ADgd3ekfkEEBCxsznAzp/43OrXvLIdUwk6v9 4PvhXV9KA2GzBIO5FVXywx6Yx4JZowfMDze3Z/5j54bStQRS61QiScPwZCgRgQ9xzt 7ai4uLewCJr+qbdYGw8j6AC1KIuz1sHeXguO+TyPFfs/5Cwmjn/6ETP0ytHeXfze8x aZXVC6Py5XJwaXp8pJ7Om4/J3JPHPE/2VYV1PRSOmD236D2IEEGhBl9TBFg0OLQxCY GWBBoVBH3FzkluFiC7gp+exF/Tlgdi2RCth1YbNLQQ91aeVldSsZAblIBpXwiH2ihX ePKdIST/oKi4w== X-BeenThere: linux-erofs@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development of Linux EROFS file system List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-mm@kvack.org, ocfs2-devel@oss.oracle.com, linux-ext4@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-btrfs@vger.kernel.org Errors-To: linux-erofs-bounces+linux-erofs=archiver.kernel.org@lists.ozlabs.org Sender: "Linux-erofs" On 2/10/20 5:03 PM, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" > > At allocation time, put the pages in the cache unless we're using > ->readpages. > > Signed-off-by: Matthew Wilcox (Oracle) > --- > mm/readahead.c | 66 ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 24 deletions(-) > > diff --git a/mm/readahead.c b/mm/readahead.c > index fc77d13af556..96c6ca68a174 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -114,10 +114,10 @@ int read_cache_pages(struct address_space *mapping, struct list_head *pages, > EXPORT_SYMBOL(read_cache_pages); > > static void read_pages(struct address_space *mapping, struct file *filp, > - struct list_head *pages, unsigned int nr_pages, gfp_t gfp) > + struct list_head *pages, pgoff_t start, > + unsigned int nr_pages) > { > struct blk_plug plug; > - unsigned page_idx; > > blk_start_plug(&plug); > > @@ -125,18 +125,17 @@ static void read_pages(struct address_space *mapping, struct file *filp, > mapping->a_ops->readpages(filp, mapping, pages, nr_pages); > /* Clean up the remaining pages */ > put_pages_list(pages); > - goto out; > - } > + } else { > + struct page *page; > + unsigned long index; > > - for (page_idx = 0; page_idx < nr_pages; page_idx++) { > - struct page *page = lru_to_page(pages); > - list_del(&page->lru); > - if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) > + xa_for_each_range(&mapping->i_pages, index, page, start, > + start + nr_pages - 1) { > mapping->a_ops->readpage(filp, page); > - put_page(page); > + put_page(page); > + } > } > > -out: > blk_finish_plug(&plug); > } > > @@ -149,17 +148,18 @@ static void read_pages(struct address_space *mapping, struct file *filp, > * Returns the number of pages requested, or the maximum amount of I/O allowed. > */ > unsigned long __do_page_cache_readahead(struct address_space *mapping, > - struct file *filp, pgoff_t offset, unsigned long nr_to_read, > + struct file *filp, pgoff_t start, unsigned long nr_to_read, > unsigned long lookahead_size) > { > struct inode *inode = mapping->host; > - struct page *page; > unsigned long end_index; /* The last page we want to read */ > LIST_HEAD(page_pool); > int page_idx; > + pgoff_t page_offset = start; > unsigned long nr_pages = 0; > loff_t isize = i_size_read(inode); > gfp_t gfp_mask = readahead_gfp_mask(mapping); > + bool use_list = mapping->a_ops->readpages; > > if (isize == 0) > goto out; > @@ -170,7 +170,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * Preallocate as many pages as we will need. > */ > for (page_idx = 0; page_idx < nr_to_read; page_idx++) { > - pgoff_t page_offset = offset + page_idx; > + struct page *page; I see two distinct things happening in this patch, and I think they want to each be in their own patch: 1) A significant refactoring of the page loop, and 2) Changing the place where the page is added to the page cache. (Only this one is mentioned in the commit description.) We'll be more likely to spot any errors if these are teased apart. thanks, -- John Hubbard NVIDIA > > if (page_offset > end_index) > break; > @@ -178,25 +178,43 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > page = xa_load(&mapping->i_pages, page_offset); > if (page && !xa_is_value(page)) { > /* > - * Page already present? Kick off the current batch of > - * contiguous pages before continuing with the next > - * batch. > + * Page already present? Kick off the current batch > + * of contiguous pages before continuing with the > + * next batch. > + * It's possible this page is the page we should > + * be marking with PageReadahead. However, we > + * don't have a stable ref to this page so it might > + * be reallocated to another user before we can set > + * the bit. There's probably another page in the > + * cache marked with PageReadahead from the other > + * process which accessed this file. > */ > - if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, > - gfp_mask); > - nr_pages = 0; > - continue; > + goto skip; > } > > page = __page_cache_alloc(gfp_mask); > if (!page) > break; > - page->index = page_offset; > - list_add(&page->lru, &page_pool); > + if (use_list) { > + page->index = page_offset; > + list_add(&page->lru, &page_pool); > + } else if (add_to_page_cache_lru(page, mapping, page_offset, > + gfp_mask) < 0) { > + put_page(page); > + goto skip; > + } > + > if (page_idx == nr_to_read - lookahead_size) > SetPageReadahead(page); > nr_pages++; > + page_offset++; > + continue; > +skip: > + if (nr_pages) > + read_pages(mapping, filp, &page_pool, start, nr_pages); > + nr_pages = 0; > + page_offset++; > + start = page_offset; > } > > /* > @@ -205,7 +223,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * will then handle the error. > */ > if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, gfp_mask); > + read_pages(mapping, filp, &page_pool, start, nr_pages); > BUG_ON(!list_empty(&page_pool)); > out: > return nr_pages; > From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Hubbard Date: Thu, 13 Feb 2020 19:36:38 -0800 Subject: [Cluster-devel] [PATCH v5 03/13] mm: Put readahead pages in cache earlier In-Reply-To: <20200211010348.6872-4-willy@infradead.org> References: <20200211010348.6872-1-willy@infradead.org> <20200211010348.6872-4-willy@infradead.org> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 2/10/20 5:03 PM, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" > > At allocation time, put the pages in the cache unless we're using > ->readpages. > > Signed-off-by: Matthew Wilcox (Oracle) > --- > mm/readahead.c | 66 ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 24 deletions(-) > > diff --git a/mm/readahead.c b/mm/readahead.c > index fc77d13af556..96c6ca68a174 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -114,10 +114,10 @@ int read_cache_pages(struct address_space *mapping, struct list_head *pages, > EXPORT_SYMBOL(read_cache_pages); > > static void read_pages(struct address_space *mapping, struct file *filp, > - struct list_head *pages, unsigned int nr_pages, gfp_t gfp) > + struct list_head *pages, pgoff_t start, > + unsigned int nr_pages) > { > struct blk_plug plug; > - unsigned page_idx; > > blk_start_plug(&plug); > > @@ -125,18 +125,17 @@ static void read_pages(struct address_space *mapping, struct file *filp, > mapping->a_ops->readpages(filp, mapping, pages, nr_pages); > /* Clean up the remaining pages */ > put_pages_list(pages); > - goto out; > - } > + } else { > + struct page *page; > + unsigned long index; > > - for (page_idx = 0; page_idx < nr_pages; page_idx++) { > - struct page *page = lru_to_page(pages); > - list_del(&page->lru); > - if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) > + xa_for_each_range(&mapping->i_pages, index, page, start, > + start + nr_pages - 1) { > mapping->a_ops->readpage(filp, page); > - put_page(page); > + put_page(page); > + } > } > > -out: > blk_finish_plug(&plug); > } > > @@ -149,17 +148,18 @@ static void read_pages(struct address_space *mapping, struct file *filp, > * Returns the number of pages requested, or the maximum amount of I/O allowed. > */ > unsigned long __do_page_cache_readahead(struct address_space *mapping, > - struct file *filp, pgoff_t offset, unsigned long nr_to_read, > + struct file *filp, pgoff_t start, unsigned long nr_to_read, > unsigned long lookahead_size) > { > struct inode *inode = mapping->host; > - struct page *page; > unsigned long end_index; /* The last page we want to read */ > LIST_HEAD(page_pool); > int page_idx; > + pgoff_t page_offset = start; > unsigned long nr_pages = 0; > loff_t isize = i_size_read(inode); > gfp_t gfp_mask = readahead_gfp_mask(mapping); > + bool use_list = mapping->a_ops->readpages; > > if (isize == 0) > goto out; > @@ -170,7 +170,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * Preallocate as many pages as we will need. > */ > for (page_idx = 0; page_idx < nr_to_read; page_idx++) { > - pgoff_t page_offset = offset + page_idx; > + struct page *page; I see two distinct things happening in this patch, and I think they want to each be in their own patch: 1) A significant refactoring of the page loop, and 2) Changing the place where the page is added to the page cache. (Only this one is mentioned in the commit description.) We'll be more likely to spot any errors if these are teased apart. thanks, -- John Hubbard NVIDIA > > if (page_offset > end_index) > break; > @@ -178,25 +178,43 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > page = xa_load(&mapping->i_pages, page_offset); > if (page && !xa_is_value(page)) { > /* > - * Page already present? Kick off the current batch of > - * contiguous pages before continuing with the next > - * batch. > + * Page already present? Kick off the current batch > + * of contiguous pages before continuing with the > + * next batch. > + * It's possible this page is the page we should > + * be marking with PageReadahead. However, we > + * don't have a stable ref to this page so it might > + * be reallocated to another user before we can set > + * the bit. There's probably another page in the > + * cache marked with PageReadahead from the other > + * process which accessed this file. > */ > - if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, > - gfp_mask); > - nr_pages = 0; > - continue; > + goto skip; > } > > page = __page_cache_alloc(gfp_mask); > if (!page) > break; > - page->index = page_offset; > - list_add(&page->lru, &page_pool); > + if (use_list) { > + page->index = page_offset; > + list_add(&page->lru, &page_pool); > + } else if (add_to_page_cache_lru(page, mapping, page_offset, > + gfp_mask) < 0) { > + put_page(page); > + goto skip; > + } > + > if (page_idx == nr_to_read - lookahead_size) > SetPageReadahead(page); > nr_pages++; > + page_offset++; > + continue; > +skip: > + if (nr_pages) > + read_pages(mapping, filp, &page_pool, start, nr_pages); > + nr_pages = 0; > + page_offset++; > + start = page_offset; > } > > /* > @@ -205,7 +223,7 @@ unsigned long __do_page_cache_readahead(struct address_space *mapping, > * will then handle the error. > */ > if (nr_pages) > - read_pages(mapping, filp, &page_pool, nr_pages, gfp_mask); > + read_pages(mapping, filp, &page_pool, start, nr_pages); > BUG_ON(!list_empty(&page_pool)); > out: > return nr_pages; >