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 9337AC6FD1D for ; Mon, 27 Mar 2023 16:22:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA193900003; Mon, 27 Mar 2023 12:22:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D2A64900002; Mon, 27 Mar 2023 12:22:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BCB04900003; Mon, 27 Mar 2023 12:22:46 -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 A7D6B900002 for ; Mon, 27 Mar 2023 12:22:46 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 71819160766 for ; Mon, 27 Mar 2023 16:22:46 +0000 (UTC) X-FDA: 80615196732.24.1194CC5 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf15.hostedemail.com (Postfix) with ESMTP id A100FA0017 for ; Mon, 27 Mar 2023 16:22:43 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="tbNGl/HF"; spf=none (imf15.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1679934164; a=rsa-sha256; cv=none; b=JJvJdLgg+q+CrVOP3+as/6slS2UW3cXSGuCwQsH7wqrTGI/LibnmkwgR4u8VNJJZEqxSf1 LhzPpNnjR8Cmxne8f4rvxsgSUNFLHXWPRS+v2NF1HB7pRE5XBB4ABBR91X9XOXEkOJAQPK C99boLG7J2OlvXOed7AUXkGjqxTx1z4= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="tbNGl/HF"; spf=none (imf15.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1679934164; 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=5gk+dREm5bLSsqD3YSf3yyJg/yMmvA9kxHAGuA6QyKw=; b=k0gQGvfszeMgWsyz0D1hFv3Q/pEcGhj5q7B4sfWLqhBpcKRcL0or5yhCV6hJvr+VId46sa 96dC/1xFoTpTs6YEUze5O6NrL62FUgmwSblJriH0GX2pkPWVhcnnh2QZyytO742kUcdmAg IlKsdtbxh6JT69bCcbry2A8bwRfs52g= 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=5gk+dREm5bLSsqD3YSf3yyJg/yMmvA9kxHAGuA6QyKw=; b=tbNGl/HFxlKsfgEwkgvNpIQn1V EdFym+Rjoi4yvX/tWDfz8RvIXF9lhk1mHVi+3eFpp7xfuz2NYmQBhx2Z5xIkl/XI4rtizrcOh5466 iN0epzVbsqEQaqiskLE4Ug2xJgG+h43clnv8CDF/ky7GbSnm6EtS2+P5oAuGwZFjZFKuJMLNC7fwP v7/LVa4xzJSmuP5A8LfURjrU5rU9F7nHAzjJrB6xCpFrq+NTNqckBPoVcfSaLSq5eKvTm8wnM88AP LlTAZN9+V2xZRHD03aJoBvsD/bHdkQMoqOyfrCX5G1CGvM8FSDu4aE6t2Lt1+DA7wghy4HTX71opC LjtvfiNw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pgpcA-007Y5s-B2; Mon, 27 Mar 2023 16:22:34 +0000 Date: Mon, 27 Mar 2023 17:22:34 +0100 From: Matthew Wilcox To: Jaegeuk Kim Cc: Chao Yu , linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, stable@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Subject: Re: [f2fs-dev] [PATCH] f2fs: get out of a repeat loop when getting a locked data page Message-ID: References: <20230323213919.1876157-1-jaegeuk@kernel.org> <8aea02b0-86f9-539a-02e9-27b381e68b66@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: A100FA0017 X-Rspamd-Server: rspam01 X-Stat-Signature: 5wyogi3yyawtof7etxwfogxfy7agh4wg X-HE-Tag: 1679934163-266770 X-HE-Meta: U2FsdGVkX18wBLGb7p7fZNORHs2kvYsS2LoxAzVwDVbIYDvfxFv4IGRG3CxHErggkStToo/XGAvkq64tR+8MB3JwXmrrVkqjp0MGpGo7b6vdmudf0naiwHuuuosgzQ+wbGxJWYMLiel/fsLKeDE37gY9cec9uLU7CzJ1UJJENtRvtVKp7EQ4kWSdrfMvgz67X5t90JyHp4RPp7VAkeDCw7uuD4no5rDj76f1Qfl5wgvdtXlUleGK0/QI936IrUlL9Y2hEz2GJ7sucC0sx9rpb+284YqqpUQFrS6fFfRbcU8azRFIu1hXZV2yh/n4ozkngig5sO78eUNdKsdQwqbAn2KgMawHaxmzhcdFIOc8ecM1GbdauQoFYxy1gPFYHXAwRQkUEJMKvF7JND24WJmbIyhaQw2ZmO2tNjr367kwGe/LkYNgIU0tt9asqQUlFeSRHXFLm5TsWH+u5qxFoTBWUbq/YOm+/gj5veJ0zPKjovZn0xk4lCspH5E7F9Y0Abbc9ViaiKM+Ki74IRPKMmEh9UpZCPYwvqcLWZhvNIbz825E8I0jBjhYB+pRQIW9mjeV2gtiob0oBjukaqg//ZM2m+l1m2RJ1ReeU/Fmiy8Nu8WpVfZxyoMF4RkKH6k3jVgxHeufwyP1CnoDxvWO4ILU4GFUpfTw918fST0B+8kDILUgzfTdCVz9IJLqmwj5CV2E1jSpzYc6OvqCyLQX/uxV0zD401fTqOVz5Tf5LJKXrIn4TqShB5YdmDuCsBTv3qRU5c9Nx18oeckcURpUYzsbVIG9Jv36E/L2hNgdvRld0duB8bXJPRtqazpwkQTfn5RpyAdi6DB/ytOY+pc7W3TmIlpi5HM6Dqk7VF31sZzg8nPyEQjud2oKet+hS4TOffd/MQLPRYM05g2ARD/5hOpkI4VhM8a9aQ4ZBlXPZeAmZwWrpptNyzg0Xiu0/TYYf1ZS/0MT8i9gpcgrJTRZ+tf ZDaxDuHL 2x9LAY/VASFES+Wqs1/hf3YCQa796ZceFi8/d1j9JfXcnY9I7Q4au9KvurNnwFRKdN5hpUJ0UivVyiae7ACwtj9wNdnZMGCZ0piEr1rsGTVcquh22c0DZvHDk4E7MMAQ0T+tnmn74+RXOKK8vqfEkTgSBUHPl1rkNZWjnIX6527QOgeBJmuCD9O2wtEQlu8N8+oHB74QYXbSMH81V/P9236QSfzEFF0EFaDnWUx9/n8xvDs+1a+8r6bxU6ZixnwG6M99Y8svmxr+nuQRFiEmmShnSdQ== 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 Mon, Mar 27, 2023 at 08:30:33AM -0700, Jaegeuk Kim wrote: > On 03/26, Chao Yu wrote: > > On 2023/3/24 5:39, Jaegeuk Kim wrote: > > > https://bugzilla.kernel.org/show_bug.cgi?id=216050 > > > > > > Somehow we're getting a page which has a different mapping. > > > Let's avoid the infinite loop. > > > > > > Cc: > > > Signed-off-by: Jaegeuk Kim > > > --- > > > fs/f2fs/data.c | 8 ++------ > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > > index bf51e6e4eb64..80702c93e885 100644 > > > --- a/fs/f2fs/data.c > > > +++ b/fs/f2fs/data.c > > > @@ -1329,18 +1329,14 @@ struct page *f2fs_get_lock_data_page(struct inode *inode, pgoff_t index, > > > { > > > struct address_space *mapping = inode->i_mapping; > > > struct page *page; > > > -repeat: > > > + > > > page = f2fs_get_read_data_page(inode, index, 0, for_write, NULL); > > > if (IS_ERR(page)) > > > return page; > > > /* wait for read completion */ > > > lock_page(page); > > > - if (unlikely(page->mapping != mapping)) { > > > > How about using such logic only for move_data_page() to limit affect for > > other paths? > > Why move_data_page() only? If this happens, we'll fall into a loop in anywhere? > > > > > Jaegeuk, any thoughts about why mapping is mismatch in between page's one and > > inode->i_mapping? > > > > > After several times code review, I didn't get any clue about why f2fs always > > get the different mapping in a loop. > > I couldn't find the path to happen this. So weird. Please check the history in the > bug. > > > > > Maybe we can loop MM guys to check whether below folio_file_page() may return > > page which has different mapping? > > Matthew may have some idea on this? There's a lot of comments in the bug ... hard to come into this one cold. I did notice this one (#119): : Interestingly, ref count is 514, which looks suspiciously as a binary : flag 1000000010. Is it possible that during 5.17/5.18 implementation : of a "pin", somehow binary flag was written to ref count, or something : like '1 << ...' happens? That indicates to me that somehow you've got hold of a THP that is in the page cache. Probably shmem/tmpfs. That indicate to me a refcount problem that looks something like this: f2fs allocates a page f2fs adds the page to the page cache f2fs puts the reference to the page without removing it from the page cache (how?) page is now free, gets reallocated into a THP lookup from the f2fs file finds the new THP things explode messily Checking page->mapping is going to avoid the messy explosion, but you'll still have a page in the page cache which doesn't actually belong to you, and that's going to lead to subtle data corruption. This should be caught by page_expected_state(), called from free_page_is_bad(), called from free_pages_prepare(). Do your testers have CONFIG_DEBUG_VM enabled? That might give you a fighting chance at finding the last place which called put_page(). It won't necessarily be the _wrong_ place to call put_page() (that may have happened earlier), but it may give you a clue. > > > > struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, > > int fgp_flags, gfp_t gfp) > > { > > struct folio *folio; > > > > folio = __filemap_get_folio(mapping, index, fgp_flags, gfp); > > if (IS_ERR(folio)) > > return NULL; > > return folio_file_page(folio, index); > > } > > > > Thanks, > > > > > - f2fs_put_page(page, 1); > > > - goto repeat; > > > - } > > > - if (unlikely(!PageUptodate(page))) { > > > + if (unlikely(page->mapping != mapping || !PageUptodate(page))) { > > > f2fs_put_page(page, 1); > > > return ERR_PTR(-EIO); > > > }