From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 BB6B03A7F74; Fri, 13 Mar 2026 14:55:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773413752; cv=none; b=LDzjf+LfqH1gOChC7ZrqeoX8ddldpj/u7gy2TmqKlZjOsSWIN1JftEDq7lH3CS8kcWXR6pA9wc+g2niXVoZKpTjyf49MywgtC7qUYo65SLVNNdImk18X+V5Gxd2E4pmll/hZh96LZ+9d9FXQZ/PvHcsQBOrzHAfkd6rd5esMm1w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773413752; c=relaxed/simple; bh=1SJdVb46rEOXp0TNCRzNQH20hOu3F1a8jHEpr/61pdg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=O7wF6pWkjx8hO36/j38Beivm2HccAJRhszXIHRi7Bb7NK/SIEWToK+tYveWFeyZWdkWFqCyIWH6Umwo+8ZeI69jX6YxyeuNZ1raj+wK2I3bItLAyg91JGRuGe3tWJth5S9iibByenRNbyEnzanDWfOrGwz4iIzwCbQplnEtfRL0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=STCiPlE7; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="STCiPlE7" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 45E4FC19421; Fri, 13 Mar 2026 14:55:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773413752; bh=1SJdVb46rEOXp0TNCRzNQH20hOu3F1a8jHEpr/61pdg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=STCiPlE7YCJ6EP08/0/rI3sm0hiVo8h5O7hPvhYrwE3F9gUgh6x4qYOG+oseUMFcp 0fco+B87d09Raz+TTKdqXsLyYzKcXAdDlu7u6dynGTXapE5MiKaRnCLZNtHfy4iPVf YrFUa2ViLJHx3lj6THIzfxNRTHyc59xkPpq2G4OYaS7/FpXFsareyepu+TXhtqXV1t +IWXOamdz1kto1W1vQdKKch43W+DgpLtRQt/JCOlpjqXu2nAG9q2e5bg95X1VbUZkm zFHSXzhBhfLcoSN9hVcPH55Kp/YFTYgSIxZnb4g0wcPn5Iu25E8YXr01CGvzb/Z3K7 vTjplDCeoUGQQ== Date: Fri, 13 Mar 2026 07:55:51 -0700 From: "Darrick J. Wong" To: Andrey Albershteyn Cc: Andrey Albershteyn , linux-xfs@vger.kernel.org, fsverity@lists.linux.dev, linux-fsdevel@vger.kernel.org, ebiggers@kernel.org, hch@lst.de, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-btrfs@vger.kernel.org Subject: Re: [PATCH v4 19/25] xfs: remove unwritten extents after preallocations in fsverity metadata Message-ID: <20260313145551.GN1770774@frogsfrogsfrogs> References: <20260309192355.176980-1-aalbersh@kernel.org> <20260309192355.176980-20-aalbersh@kernel.org> <20260310012914.GG1105363@frogsfrogsfrogs> <20260312145250.GE6069@frogsfrogsfrogs> Precedence: bulk X-Mailing-List: fsverity@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Fri, Mar 13, 2026 at 12:17:15PM +0100, Andrey Albershteyn wrote: > On 2026-03-12 07:52:50, Darrick J. Wong wrote: > > On Thu, Mar 12, 2026 at 02:50:57PM +0100, Andrey Albershteyn wrote: > > > On 2026-03-09 18:29:14, Darrick J. Wong wrote: > > > > > XFS preallocates spaces during writes. In normal I/O this space, if > > > > > unused, is removed by truncate. For files with fsverity, XFS does not > > > > > use truncate as fsverity metadata is stored past EOF. > > > > > > > > > > After we're done with writing fsverity metadata iterate over extents in > > > > > that region and remove any unwritten ones. These would be preallocation > > > > > leftovers in the merkle tree holes and past fsverity descriptor. > > > > > > > > > > Signed-off-by: Andrey Albershteyn > > > > > --- > > > > > > > There's an upper limit on the number of blocks you can unmap/free in a > > > > single transaction. Maybe move the xfs_trans_{alloc,commit} into the > > > > loop body? > > > > > > > > Oh wait, you skip the written extents. Ok, so maybe just roll it after > > > > you've done a bunmapi. > > > > > > I see, I will add a rolling transaction. Btw, why skipping the > > > written extents let's us use rolling here? > > > > Hrmm. At first I thought: why can't xfs_fsverity_cancel_unwritten > > allocate (and commit) the transaction inside the loop body? Then I > > thought "well, it's only conditionally unmapping things, and it's sort > > of a pain to allocate a transaction only then to find out if you > > actually want to run one". > > > > OTOH there could be billions of extents in the data fork, so holding the > > ILOCK for that many transactions isn't a good thing because tr_write > > only preallocates space for a certain number of transaction rolls. > > fsverity already holds IOLOCK_EXCL so there can't be any other programs > > using the file. > > > > So now I arrive back at "The transaction allocation and commit/cancel > > should be inside the loop body, since crashing midway through wouldn't > > result in user-visible changes to the file data". > > > > > > Do you need to purge the cow fork too? > > > > > > hmm, what case are you thinking about here? The fsverity is written > > > in past EOF region, I don't see how COW extent could be left there. > > > Can they somehow be left mapped for blocks past i_size? > > > > I was talking about speculative cow fork preallocations below i_size. > > They'll eventually get purged by blockgc, but you could give the space > > back once you've committed enabling the fsverity file flag. > > hmm, if they're purged by blockgc, then why to do it here? I'm Since you've made the file read-only by turning on fsverity, I think it's appropriate to return the COW fork preallocations to the free space pool. Granted, you're correct that blockgc will eventually call xfs_reflink_cancel_cow_range for you either due to timeout or because something hit ENOSPC and kicked blockgc. So I guess its not a requirement to land this series; I was just surprised not to see xfs_reflink_cancel_cow_range done explicitly here. > iterating and removing unwritten extents here only because > xfs_inode_free_eofblocks() will skip fsverity inodes, the check in > xfs_can_free_eofblocks(): > > if (IS_VERITY(VFS_I(ip))) > return false; > > The skipping is done to not remove all the fsverity metadata > extents. With preallocations enabled there could be unwritten > extents left in the metadata. Cleaning out the unwritten extents from the data fork when enabling fsverity makes sense to me and is totally fine. > Am I missing something? Nah, but we might be talking past each other a bit at this point. :/ --D 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 lists.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C4C97105F7A6 for ; Fri, 13 Mar 2026 14:56:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type:Cc: Reply-To:From:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Subject:In-Reply-To:MIME-Version:References: Message-ID:To:Date:Sender:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=dby4yiXT5+k3Sj7m69VnkF4He2NGfaCBzVLirBWWBSs=; b=PDlqjhbYVVOU3ZbFz0r3k3+Ebg nfQ1UTHcL+Vw4J6rf5e0nAV5iE7DxmmU53mNSU7UDDw949EntYLMJfmu1oASE7jah+TS57iuMxbcq Rk0T/CHh8rMeNshSXIv2OMHCkXZsZgEkVa5ZnB9bDZuQJCrRDN/uRjFxVmXDmQaqNZOY=; Received: from [127.0.0.1] (helo=sfs-ml-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1w13vp-0005aZ-4F; Fri, 13 Mar 2026 14:56:05 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1w13vo-0005aT-1R for linux-f2fs-devel@lists.sourceforge.net; Fri, 13 Mar 2026 14:56:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; 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:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=foPxZaH8+x0Jr6Nf9FkeluUSSUKac+3kBLmpVolmnao=; b=KhG450rYP7Heu1/mDltW/WWgOS iXYdB/BJkTs5wwMHgapZVTVQrgTHDAy1HKBpLkOy+QsxoXmNiTUyC9atEB/zF0j4paQd4CX577zOp QHkKDN0939u7uUig43xRCYzEZTZlDB5Z4hdnMm+Py7K4goPFLfC0hQWKISBQcsDGmnAs=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; 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:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=foPxZaH8+x0Jr6Nf9FkeluUSSUKac+3kBLmpVolmnao=; b=bqA3UvptLoyWP/c91sjertueYi b6f99JpbuJnnRvSU70BwLZ9wRK8KbiDIEqpGGxGqJhkGefE9SYxFu907t8z5Q6u0mwnyA+yz/2uGY 1VOhS/qyXaZRDe9Ku5rkq4ax1mlKztYv4/beKNY2+mSTbNHiz7Ux374ygWmKxKj126vg=; Received: from tor.source.kernel.org ([172.105.4.254]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1w13vn-0002Av-G2 for linux-f2fs-devel@lists.sourceforge.net; Fri, 13 Mar 2026 14:56:03 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 9A51161851; Fri, 13 Mar 2026 14:55:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 45E4FC19421; Fri, 13 Mar 2026 14:55:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773413752; bh=1SJdVb46rEOXp0TNCRzNQH20hOu3F1a8jHEpr/61pdg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=STCiPlE7YCJ6EP08/0/rI3sm0hiVo8h5O7hPvhYrwE3F9gUgh6x4qYOG+oseUMFcp 0fco+B87d09Raz+TTKdqXsLyYzKcXAdDlu7u6dynGTXapE5MiKaRnCLZNtHfy4iPVf YrFUa2ViLJHx3lj6THIzfxNRTHyc59xkPpq2G4OYaS7/FpXFsareyepu+TXhtqXV1t +IWXOamdz1kto1W1vQdKKch43W+DgpLtRQt/JCOlpjqXu2nAG9q2e5bg95X1VbUZkm zFHSXzhBhfLcoSN9hVcPH55Kp/YFTYgSIxZnb4g0wcPn5Iu25E8YXr01CGvzb/Z3K7 vTjplDCeoUGQQ== Date: Fri, 13 Mar 2026 07:55:51 -0700 To: Andrey Albershteyn Message-ID: <20260313145551.GN1770774@frogsfrogsfrogs> References: <20260309192355.176980-1-aalbersh@kernel.org> <20260309192355.176980-20-aalbersh@kernel.org> <20260310012914.GG1105363@frogsfrogsfrogs> <20260312145250.GE6069@frogsfrogsfrogs> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Headers-End: 1w13vn-0002Av-G2 Subject: Re: [f2fs-dev] [PATCH v4 19/25] xfs: remove unwritten extents after preallocations in fsverity metadata X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: "Darrick J. Wong via Linux-f2fs-devel" Reply-To: "Darrick J. Wong" Cc: fsverity@lists.linux.dev, ebiggers@kernel.org, Andrey Albershteyn , linux-f2fs-devel@lists.sourceforge.net, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, hch@lst.de, linux-btrfs@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On Fri, Mar 13, 2026 at 12:17:15PM +0100, Andrey Albershteyn wrote: > On 2026-03-12 07:52:50, Darrick J. Wong wrote: > > On Thu, Mar 12, 2026 at 02:50:57PM +0100, Andrey Albershteyn wrote: > > > On 2026-03-09 18:29:14, Darrick J. Wong wrote: > > > > > XFS preallocates spaces during writes. In normal I/O this space, if > > > > > unused, is removed by truncate. For files with fsverity, XFS does not > > > > > use truncate as fsverity metadata is stored past EOF. > > > > > > > > > > After we're done with writing fsverity metadata iterate over extents in > > > > > that region and remove any unwritten ones. These would be preallocation > > > > > leftovers in the merkle tree holes and past fsverity descriptor. > > > > > > > > > > Signed-off-by: Andrey Albershteyn > > > > > --- > > > > > > > There's an upper limit on the number of blocks you can unmap/free in a > > > > single transaction. Maybe move the xfs_trans_{alloc,commit} into the > > > > loop body? > > > > > > > > Oh wait, you skip the written extents. Ok, so maybe just roll it after > > > > you've done a bunmapi. > > > > > > I see, I will add a rolling transaction. Btw, why skipping the > > > written extents let's us use rolling here? > > > > Hrmm. At first I thought: why can't xfs_fsverity_cancel_unwritten > > allocate (and commit) the transaction inside the loop body? Then I > > thought "well, it's only conditionally unmapping things, and it's sort > > of a pain to allocate a transaction only then to find out if you > > actually want to run one". > > > > OTOH there could be billions of extents in the data fork, so holding the > > ILOCK for that many transactions isn't a good thing because tr_write > > only preallocates space for a certain number of transaction rolls. > > fsverity already holds IOLOCK_EXCL so there can't be any other programs > > using the file. > > > > So now I arrive back at "The transaction allocation and commit/cancel > > should be inside the loop body, since crashing midway through wouldn't > > result in user-visible changes to the file data". > > > > > > Do you need to purge the cow fork too? > > > > > > hmm, what case are you thinking about here? The fsverity is written > > > in past EOF region, I don't see how COW extent could be left there. > > > Can they somehow be left mapped for blocks past i_size? > > > > I was talking about speculative cow fork preallocations below i_size. > > They'll eventually get purged by blockgc, but you could give the space > > back once you've committed enabling the fsverity file flag. > > hmm, if they're purged by blockgc, then why to do it here? I'm Since you've made the file read-only by turning on fsverity, I think it's appropriate to return the COW fork preallocations to the free space pool. Granted, you're correct that blockgc will eventually call xfs_reflink_cancel_cow_range for you either due to timeout or because something hit ENOSPC and kicked blockgc. So I guess its not a requirement to land this series; I was just surprised not to see xfs_reflink_cancel_cow_range done explicitly here. > iterating and removing unwritten extents here only because > xfs_inode_free_eofblocks() will skip fsverity inodes, the check in > xfs_can_free_eofblocks(): > > if (IS_VERITY(VFS_I(ip))) > return false; > > The skipping is done to not remove all the fsverity metadata > extents. With preallocations enabled there could be unwritten > extents left in the metadata. Cleaning out the unwritten extents from the data fork when enabling fsverity makes sense to me and is totally fine. > Am I missing something? Nah, but we might be talking past each other a bit at this point. :/ --D _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel