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 1AFE5E7718B for ; Mon, 23 Dec 2024 06:59:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5567A6B007B; Mon, 23 Dec 2024 01:59:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 506546B0082; Mon, 23 Dec 2024 01:59:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3CD626B0083; Mon, 23 Dec 2024 01:59:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 1B0876B007B for ; Mon, 23 Dec 2024 01:59:30 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 64AF345D7F for ; Mon, 23 Dec 2024 06:59:29 +0000 (UTC) X-FDA: 82925321766.12.E3BCC7B Received: from out-185.mta1.migadu.com (out-185.mta1.migadu.com [95.215.58.185]) by imf23.hostedemail.com (Postfix) with ESMTP id 3FDE2140007 for ; Mon, 23 Dec 2024 06:59:02 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=moHAoNAT; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf23.hostedemail.com: domain of muchun.song@linux.dev designates 95.215.58.185 as permitted sender) smtp.mailfrom=muchun.song@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734937150; a=rsa-sha256; cv=none; b=lURkzcSiVQj72Sse6HWKUDdzmliQiId4hCnmXpwR8HbdC/fBUlioUbuvn/WAb6axqt04+B 9zfHbYEbB7LeCwuylTctMayKGjHwk85UFCbxkofBknvIIyAJRBqA5P+SAVp+eDljps+l/P njD4d7CCvhPHuwbyvxQAfNFuocAnsyU= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=moHAoNAT; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf23.hostedemail.com: domain of muchun.song@linux.dev designates 95.215.58.185 as permitted sender) smtp.mailfrom=muchun.song@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734937150; 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=SNj5OAreCtWnyMSUyYMFWWVofqOXNGMfoPvDSwrklQ8=; b=mRrsrsS9ZlaFFqqcgVH7NDl8xAW4B3a3jXxZmqXJoGhY+6R2/KA7IWADyxtthND84cJL0n DwAioRzAezL1RfVXgII+qPOU+7EFuX8jjIp2S5EBbyTozDCWCUqlSV9drOie4fV9ALZdoH G2ELeA2AQgBeDv6TwdJmOUL4p5FRkfc= Content-Type: text/plain; charset=utf-8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1734937165; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SNj5OAreCtWnyMSUyYMFWWVofqOXNGMfoPvDSwrklQ8=; b=moHAoNATiiPlQsSqS5MR+uZms+TpPalyXrKrEIA47lpvhUwHByzzQGdJDJLkBVeZPEF28K i1/v2Lv0LM4oRjdGPVkd4a1jLZGZeZdXAe8ZOFINPOVMEOR4r52QytPK2HxPXG+nwfLxQZ peCOsJemOl4hHc0pm6iSpfZ+m3r+Tuc= Content-Transfer-Encoding: quoted-printable X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song Mime-Version: 1.0 (1.0) Subject: Re: [PATCH] mm, madvise: fix potential workingset node list_lru leaks Date: Mon, 23 Dec 2024 14:58:48 +0800 Message-Id: References: <20241222122936.67501-1-ryncsn@gmail.com> Cc: linux-mm@kvack.org, Andrew Morton , Matthew Wilcox , Johannes Weiner , Roman Gushchin , Shakeel Butt , Michal Hocko , Chengming Zhou , Qi Zheng , Yu Zhao , Sasha Levin , linux-kernel@vger.kernel.org, syzbot+38a0cbd267eff2d286ff@syzkaller.appspotmail.com, Kairui Song In-Reply-To: <20241222122936.67501-1-ryncsn@gmail.com> To: Kairui Song X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 3FDE2140007 X-Stat-Signature: cx7ezcjh44jp8n3gwzg3my65au38w6ac X-HE-Tag: 1734937142-629437 X-HE-Meta: U2FsdGVkX1913RS/75Ngz13KXTyS9ZfPde1Dd5Oda91OUzCtk2k4kjLAiS1cTeSrjqa9sjqdFjhOm7Gdm8IfYilygUHPWAMQTLxk/DokNVDF8oXs3bd95AO/cdLfxfDv96AI0Ik4X2T0Y5pMsldc9ins9w7+6aqvHVT5ArD9QKBqoog/jh22B/8n/UPZbjceY2NSIZKY/PI/QaJKZuQFLD+1EJD1+AtOOlEPxgiQBEMSrFPJXt/wHphhPyeFNHblI6DV1cNluYyUw0cZsSlGZy0NWBQZhRU322RYsawxhhQFqeW/tdokygg/cLAONH5/s3VVcgP/Q77NQ/4yx57m06wfx1HEUa55DymY914wr3qWFIvSS55SJ97m3as/SQRheFQgC6UmamnFVqvpyKQwnW2MZLILPG+N3g793LX6sE5R6+7si7+/R2CbX77751Du4ec3ErBupIw5iQBLIgAoQF0Jid0spnEYGdkRxCXTejjFlEmIaoX7fnFzgseQNJlqHApVM/ckSzqHc5TwuBMvVPLYP5YBJCeZeRtVUapE0y3lAPY5OtvRhp3WZEH6mhJzKneCWrnw+g1SUZnuY/9mhubXcwv9ArHZRt+d3WBnjNZTSqWNvwMVVN3iRCv3ZzCSxwHaxJZ7vlx030ASwjQ4Ym1kbaXlMTpB89AWMd0W69INQ90Oz3psY7K3aTIwkf6ccdMjLv2zBUW0/tbrkPR7f4C6hMCkYBztu15+8bpO8JnBlnKawMJQDYPCQu1f6nV0WIxcxduDKHux1CMoQu8BZwjYoRX74hKq2u9GhmZWOjeIrBISN9JcEjqZ/f7QW4MoEpZQpw7d4pSJmGToLIH/1TimaBRQoDjuAVRBbQzAHx/nUDFIJZiEBzPnbpH4DrfqNCPjrwVRnZC6lRSLtkcDsUBONKC+ZUMVry30I6g4IWVuO5cKROHkGek8dkip0eJ0UTyQtLzrvEpi3lM/2vd fOr1pTHP SxN6W/1iqGy1eviKrn87blXRUPt0mmqqMmaVYTqguXEHbj98yrkla43e9nEPCYaQJ6l7we2iQjOVbwPZ/KhkRPN1vB+Of+vhLpjZec1OY/dpZhwqvqIwAXsIwYRcDJuYUFaJSjYYHENDln9tPNioGabEryzG71IiyVoTr97TTC4Ssy21txAU0Jiq2Z6hLt1kEBUy3XD1sM9q0slQ2xzZ+0Yv+BqBCoxBePtzS0ON26yS6dTVIZYl3bV8YGtBzIJ71Zi/7iq9wVdcTmTNsUb1wz+g5pznZITbmqCzDVY5DkNDFyRwxUWyQEQePHl1r+5KMD0XziEayfSOFdJ1eK5o9b+Yjcpk7a3RtfzLEQSOMhTrt0P++T6DkpK4Hu8tBzfbqJs0kQDlNjW8dEZWIRdwBBLDXd/dkzxiEKBzM5w7Q0x3FkK+8sxxzcPYkpGbQ9uwzXOtsoBuJRHUDtwkPeYpdNupczepw0tKe+p5r 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 Dec 22, 2024, at 20:29, Kairui Song wrote: >=20 > =EF=BB=BFFrom: Kairui Song >=20 > Since commit 5abc1e37afa0 ("mm: list_lru: allocate list_lru_one > only when needed"), all list_lru users need to allocate the items using > the new infrastructure that provides list_lru info for slab allocation, > ensuring that the corresponding memcg list_lru is allocated before use. >=20 > For workingset shadow nodes (which are xa_node), users are converted to > use the new infrastructure by commit 9bbdc0f32409 ("xarray: use > kmem_cache_alloc_lru to allocate xa_node"). The xas->xa_lru will be > set correctly for filemap users. However, there is a missing case: > xa_node allocations caused by madvise(..., MADV_COLLAPSE). >=20 > madvise(..., MADV_COLLAPSE) will also read in the absent parts of file > map, and there will be xa_nodes allocated for the caller's memcg > (assuming it's not rootcg). However, these allocations won't trigger > memcg list_lru allocation because the proper xas info was not set. >=20 > If nothing else has allocated other xa_nodes for that memcg to trigger > list_lru creation, and memory pressure starts to evict file pages, > workingset_update_node will try to add these xa_nodes to their > corresponding memcg list_lru, and it does not exist (NULL). So they > will be added to rootcg's list_lru instead. >=20 > This shouldn't be a significant issue in practice, but it is indeed > unexpected behavior, and these xa_nodes will not be reclaimed > effectively. And may lead to incorrect counting of the > list_lru->nr_items counter. >=20 > This problem wasn't exposed until recent commit 28e98022b31ef > ("mm/list_lru: simplify reparenting and initial allocation") added a > sanity check: only dying memcg could have a NULL list_lru when > list_lru_{add,del} is called. This problem triggered this WARNING. >=20 > So make madvise(..., MADV_COLLAPSE) also call xas_set_lru() to pass > the list_lru which we may want to insert xa_node into later. And move > mapping_set_update to mm/internal.h, and turn into a macro to avoid > including extra headers in mm/internal.h. >=20 > Fixes: 9bbdc0f32409 ("xarray: use kmem_cache_alloc_lru to allocate xa_node= ") Hi Kairui, This should be 7d8faaf15545 introducing MADV_COLLAPSE, right? Because it is d= eveloped after 9bbdc0f32409. Thanks. > Reported-by: syzbot+38a0cbd267eff2d286ff@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/lkml/675d01e9.050a0220.37aaf.00be.GAE@goog= le.com/ > Signed-off-by: Kairui Song > --- > mm/filemap.c | 9 --------- > mm/internal.h | 6 ++++++ > mm/khugepaged.c | 3 +++ > 3 files changed, 9 insertions(+), 9 deletions(-) >=20 > diff --git a/mm/filemap.c b/mm/filemap.c > index f61cf51c2238..33b60d448fca 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -124,15 +124,6 @@ > * ->private_lock (zap_pte_range->block_dirty_folio) > */ >=20 > -static void mapping_set_update(struct xa_state *xas, > - struct address_space *mapping) > -{ > - if (dax_mapping(mapping) || shmem_mapping(mapping)) > - return; > - xas_set_update(xas, workingset_update_node); > - xas_set_lru(xas, &shadow_nodes); > -} > - > static void page_cache_delete(struct address_space *mapping, > struct folio *folio, void *shadow) > { > diff --git a/mm/internal.h b/mm/internal.h > index cb8d8e8e3ffa..4e7a3a93d0a2 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -1510,6 +1510,12 @@ static inline void shrinker_debugfs_remove(struct d= entry *debugfs_entry, > /* Only track the nodes of mappings with shadow entries */ > void workingset_update_node(struct xa_node *node); > extern struct list_lru shadow_nodes; > +#define mapping_set_update(xas, mapping) do { \ > + if (!dax_mapping(mapping) && !shmem_mapping(mapping)) { \ > + xas_set_update(xas, workingset_update_node); \ > + xas_set_lru(xas, &shadow_nodes); \ > + } \ > +} while (0) >=20 > /* mremap.c */ > unsigned long move_page_tables(struct vm_area_struct *vma, > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 6f8d46d107b4..653dbb1ff05c 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include >=20 > #include > @@ -1837,6 +1838,8 @@ static int collapse_file(struct mm_struct *mm, unsig= ned long addr, > if (result !=3D SCAN_SUCCEED) > goto out; >=20 > + mapping_set_update(&xas, mapping); > + > __folio_set_locked(new_folio); > if (is_shmem) > __folio_set_swapbacked(new_folio); > -- > 2.47.1 >=20