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 864C3C54E5D for ; Mon, 18 Mar 2024 06:16:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 991A86B0082; Mon, 18 Mar 2024 02:16:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 940AA6B0083; Mon, 18 Mar 2024 02:16:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 809166B0085; Mon, 18 Mar 2024 02:16:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 704816B0082 for ; Mon, 18 Mar 2024 02:16:12 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 11DA1A0847 for ; Mon, 18 Mar 2024 06:16:12 +0000 (UTC) X-FDA: 81909149784.17.80FB115 Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by imf26.hostedemail.com (Postfix) with ESMTP id 2381F140010 for ; Mon, 18 Mar 2024 06:16:09 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PVaRFD4z; spf=pass (imf26.hostedemail.com: domain of huangzhaoyang@gmail.com designates 209.85.208.172 as permitted sender) smtp.mailfrom=huangzhaoyang@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710742570; 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=2HlTC/PQyLZ9gsecrmYd7shisTwC2CsnXMSCozFMXgo=; b=h64/BL3Fn8bA+D3/vNCefc78njbLDNdrysVulHu8r4In3Mo4o74TNsmXB3pgBzHhosX6f3 ph7KS8LXt4qM/62ceoxVoa4/CdWrIEWMTyT4YmG/+F4K9L+uTqN6wY+dAUDHuQsifJDE+R To5ZbDN6Drx7I2daH9T9aIxWGh+pgxI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710742570; a=rsa-sha256; cv=none; b=IpGsAjxY10ZNdvMymNYGI+uLwnNMbJQLtura3o9M83UlR42sFK+4BCqOOxKMHrWzDpyWX0 ZuTdxmAmVHGEAI1s1QvS+f2436j5Y/vl9bC716MMbvYgqmbYqlqAY8JLRN/7ss2gt5q/xI yYL78J9XafqJa0Gv3iLAYhJzfaXbjyk= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PVaRFD4z; spf=pass (imf26.hostedemail.com: domain of huangzhaoyang@gmail.com designates 209.85.208.172 as permitted sender) smtp.mailfrom=huangzhaoyang@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2d4515ec3aaso32535621fa.1 for ; Sun, 17 Mar 2024 23:16:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710742568; x=1711347368; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=2HlTC/PQyLZ9gsecrmYd7shisTwC2CsnXMSCozFMXgo=; b=PVaRFD4zJ7iJSZu995IA8EHaYqh5hvhG+PDKRWD4d1iCr+MwwseQtT4UBmaumlCN0X jjPFwbJKSzpGVGqr2if4qfJATFErDQDdtRgwcX9kOUrx3VEINdNCTFCZ7gWCbn7C1Rmx DuFnHCQtrMSoI3plttFp0GWkf+DCk8asSm+JkUUVWC/TjvfKf4OFm6vmoAcCVXO+zL6k aayjZVOVR5JDyzEAGM8H0x/zMOUR6h3YlanxGi7x4tyUX0eu1W2k8tyb2EqM5fyQI7xr E0WkAihOnAEVUKhXY+FnNe8+HRAITNWl3oPBpVBKihlE0gh3CBmRFggWuwA60B767Of1 J6lw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710742568; x=1711347368; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2HlTC/PQyLZ9gsecrmYd7shisTwC2CsnXMSCozFMXgo=; b=SxS8x095wMeKH6nHHa5rJU27ouZ+4Yyhii03z3aJeSa+X5q330i3lTpCoOjuBjFrzg LS6otQAYR9P4PVsCKqKCxs08N187AdjauCxoEiz7EQWS9gKLgeJ38DYIW7+SQM3abx6+ rwnx9kX292+ETh48U7ls+lbsmoT6K1ntVjbi+72hNykKWzUpkW4eotLeHhXJ8PaBj1Ms 4SdfXIPS6SBdAI4cTOWdqtUJVSve1iwkTVVv6d+yYS682Qn2K02nEYUB93HhP9rUPEyY qCgEx90ypVxstZmRNPN1q7XOF8W/Ljg4z+yJ/QZFyBhSMTUJXJCuf+yylg89VsR8uADH KrQA== X-Forwarded-Encrypted: i=1; AJvYcCU8jaUXFQzAW4O25Qrn6ZVVTyf59oP8hgNn2UZsJHOirGVTHQfvYq5kIpCoaus77EM5wHUdLUmvqfEVvOJawK9nsa8= X-Gm-Message-State: AOJu0Yxj7Y2HfUF7qI4Rc5vEHPLcoGK39jXwtOebQsnBNBkFQL17QP1A EiVrUKL92sHGUXg8NvadAyeQm5/bq+h6aXIW5TEtgf4sDq7Yoo0OBfo3jDFPCQ1OBSki+MBjzSd ekO+jggTcHscBLdwg/yPaecOrRHmlFN/CD80= X-Google-Smtp-Source: AGHT+IGDo1rnN/i8+dbudhy57Z3WozyjNrzH1Q58Vyf48bLYj8/R0SepXkiFiJwlIJKHQv/xwprG8ZJyi0aMzjJlX/E= X-Received: by 2002:a2e:7a01:0:b0:2d4:91c4:fca8 with SMTP id v1-20020a2e7a01000000b002d491c4fca8mr2519373ljc.11.1710742567999; Sun, 17 Mar 2024 23:16:07 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Zhaoyang Huang Date: Mon, 18 Mar 2024 14:15:56 +0800 Message-ID: Subject: Re: reply: [PATCH] mm: fix a race scenario in folio_isolate_lru To: Matthew Wilcox Cc: =?UTF-8?B?6buE5pyd6ZizIChaaGFveWFuZyBIdWFuZyk=?= , Andrew Morton , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , =?UTF-8?B?5bq357qq5ruoIChTdGV2ZSBLYW5nKQ==?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 2381F140010 X-Rspam-User: X-Stat-Signature: p5y43mgbhoeobc1yozst8cktfyf7whjs X-Rspamd-Server: rspam03 X-HE-Tag: 1710742569-940379 X-HE-Meta: U2FsdGVkX18WWyagcvOfQ1NR4HuSuhhqNeaE0rg3o5LCE8h2biRb2F0Dk3PPjJibOi2W42HJRVcGEqhdQNRK/KSPDHkojNpnaANBiTsFW57Wg6MqDVo0XU2X74vbWzm8mxM8YcY2bVj342i9jkDhaoWv84zzxw30Xoar0DZ3WbBfXu1lYQ+8yg0qQaMGZ6/RdkGxrbUpvGgvo+19BFRD8W/9vrvtKWWjDzrLw18arpO7H0Ey55YKkJs7ZEYTK0fiKvHlMHc4oX8NsJOTtm8JcS/yUARLMfxienHpdJdQKWGf9McZB0iQJhvi1vkuPa7VRyOnnyx6nWNDQS9bV4LOQC2d66inU3WjWkNwwOkyOyCvWt4xvUE4MYLxQnT9ot1b4GkCZPMyOAIa/AK9vFZs5NN3TSytzsh3lTynTudJijZbldcDoffKV6Lp0tgMRyc8qWcSyuxlMO2J1QUrAsK04k8O59/cng+SH2Wm8WRGMyjeYlKHIOGPJLmOawxlXmO35lQYjXTfKjCGngzOP830OVJe/pSMk44rOYIaudeghK+dMCfzxjo6nD65BrTXS4i+V5biCm/Z9pHlHbq5O66sWgeQPlvg9hUsx20BqKVYYCXDmJVlB9Jw4JlPH34cjo2ojVrzMgJ+kX+sB69ZdLaikuJBBUr+XdtVIgQB40uudITa3JJNEfMqbiqBj0HvRyXPkjJSpNkCrIdNhyVmqQj1+wMau5DS1qkbWpTosg3JSH0DfWW4It+UHpybs4jfEQ/BwglMo2i6uMQeP9V0PUIOivbngh/qa3Ztii8UO3KHHCB4QCFiQg+wfqFl6uBrNDWZ3tVkOQlJUN7P67+OdHliPsKs8l1oQNg6xr75UKMJjXrM7Zigb7teHOHnaH5AQp8ulLDRzQ7sQnjarCyz+sZ3++jCATsDreH17BhqPO559O20mPXCLOn1EKyDkjy/bcKnXkDWLgl4P0EVDfiXcqa VBTG3Nal xhU8zLar+jeM48gpBxRlswKvnFQPavtNRYvccGL2doJdV34xpnMawxIDGHwSRV49RPeTOzczVLa8ZyN2WA+U2wRD7zCuKC0xTq/cE09Cn2XLUI1MoYNh2jTK6iT7lLV/Yv/tBSwu3jJXxjWd9rg2i5otfRYd+uQfQiRpsNWWvB7sgAN59EUWN7M5TxgDKiyR7lesuehx6t11tpd2YU6hpY0+tsNPX3o/PYGlSDw6LRRcIVIQ7gio9YV4C0//Qwz0dOus5Fs9VyaejO95LpslHJWHfR7ZxmxM7kNQPwUPENQy1gAn11c7mZbx2INA6p+a5zqrto1Kaey2HMWy9puXZ/xS5N5WHCuINQVMLxeJNJCQik71ylM51ZI1g50xdkqsAX0os5Ue9CqwtzxFfWjVliIP6I9gEmU943eHye0ORN40+VaICXa+Ecsqqh4pLtAEuZQ8RdSnhaU1EBQz0ZOehl0NhdZl2Kf1CRAT9Tm+cqPlAGagRkhI3e3RB7A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000014, 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 Mon, Mar 18, 2024 at 11:28=E2=80=AFAM Matthew Wilcox wrote: > > On Mon, Mar 18, 2024 at 01:37:04AM +0000, =E9=BB=84=E6=9C=9D=E9=98=B3 (Zh= aoyang Huang) wrote: > > >On Sun, Mar 17, 2024 at 12:07:40PM +0800, Zhaoyang Huang wrote: > > >> Could it be this scenario, where folio comes from pte(thread 0), loc= al > > >> fbatch(thread 1) and page cache(thread 2) concurrently and proceed > > >> intermixed without lock's protection? Actually, IMO, thread 1 also > > >> could see the folio with refcnt=3D=3D1 since it doesn't care if the = page > > >> is on the page cache or not. > > >> > > >> madivise_cold_and_pageout does no explicit folio_get thing since the > > >> folio comes from pte which implies it has one refcnt from pagecache > > > > > >Mmm, no. It's implicit, but madvise_cold_or_pageout_pte_range() > > >does guarantee that the folio has at least one refcount. > > > > > >Since we get the folio from vm_normal_folio(vma, addr, ptent); we know= that > > >there is at least one mapcount on the folio. refcount is always >=3D = mapcount. > > >Since we hold pte_offset_map_lock(), we know that mapcount (and theref= ore > > >refcount) cannot be decremented until we call pte_unmap_unlock(), whic= h we > > >don't do until we have called folio_isolate_lru(). > > > > > >Good try though, took me a few minutes of looking at it to convince my= self that > > >it was safe. > > > > > >Something to bear in mind is that if the race you outline is real, fai= ling to hold a > > >refcount on the folio leaves the caller susceptible to the > > >VM_BUG_ON_FOLIO(!folio_ref_count(folio), folio); if the other thread c= alls > > >folio_put(). > > Resend the chart via outlook. > > I think the problem rely on an special timing which is rare, I would li= ke to list them below in timing sequence. > > > > 1. thread 0 calls folio_isolate_lru with refcnt =3D=3D 1 > > (i assume you mean refcnt =3D=3D 2 here, otherwise none of this makes sen= se) > > > 2. thread 1 calls release_pages with refcnt =3D=3D 2.(IMO, it could be = 1 as release_pages doesn't care if the folio is used by page cache or fs) > > 3. thread 2 decrease refcnt to 1 by calling filemap_free_folio.(as I me= ntioned in 2, thread 2 is not mandatary here) > > 4. thread 1 calls folio_put_testzero and pass.(lruvec->lock has not bee= n take here) > > But there's already a bug here. > > Rearrange the order of this: > > 2. thread 1 calls release_pages with refcount =3D=3D 2 (decreasing refcou= nt to 1) > 3. thread 2 decrease refcount to 0 by calling filemap_free_folio > 1. thread 0 calls folio_isolate_lru() and hits the BUG(). > > > 5. thread 0 clear folio's PG_lru by calling folio_test_clear_lru. The f= olio_get behind has no meaning there. > > 6. thread 1 failed in folio_test_lru and leave the folio on the LRU. > > 7. thread 1 add folio to pages_to_free wrongly which could break the LR= U's->list and will have next folio experience list_del_invalid > > > > #thread 0(madivise_cold_and_pageout) #1(lru_add_drain->fbatch_re= lease_pages) #2(read_pages->filemap_remove_folios) > > refcnt =3D=3D 1(represent page cache) refcnt=3D=3D2(another= one represent LRU) folio comes from page cache > > This is still illegible. Try it this way: > > Thread 0 Thread 1 Thread 2 > madvise_cold_or_pageout_pte_range > lru_add_drain > fbatch_release_pages > read_pages > filemap_remove_folio Thread 0 Thread 1 Thread 2 madvise_cold_or_pageout_pte_range truncate_inode_pages_range fbatch_release_pages truncate_inode_pages_range filemap_remove_folio Sorry for the confusion. Rearrange the timing chart like above according to the real panic's stacktrace. Thread 1&2 are all from truncate_inode_pages_range(I think thread2(read_pages) is not mandatory here as thread 0&1 could rely on the same refcnt=3D=3D1). > > Some accuracy in your report would also be appreciated. There's no > function called madivise_cold_and_pageout, nor is there a function called > filemap_remove_folios(). It's a little detail, but it's annoying for > me to try to find which function you're actually referring to. I have > to guess, and it puts me in a bad mood. > > At any rate, these three functions cannot do what you're proposing. > In read_page(), when we call filemap_remove_folio(), the folio in > question will not have the uptodate flag set, so can never have been > put in the page tables, so cannot be found by madvise(). > > Also, as I said in my earlier email, madvise_cold_or_pageout_pte_range() > does guarantee that the refcount on the folio is held and can never > decrease to zero while folio_isolate_lru() is running. So that's two > ways this scenario cannot happen. The madivse_xxx comes from my presumption which has any proof. Whereas, It looks like truncate_inode_pages_range just cares about page cache refcnt by folio_put_testzero without noticing any task's VM stuff. Furthermore, I notice that move_folios_to_lru is safe as it runs with holding lruvec->lock. >