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 CCD21C433EF for ; Tue, 7 Jun 2022 14:19:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4CA0A8D0003; Tue, 7 Jun 2022 10:19:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 478758D0001; Tue, 7 Jun 2022 10:19:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 367278D0003; Tue, 7 Jun 2022 10:19:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 26CC78D0001 for ; Tue, 7 Jun 2022 10:19:38 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id EE9F180C29 for ; Tue, 7 Jun 2022 14:19:37 +0000 (UTC) X-FDA: 79551647994.21.E093B50 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf10.hostedemail.com (Postfix) with ESMTP id A6742C006C for ; Tue, 7 Jun 2022 14:18:46 +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=FMkgIVtfc6b3ABx6BakkG50qZIp134U+1xyaQI6WTD8=; b=k8JbLNQUimojEC6ifvDTH5sfBP fpvni16t938kNcvgN3NGuZMkLpTFRwZPmW+HZ/szX/M1gGJT13+XYYDZNUOwIf+qlQUkEk+K4u8Pk 8dxFvwbIbB7bjovt9aU+7G9tZVK4BB4Cl1kMQw5IdwzDWJqPG3XNsEXvqWW06fxwhPqcugWKR5B4T TQxRRM9rGeaaKHQlYG5seTLr3t7WtmgAhc0olggJu5sWL3LNwot8Mc1jH0IPRY/idksvHnw7yTP9o W7g1ZyAaLTf8Nafz1asHScega2Y5dCD87Sm2x7vmWBBVnjiLLH8HfL4/qx5iWzbEsrBxZJBaAhjvE ByDDgtUg==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nyZm6-00Bgqd-J3; Tue, 07 Jun 2022 14:01:38 +0000 Date: Tue, 7 Jun 2022 15:01:38 +0100 From: Matthew Wilcox To: Alistair Popple Cc: akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/filemap.c: Always read one page in do_sync_mmap_readahead() Message-ID: References: <20220607083714.183788-1-apopple@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220607083714.183788-1-apopple@nvidia.com> Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=k8JbLNQU; dmarc=none; spf=none (imf10.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: A6742C006C X-Rspam-User: X-Stat-Signature: wkphdr9br4576ca6pz5gtoqmeexf6kkw X-HE-Tag: 1654611526-545592 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, Jun 07, 2022 at 06:37:14PM +1000, Alistair Popple wrote: > --- > include/linux/pagemap.h | 7 +++--- > mm/filemap.c | 47 +++++++++++++---------------------------- > 2 files changed, 18 insertions(+), 36 deletions(-) Love the diffstat ;-) > @@ -3011,14 +3001,8 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf) > } > #endif > > - /* If we don't want any read-ahead, don't bother */ > - if (vmf->vma->vm_flags & VM_RAND_READ) > - return fpin; > - if (!ra->ra_pages) > - return fpin; > - > + fpin = maybe_unlock_mmap_for_io(vmf, fpin); > if (vmf->vma->vm_flags & VM_SEQ_READ) { > - fpin = maybe_unlock_mmap_for_io(vmf, fpin); > page_cache_sync_ra(&ractl, ra->ra_pages); > return fpin; > } Good. Could even pull the maybe_unlock_mmap_for_io() all the way to the top of the file and remove it from the VM_HUGEPAGE case? > @@ -3029,19 +3013,20 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf) > WRITE_ONCE(ra->mmap_miss, ++mmap_miss); > > /* > - * Do we miss much more than hit in this file? If so, > - * stop bothering with read-ahead. It will only hurt. > + * mmap read-around. If we don't want any read-ahead or if we miss more > + * than we hit don't bother with read-ahead and just read a single page. > */ > - if (mmap_miss > MMAP_LOTSAMISS) > - return fpin; > + if ((vmf->vma->vm_flags & VM_RAND_READ) || > + !ra->ra_pages || mmap_miss > MMAP_LOTSAMISS) { > + ra->start = vmf->pgoff; > + ra->size = 1; > + ra->async_size = 0; > + } else { I'd put the: /* mmap read-around */ here > + ra->start = max_t(long, 0, vmf->pgoff - ra->ra_pages / 2); > + ra->size = ra->ra_pages; > + ra->async_size = ra->ra_pages / 4; > + } > > - /* > - * mmap read-around > - */ > - fpin = maybe_unlock_mmap_for_io(vmf, fpin); > - ra->start = max_t(long, 0, vmf->pgoff - ra->ra_pages / 2); > - ra->size = ra->ra_pages; > - ra->async_size = ra->ra_pages / 4; > ractl._index = ra->start; > page_cache_ra_order(&ractl, ra, 0); > return fpin; > @@ -3145,9 +3130,7 @@ vm_fault_t filemap_fault(struct vm_fault *vmf) > filemap_invalidate_lock_shared(mapping); > mapping_locked = true; > } > - folio = __filemap_get_folio(mapping, index, > - FGP_CREAT|FGP_FOR_MMAP, > - vmf->gfp_mask); > + folio = filemap_get_folio(mapping, index); > if (!folio) { > if (fpin) > goto out_retry; I think we also should remove the filemap_invalidate_lock_shared() here, no? We also need to handle the !folio case differently. Before, if it was gone, that was definitely an OOM. Now if it's gone it might have been truncated, or removed due to memory pressure, or it might be an OOM situation where readahead didn't manage to create the folio.