From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout07.his.huawei.com (canpmsgout07.his.huawei.com [113.46.200.222]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9E0631A0B15 for ; Wed, 17 Jun 2026 02:01:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.222 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781661668; cv=none; b=N34Ylze8saHquBRdF7W+KXhdTXLawqKv/Ma0CJbcIhbRFJWUjDPnWxIEQvFRKaQBR9xohhQwd4B3ILzBICSIvegiea1+0kZGiIE+UwljK5GoZ2vrim1y4JZ1i0mhBEI2OdHw+UTbndKkQeZpFglgFVfosQM98sGNcBw03JP5Ssw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781661668; c=relaxed/simple; bh=sMO+737vhI1DtiXhPgWjhKiG3mbXFSJuzzUxFSD34jo=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=K6WoLw0A48NIvKRgWibDO5hKab14cl0hJJuOLh2Uh6TPjLWAA1ZnsN+Svj17Vok61sdiAU9gmGC4D+/1BaqmfaVDZx+8eWoUmhxgTE9MVi+3vzjpycVgjqVc5wSOg5oZcylzGXq8hFlQmqXKG6R/fkFCt8Ef6/iZzsV/gaCPXj4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=51qWupXg; arc=none smtp.client-ip=113.46.200.222 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="51qWupXg" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=PEGy/0Yc6RbsNR1CzoCHl4JOysn72pcydBscXTJDoAQ=; b=51qWupXghRRLKpqModGwC5GnkF8kkTXCULtk3m9It+i4OCKHMIewFucRgdC5g8QFFlzVLTJtu JiaSZYnzi+im7StBV2h2xUCSrA97KbJzrlAOGZPhU9X18HvDsDFLEf32okPeJk3c2yU3cVHOjPZ Wxo3w+5IJ7BkFjJe46yNpNk= Received: from mail.maildlp.com (unknown [172.19.163.163]) by canpmsgout07.his.huawei.com (SkyGuard) with ESMTPS id 4gg6M35ckQzLlSc; Wed, 17 Jun 2026 09:53:03 +0800 (CST) Received: from kwepemr500001.china.huawei.com (unknown [7.202.194.229]) by mail.maildlp.com (Postfix) with ESMTPS id B775C405D0; Wed, 17 Jun 2026 10:01:01 +0800 (CST) Received: from [10.174.178.9] (10.174.178.9) by kwepemr500001.china.huawei.com (7.202.194.229) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Wed, 17 Jun 2026 10:01:00 +0800 Message-ID: <8a4caadc-f414-4874-97d4-2fbaebe2af39@huawei.com> Date: Wed, 17 Jun 2026 10:00:59 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file() To: Matthew Wilcox , Rik van Riel , CC: , , Andrew Morton , David Hildenbrand , Lorenzo Stoakes , Hugh Dickins , Zi Yan , Baolin Wang , "Liam R. Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , References: <20260616145413.1491961-1-riel@surriel.com> From: Jinjiang Tu In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: kwepems200002.china.huawei.com (7.221.188.68) To kwepemr500001.china.huawei.com (7.202.194.229) 在 2026/6/16 23:29, Matthew Wilcox 写道: > On Tue, Jun 16, 2026 at 10:54:13AM -0400, Rik van Riel wrote: >> syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem >> mapping (collapse aborts with SCAN_TRUNCATED because the range is empty), >> and also via slab fault injection, which forces xas_create_range() down >> the xas_nomem() path before the same abort. kmemleak then reports the >> 576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem(). > I think the ways to produce this problem are sufficiently rare/unlikely I can reproduce it easily with following steps. [1] 1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, and then mmap the file 2) memset for the first 2MB 3) madvise(MADV_COLLAPSE) for the second 2MB 4) unlink the file This might be the only path easy to trigger; all other failure paths seem to be difficult to trigger. [1] https://lore.kernel.org/all/20260121062243.1893129-1-tujinjiang@huawei.com/ > to not merit this level of cleanup. Why don't we defer it to > clear_inode() instead? > > reasons not to: > > 1. the page cache is not the only user of xarrays and > theoretically either of these things could happen elsewhere > > 2. can a user do this on purpose to screw other users over? i don't > think so, any more than they can dos the system by bringing a lot of > inodes into memory; we have cgroups and ulimits to protect against that > > diff --git a/fs/inode.c b/fs/inode.c > index 6a3cbc7dcd28..d92c5a296504 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -779,21 +779,20 @@ void clear_inode(struct inode *inode) > fsverity_cleanup_inode(inode); > > /* > - * We have to cycle the i_pages lock here because reclaim can be in the > - * process of removing the last page (in __filemap_remove_folio()) > - * and we must not free the mapping under it. > + * We have to cycle the i_pages lock here because reclaim > + * can be in the process of removing the last page (in > + * __filemap_remove_folio()) and we must not free the mapping > + * under it. We also remove nodes which are empty; these > + * can occur in two different ways. The first is that radix > + * tree expansion can fail partway and the second is that THP > + * collapse_file() can allocate some temporary nodes and not > + * clean them up. > */ > xa_lock_irq(&inode->i_data.i_pages); > BUG_ON(inode->i_data.nrpages); > - /* > - * Almost always, mapping_empty(&inode->i_data) here; but there are > - * two known and long-standing ways in which nodes may get left behind > - * (when deep radix-tree node allocation failed partway; or when THP > - * collapse_file() failed). Until those two known cases are cleaned up, > - * or a cleanup function is called here, do not BUG_ON(!mapping_empty), > - * nor even WARN_ON(!mapping_empty). > - */ > + __xa_destroy(&inode->i_data.i_pages); > xa_unlock_irq(&inode->i_data.i_pages); > + > BUG_ON(!(inode_state_read_once(inode) & I_FREEING)); > BUG_ON(inode_state_read_once(inode) & I_CLEAR); > BUG_ON(!list_empty(&inode->i_wb_list)); > diff --git a/include/linux/xarray.h b/include/linux/xarray.h > index be850174e802..d776a6e9ad18 100644 > --- a/include/linux/xarray.h > +++ b/include/linux/xarray.h > @@ -571,6 +571,7 @@ int __must_check __xa_alloc_cyclic(struct xarray *, u32 *id, void *entry, > struct xa_limit, u32 *next, gfp_t); > void __xa_set_mark(struct xarray *, unsigned long index, xa_mark_t); > void __xa_clear_mark(struct xarray *, unsigned long index, xa_mark_t); > +void __xa_destroy(struct xarray *); > > /** > * xa_store_bh() - Store this entry in the XArray. > diff --git a/lib/xarray.c b/lib/xarray.c > index 9a8b4916540c..ee2459ecdc9b 100644 > --- a/lib/xarray.c > +++ b/lib/xarray.c > @@ -2370,6 +2370,21 @@ void xa_delete_node(struct xa_node *node, xa_update_node_t update) > } > EXPORT_SYMBOL_GPL(xa_delete_node); /* For the benefit of the test suite */ > > +void __xa_destroy(struct xarray *xa) > +{ > + XA_STATE(xas, xa, 0); > + void *entry; > + > + xas.xa_node = NULL; > + entry = xa_head_locked(xa); > + RCU_INIT_POINTER(xa->xa_head, NULL); > + xas_init_marks(&xas); > + if (xa_zero_busy(xa)) > + xa_mark_clear(xa, XA_FREE_MARK); > + if (xa_is_node(entry)) > + xas_free_nodes(&xas, xa_to_node(entry)); > +} > + > /** > * xa_destroy() - Free all internal data structures. > * @xa: XArray. > @@ -2382,21 +2397,11 @@ EXPORT_SYMBOL_GPL(xa_delete_node); /* For the benefit of the test suite */ > */ > void xa_destroy(struct xarray *xa) > { > - XA_STATE(xas, xa, 0); > unsigned long flags; > - void *entry; > > - xas.xa_node = NULL; > - xas_lock_irqsave(&xas, flags); > - entry = xa_head_locked(xa); > - RCU_INIT_POINTER(xa->xa_head, NULL); > - xas_init_marks(&xas); > - if (xa_zero_busy(xa)) > - xa_mark_clear(xa, XA_FREE_MARK); > - /* lockdep checks we're still holding the lock in xas_free_nodes() */ > - if (xa_is_node(entry)) > - xas_free_nodes(&xas, xa_to_node(entry)); > - xas_unlock_irqrestore(&xas, flags); > + xa_lock_irqsave(xa, flags); > + __xa_destroy(xa); > + xa_unlock_irqrestore(xa, flags); > } > EXPORT_SYMBOL(xa_destroy); > >