From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (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 6261343E9D1 for ; Tue, 16 Jun 2026 15:29:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781623772; cv=none; b=PZ2mAMpJU/3G4dK4D0+Acl3jt41d7jhiKPzjJKzg7z28MzFAXNrh2jqdUIDGd5uG0i0DF1lCbxD9t0Tv8KsqePXFLDbYTL3kgQKobaxdB7LDisHRs1jblwTudvEURdWTxP9tOWTUctJvQ3PW7H5BGeWHGlqGDoB6jV+DT0v7yU0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781623772; c=relaxed/simple; bh=iTOdCCxOciqDq3QEiRkj/1mAirPmZrWZ2VW8I62pb4o=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QR2vwUdS8TCbTK5PpE3YavVb0fKPXQB1bFI3FHbvOYP5eB4jx4KpTgU98tuKt92sVs7fkMikWOhVvoS+XlzHR2suV/DLnGzf8YXpXTsbZZKhEqssEojsbez6yz5SyvU5jW4ZBaXI8NLo7PevFj0p2xZweddFTo+xQ/JUlfePzDM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=pass smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=I1V+L04M; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="I1V+L04M" 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=lwNI3TC3ZkSqHP1ZixJ+WWxbe0YFuS4gEBaM5KRIT7g=; b=I1V+L04MelcOTOgv85PNkmqbZc Q2uV34S5ntdDUDsNXSknZ6otAZQ71RV6DAPYIFP0pOraYqGqxNF6kQZ+FsBx8U7exhSoRfUmDDhCJ TIElMDq/maMiurbZlGJx6QYP/3z7WSXzqp0FahLK1lv5V5nvPruVmQ8A5STUiFmagJZ+NG/JEoYYq jJRTG0+EPXOGSbxnLxBepZnEXbC9Tmv5/T0ahjW8EcQDv3nSZw6/fd/pWxzdAJ5e9pJw2CuLamJc0 U2MG7FgEBw571xw15eVHIsAlGBzvsohVuhFrZrfQCuagW/ildtdRs7/InyGL30zayWbW7ZfNXHQDw Va4Pr1DA==; Received: from willy by casper.infradead.org with local (Exim 4.99.1 #2 (Red Hat Linux)) id 1wZViw-0000000AzBP-3A0I; Tue, 16 Jun 2026 15:29:10 +0000 Date: Tue, 16 Jun 2026 16:29:10 +0100 From: Matthew Wilcox To: Rik van Riel Cc: linux-kernel@vger.kernel.org, kernel-team@meta.com, 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 , linux-mm@kvack.org Subject: Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file() Message-ID: References: <20260616145413.1491961-1-riel@surriel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260616145413.1491961-1-riel@surriel.com> 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 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);