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 F30A0C52D7C for ; Tue, 13 Aug 2024 19:58:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 87C926B0082; Tue, 13 Aug 2024 15:58:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 82BCB6B0083; Tue, 13 Aug 2024 15:58:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 71A266B0085; Tue, 13 Aug 2024 15:58:20 -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 5531B6B0082 for ; Tue, 13 Aug 2024 15:58:20 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CFCB880A72 for ; Tue, 13 Aug 2024 19:58:19 +0000 (UTC) X-FDA: 82448283918.17.ADC7A0F Received: from out-171.mta1.migadu.com (out-171.mta1.migadu.com [95.215.58.171]) by imf05.hostedemail.com (Postfix) with ESMTP id EAC87100004 for ; Tue, 13 Aug 2024 19:58:16 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=gBchu9dB; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf05.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.171 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723579018; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=2DeqsUQeTctJ1evkgBhhV7kkVgKg1hWqpw05981bLOE=; b=c5OAF7jzG68iQyRoKYbkmR3B9IGcrlhGMy4UGC4fOYRQDMWn5a4bn2hfTNBmQiqE7y89HP jJqLVSCXm18A8CbUM05++ylQSJb5RssQsJSEhLMd3NDTCn1+NQaEj3ajxrWP7fb4hZZIK7 ix7XMDzv4vmi3cdeL06Lw1KQqUI655s= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723579018; a=rsa-sha256; cv=none; b=CDvf9567nDDVIg1CW3siYmCAt/BOSTB3pazXEmCDOHvjgLsh+T5uMU1G/vjKrk2ixAaZkZ xe+XCanVjwucEZvSFi13qqkHyQ04NmbEqCiHcjDBjJzogdHV6Q/DnKKfAKTvpIF7D6abkV dReTbIimCOychJVSmGk+QcajOebf5lg= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=gBchu9dB; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf05.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.171 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev Date: Tue, 13 Aug 2024 12:58:09 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1723579094; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2DeqsUQeTctJ1evkgBhhV7kkVgKg1hWqpw05981bLOE=; b=gBchu9dBnTFGChi9jNihFGO3hfifMJU88ipENiov0rvZDgRnQ5+IKmlD1jiroB/YiM2PR5 j9lsT1kYR1nqYttI2fT3wbPbG2K4hAKXQbx2S3htnPHcrtxFXBfYodyAcboBb38q3PyuAB ydObxDq8qfq9SU4IUkQJTMQPxsXh1xQ= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Boris Burkov Cc: linux-mm@kvack.org, willy@infradead.org, linmiaohe@huawei.com Subject: Re: [PATCH RFC] mm: fix refcount check in mapping_evict_folio Message-ID: <5qxfn7y43mf6vkkk6adfgenilahe5uykscz7muxq7tjmjijxg3@dpshd4mrcxgn> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: EAC87100004 X-Stat-Signature: uk48jx4s8d53yk3du9tkuwzfsnyak1b6 X-Rspam-User: X-HE-Tag: 1723579096-41406 X-HE-Meta: U2FsdGVkX1/lbUg2yyBFi0wypbTq6LHyZ+uqTCJU8bV79xQ5dZ5dJ0+KTlS+32ZNB84S+nDRZJAPtBw8viuUh9zm9AGEpyjGG8uSqTwh8BjdRMW1QqsP2B2BtjRC9rcXUJnnhpC+bpVxc146wJ+j9/ANJKUip2z6UYk5NRkOMKKLqAaYdF/CSq4D2bPYgfBxuKJboFbpDMs8Bz8s53EuMVNgNil5InyK4sv5akB4ApQrYQVcFIxOphf+0K1p9zOJQeVsSgkGE0ZjZaX/nZhhGuFZ/s6Km5e6Z1R4wcYb0J/uTD34JnNPqYp5Cqxp2GhpOXGTlkFaGHcP/9z5pFWtqRZ21VyViEP7+upY+hM6WhxiMRfhShUSrCiBfo5JuHBVaKgjbk0YvX0ukxfi1dHzYB2SXRuYbNkuqxD0KeqOBa8iHhYSJMuGWHQLf/vgRi0ZdgdJPXCU/GaspBiBRKzunxyaHM7CbU1XjTF5jjuO/6Og3RWMsXtRcZgYUdWpPlRcwIS9hRKoPgJxjOnJFd4CXuwth0vUhAsFy9/o4q3RZ2codlqSKMQyznxa8yX0gMTIU5lSaIYMVUYMpszJFqNVwKrNc2yGE1oeq8FbEy3cBKAiZfSFm+BtsIYfUqTv0uFpSH4iOcHCpu8CCcn1HX3O9Qhx2ObYfZw0RJQgvfJLWq3emHeNHrhEa/K+Eob9Vb+e7xxh36edP0pc8GjypMsTR9i42OdrUJY0fqLK+PrjPrm3Mi9eCB4J6+7l9WoIUsmBMNZ+46nI9aq2MyI50JfT1z1xT4z3LZ3afheVzxngcIv7Xl7wJ3hVqWOkHFese5MW92PYVfw8N3fZOCccSH9dKJBrXTWEQCQGUdAJtu0Ul9/ZpPlQ93CSsjQoGQKC39cODpK3rRJaEe7KyK4SNfIUVf8HynC6vgx/S/JHCC9fSft4qtmaJxWljKGyEke99GIs0MlpORaMRtY82i171SR tpWngo5U ylhuvhdFVxE/tMSgRctkg7JfpRMr7G8wIzQyesguU4GANeIvCw/Cd70mTm4/tc/2FzKbwQWwPkSZ2V4XTKxZ+BO7JWeZ1Ej5IS6y4gNNuGShqV2pci+1/+Z1Uo94qQA9GP2gVu5/U8fUslemhv9P4D+Bi/aKsxFADu6CPYmgBUkqvJOKZp92unwH+YA== 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: List-Subscribe: List-Unsubscribe: CCing Miaohe Lin On Tue, Aug 13, 2024 at 11:25:56AM GMT, Boris Burkov wrote: > The commit e41c81d0d30e ("mm/truncate: Replace page_mapped() call in > invalidate_inode_page()") replaced the page_mapped(page) check with a > refcount check. However, this refcount check does not work as expected > with drop_caches, at least for btrfs's metadata pages. > > Btrfs has a per-sb metadata inode with cached pages, and when not in > active use by btrfs, they have a refcount of 3. One from the initial > call to alloc_pages, one (nr_pages == 1) from filemap_add_folio, and one > from folio_attach_private. We would expect such pages to get dropped by > drop_caches. However, drop_caches calls into mapping_evict_folio via > mapping_try_invalidate which gets a reference on the folio with > find_lock_entries(). As a result, these pages have a refcount of 4, and > fail this check. > > For what it's worth, such pages do get reclaimed under memory pressure, > and if I change this refcount check to `if folio_mapped(folio)` > > The following script produces such pages and uses drgn to further > analyze the state of the folios: > https://github.com/boryas/scripts/blob/main/sh/strand-meta/run.sh > It should at least outline the basic idea for producing some btrfs > metadata via creating inlined-extent files. > > My proposed fix for the issue is to add one more hardcoded refcount to > this check to account for the caller having a refcount on the page. > However, I am less familiar with the other caller into > mapping_evict_folio in the page fault path, so I am concerned this fix > will not work properly there, and would appreciate extra scrutiny there. > > Fixes: e41c81d0d30e ("mm/truncate: Replace page_mapped() call in invalidate_inode_page()") > Signed-off-by: Boris Burkov > --- > mm/truncate.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/mm/truncate.c b/mm/truncate.c > index 4d61fbdd4b2f..c710c84710b4 100644 > --- a/mm/truncate.c > +++ b/mm/truncate.c > @@ -267,9 +267,17 @@ long mapping_evict_folio(struct address_space *mapping, struct folio *folio) > return 0; > if (folio_test_dirty(folio) || folio_test_writeback(folio)) > return 0; > - /* The refcount will be elevated if any page in the folio is mapped */ > + /* > + * The refcount will be elevated if any page in the folio is mapped. > + * > + * The refcounts break down as follows: > + * 1 per mapped page > + * 1 from folio_attach_private, if private is set > + * 1 from allocating the page in the first place > + * 1 from the caller > + */ I think the above explanation is correct at least from my code inspection. Most of the callers are related to memory failure. I would reword the "1 per mapped page" to "1 per page in page cache" or something as mapped here might mean mapped in page tables. > if (folio_ref_count(folio) > > - folio_nr_pages(folio) + folio_has_private(folio) + 1) > + folio_nr_pages(folio) + folio_has_private(folio) + 1 + 1) > return 0; > if (!filemap_release_folio(folio, 0)) > return 0; > -- > 2.46.0 > >