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 AEFFCC54E67 for ; Thu, 28 Mar 2024 14:13:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 25BAF6B0082; Thu, 28 Mar 2024 10:13:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 20C336B0085; Thu, 28 Mar 2024 10:13:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0D57E6B0087; Thu, 28 Mar 2024 10:13:05 -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 E19BD6B0082 for ; Thu, 28 Mar 2024 10:13:04 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 6BEBB120820 for ; Thu, 28 Mar 2024 14:13:04 +0000 (UTC) X-FDA: 81946639488.25.57E85FA Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf13.hostedemail.com (Postfix) with ESMTP id EDE9F2000C for ; Thu, 28 Mar 2024 14:13:00 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=aSw1bPJv; spf=none (imf13.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=1711635182; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6M0JjTgPpVxG5fBfSzA5TFEpVDFwxFOSbAPU01cFVuA=; b=Vx3cEGDA+PmRhmJWHCOSDCAz2/FMyqGsIxZS1CDXtELZ1ROgIXvbfLc3nGRwYy90+wImBi wawyG9iRCehAkEM5KgG3O+wdqJ9+9ak5wNb0dUSUkpn6sl77fI0gb8S5bifUhXWzfWCYXx jcuhGB9Qad4Bbt89zBtsZHFOzDYHzaU= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=aSw1bPJv; spf=none (imf13.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=1711635182; a=rsa-sha256; cv=none; b=AE619TfQcySWbHWlxxU6Pfi2ILMdg0wmHS8uXaRjQAQTLcSjHf+bcHl76n2QaP8s1HF5xK N5X1YepFOyqkuNFZBDsYciLzuGZfkm2PahfOmzV7uPfrfq6BvkXYoFNmX+VIkgDpu5bhQz Bw5QtLHEjCp//ONH1GMMH81MzEw9q2s= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=6M0JjTgPpVxG5fBfSzA5TFEpVDFwxFOSbAPU01cFVuA=; b=aSw1bPJv1K6haWj1FpTAbHPJMN Eu+x2QWdUimWeLeVSyrDjrdFbBU/WWogT56C+BfDLR2Wy8mRXog1B60RFetdRlNMSvdjnkDxCM+ZP RxcO2FD6tdf0QZwGMkE4seB1rFn8a2xohtovgS/6g/7CjQNCAR57mhULH0F8+6DfkhA6kaYcz8RtK o0OvHvRXvkmIpYfwBXxZSAS8R0DCHnmlgeMi9KsjRFQljYteX8hjf0y9UTrMlfX0stBASwAMlfK+g 74rzUrv3VNaV8MlUaKb/JSKQ4SNyK1Mkd0eZ7gUmX/voOKpQ/+RmLDBHBw4gXLBMrdpGV+UbWJy40 zTs+5jrA==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rpqUv-00000006UyX-43Jl; Thu, 28 Mar 2024 14:12:53 +0000 Date: Thu, 28 Mar 2024 14:12:53 +0000 From: Matthew Wilcox To: Zhaoyang Huang Cc: =?utf-8?B?6buE5pyd6ZizIChaaGFveWFuZyBIdWFuZyk=?= , Andrew Morton , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , =?utf-8?B?5bq357qq5ruoIChTdGV2ZSBLYW5nKQ==?= Subject: Re: summarize all information again at bottom//reply: reply: [PATCH] mm: fix a race scenario in folio_isolate_lru Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: EDE9F2000C X-Rspam-User: X-Stat-Signature: kyr85j4g3tofye9uq4noy4nmssxueo6u X-Rspamd-Server: rspam01 X-HE-Tag: 1711635180-810226 X-HE-Meta: U2FsdGVkX1+LQHPL9j87TLfjifQjJjLfvKWrSkmkGVCwFmrL/cLX5VNJlKA0+EmtqhAZ8lxwDyinoS5e7hNB12vSAm7xQmyswJcq7GlqHI6Yys4sWHeudMnuROdqVlIhE27jjNPe5ETZWB/gTitkaEKylINXgr2muNJoTOPUlsfy1ZH5P1AKWOJ6ArFnjYj9KBMBaPU1GMkkqdIu1DC0brl/O2mBCrVgpUn6Ef012QuMCLjJJWCMdOdfrmlVRlExUVKnZUF2uHc4t3ZgaMmJC+KljEdH70fEpawsPLKKB29sGL6yAhH/XeSUtkJjvLO0Nyrf7lYfjs7kWHFvV0cTZzvtkFSR/Y7y3caXSbCK4XRZaOvEflfvb0x2jvrOnauMaInvhDvVFkiGozrB0PyeUnkd7ZgEHLGnPdO5/WsmsDf9NivtSF4hk8sVCt0pxxGW7/DF56XgTlITi5z3cydhStMBrawD/qZn9D6nHGwHlIgCgCu4FbBtqlljHyfmxNq9STIBBvEzkxhYDHLNNIPxmSihBz7ytUBep+VcD4aaXv6LmsEkGByGusyVc488wWNY0it2y6ZgFQiSQ+zl9+oNduba/MlULFUknkshlHNF34akH21LaKPVi92/vwt/rrX+W2tayS8QL9E5Vb+iGxpYrKHzrz+nGP7tSKRIcvyhSF2e8bWSjTTvXOSfTRS7m8x3lsax5PaVTGE3OG0jGMpeAMtq1yu4cLNLSzMhDUTmkpXyZ8dHf1ohrvIQ+cZrakMkCH4Cmjm8V+37lKCb2MOJqQHp8L7FYFuchqprajAN8WsnLjQMk02WoK1W+mfboJrhAyHZa9VWJBakpKdtM6G32ydoFA0Ri8GVB7FpNklTSgwqzgvgsI3jj4NTPlmLE1m5sFOoRAst/5QpM3se79uyWLdQHdDAsKfIu1yOqPNAgNNNTML+BL6/WCdaNghvw3KbqPttIVSGki7eXBLvuWo gQAAxH1y go54sEB04SGjQXR6/UFW0qPGOWe51g+Z8mdYgBe58suxAY8OybTboQkJ+FBU8tMTohHO1j05oHlQbb9Z/SU3zHx159/8RoXg2GunpDdbqxTUfueTKPLaPLY1MPYZSujxa0MBdDSVN3TWMmF5+Jmdbsof8DlsmfsxfRB6K/+R6LyjEcA7pnp4AODTLxGNhz8zsQptzH1Mez8dG5e9S5oVKuEmEGpBbJQcZD0uHjczWs75Vil3vUHQaoJ/iQOBij1OguDSyzUFf9oGQT6KZp2woI56vBfdRP6cZcaLilM2coTdOeyQVgcAE3AJp2A== 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: On Thu, Mar 28, 2024 at 12:03:02PM +0800, Zhaoyang Huang wrote: > On Thu, Mar 28, 2024 at 11:18 AM Matthew Wilcox wrote: > > > > On Thu, Mar 28, 2024 at 09:27:31AM +0800, Zhaoyang Huang wrote: > > > ok, I missed the refcnt from alloc_pages. However, I still think it is > > > a bug to call readahead_folio in read_pages as the refcnt obtained by > > > alloc_pages should be its final guard which is paired to the one which > > > checked in shrink_folio_list->__remove_mapping->folio_ref_freeze(2)(this > > > 2 represent alloc_pages & page cache). If we removed this one without > > > > __remove_mapping() requires that the caller holds the folio locked. > > Since the readahead code unlocks the folio, __remove_mapping() cannot > > be run because the caller of __remove_mapping() will wait for the folio > > lock. > repost the whole timing sequence to make it more clear and fix > incorrect description of previous feedback I can't understand what you think the problem is here. Please try again. > Follow the refcount through. > > In page_cache_ra_unbounded(): > > folio = filemap_alloc_folio(gfp_mask, 0); > (folio has refcount 1) > ret = filemap_add_folio(mapping, folio, index + i, gfp_mask); > (folio has refcount 2, PG_lru) > > Then we call read_pages() > First we call ->readahead() which for some reason stops early. > Then we call readahead_folio() which calls folio_put() > (folio has refcount 1) > Then we call folio_get() > (folio has refcount 2) > Then we call filemap_remove_folio() > (folio has refcount 1) > Then we call folio_unlock() > Then we call folio_put() > > Amending steps for previous timing sequence below where [1] races with > [2] that has nothing to do with __remove_mapping(). IMO, no file_folio > should be freed by folio_put as the refcnt obtained by alloc_pages > keep it always imbalanced until shrink_folio_list->__remove_mapping, > where the folio_ref_freeze(2) implies the refcnt of alloc_pages and > isolation should be the last two. release_pages is a special scenario > that the refcnt of alloc_pages is freed implicitly in > delete_from_page_cache_batch->filemap_free_folio. > > folio_put() > { > if(folio_put_test_zero()) > *** we should NOT be here as the refcnt of alloc_pages should NOT be dropped *** > if (folio_test_lru()) > *** preempted here with refcnt == 0 and pass PG_lru check *** > [1] > lruvec_del_folio() > Then thread_isolate call folio_isolate_lru() > folio_isolate_lru() > { > folio_test_clear_lru() > folio_get() > [2] > lruvec_del_folio() > } > -------------------------------------------------------------------------------------------- > shrink_folio_list() > { > __remove_mapping() > { > refcount = 1 + folio_nr_pages; > *** the refcount = 1 + 1 implies there should be only the refcnt of > alloc_pages and previous isolation for a no-busy folio as all PTE has > gone*** > if (!folio_ref_freeze(refcount)) > goto keeplock; > } > }