Linux EXT4 FS development
 help / color / mirror / Atom feed
* Warning Trace seen at https://elixir.bootlin.com/linux/v6.6.69/source/fs/ext4/ext4_jbd2.c#L73 post remount-ro
From: Shramana Chakraborty @ 2026-03-16 19:32 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Hi All,

I am following up on this bug, https://bugzilla.kernel.org/show_bug.cgi?id=220299, I raised last year regarding warning traces seen during systemd shutdown.

Warning Trace seen at https://elixir.bootlin.com/linux/v6.6.69/source/fs/ext4/ext4_jbd2.c#L73 is hit by the writeback system even though there is nothing to write and the filesystem has been mounted as readonly. 
An ext4 fs is mounted with journal=data and a large file is written to it. Then the fs is mounted as readonly-ro post a sync. At this point of time, data consistency is preserved through the sync itself followed by a sync from the remount-ro action. 
All the dirty pages should be marked as clean in the cache. Yet post the remount-ro, it hits https://elixir.bootlin.com/linux/v6.6.69/source/fs/ext4/ext4_jbd2.c#L73 and prints the trace as the fs is readonly now. 
Even though there is nothing to writeback, the wb_writeback is scheduled every 5 secs or so and the same trace can be seen if changed from WARN_ON_ONCE to WARN_ON.

I found that this was reported earlier as well via https://lkml.org/lkml/2024/5/17/983. 

This is still seen on 6.6.x LTS kernel with rootfs mounted with data=journal option. I wonder whether/how this could be addressed.

Thanks and Regards
Shramana Chakraborty

^ permalink raw reply

* [PATCH v2] crypto: tegra - Add missing CRYPTO_ALG_ASYNC
From: Eric Biggers @ 2026-03-16 20:21 UTC (permalink / raw)
  To: Herbert Xu, linux-crypto
  Cc: linux-ext4, linux-fscrypt, Thierry Reding, Jonathan Hunter,
	Eric Biggers, Zorro Lang, stable, Akhil R

The tegra crypto driver failed to set the CRYPTO_ALG_ASYNC on its
asynchronous algorithms, causing the crypto API to select them for users
that request only synchronous algorithms.  This causes crashes (at
least).  Fix this by adding the flag like what the other drivers do.
Also remove the unnecessary CRYPTO_ALG_TYPE_* flags, since those just
get ignored and overridden by the registration function anyway.

Reported-by: Zorro Lang <zlang@redhat.com>
Closes: https://lore.kernel.org/r/20260314080937.pghb4aa7d4je3mhh@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com
Fixes: 0880bb3b00c8 ("crypto: tegra - Add Tegra Security Engine driver")
Cc: stable@vger.kernel.org
Cc: Akhil R <akhilrajeev@nvidia.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---

This patch is targeting crypto/master.

v2: fix tegra-se-hash.c as well, and remove unnecessary type flags

 drivers/crypto/tegra/tegra-se-aes.c  | 11 ++++++----
 drivers/crypto/tegra/tegra-se-hash.c | 30 ++++++++++++++++------------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/tegra/tegra-se-aes.c b/drivers/crypto/tegra/tegra-se-aes.c
index 0e07d0523291..9210cceb4b7b 100644
--- a/drivers/crypto/tegra/tegra-se-aes.c
+++ b/drivers/crypto/tegra/tegra-se-aes.c
@@ -527,11 +527,11 @@ static struct tegra_se_alg tegra_aes_algs[] = {
 			.ivsize	= AES_BLOCK_SIZE,
 			.base = {
 				.cra_name = "cbc(aes)",
 				.cra_driver_name = "cbc-aes-tegra",
 				.cra_priority = 500,
-				.cra_flags = CRYPTO_ALG_TYPE_SKCIPHER | CRYPTO_ALG_ASYNC,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = AES_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_aes_ctx),
 				.cra_alignmask = 0xf,
 				.cra_module = THIS_MODULE,
 			},
@@ -548,11 +548,11 @@ static struct tegra_se_alg tegra_aes_algs[] = {
 			.max_keysize = AES_MAX_KEY_SIZE,
 			.base = {
 				.cra_name = "ecb(aes)",
 				.cra_driver_name = "ecb-aes-tegra",
 				.cra_priority = 500,
-				.cra_flags = CRYPTO_ALG_TYPE_SKCIPHER | CRYPTO_ALG_ASYNC,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = AES_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_aes_ctx),
 				.cra_alignmask = 0xf,
 				.cra_module = THIS_MODULE,
 			},
@@ -570,11 +570,11 @@ static struct tegra_se_alg tegra_aes_algs[] = {
 			.ivsize	= AES_BLOCK_SIZE,
 			.base = {
 				.cra_name = "ctr(aes)",
 				.cra_driver_name = "ctr-aes-tegra",
 				.cra_priority = 500,
-				.cra_flags = CRYPTO_ALG_TYPE_SKCIPHER | CRYPTO_ALG_ASYNC,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = 1,
 				.cra_ctxsize = sizeof(struct tegra_aes_ctx),
 				.cra_alignmask = 0xf,
 				.cra_module = THIS_MODULE,
 			},
@@ -592,10 +592,11 @@ static struct tegra_se_alg tegra_aes_algs[] = {
 			.ivsize	= AES_BLOCK_SIZE,
 			.base = {
 				.cra_name = "xts(aes)",
 				.cra_driver_name = "xts-aes-tegra",
 				.cra_priority = 500,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = AES_BLOCK_SIZE,
 				.cra_ctxsize	   = sizeof(struct tegra_aes_ctx),
 				.cra_alignmask	   = (__alignof__(u64) - 1),
 				.cra_module	   = THIS_MODULE,
 			},
@@ -1920,10 +1921,11 @@ static struct tegra_se_alg tegra_aead_algs[] = {
 			.ivsize	= GCM_AES_IV_SIZE,
 			.base = {
 				.cra_name = "gcm(aes)",
 				.cra_driver_name = "gcm-aes-tegra",
 				.cra_priority = 500,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = 1,
 				.cra_ctxsize = sizeof(struct tegra_aead_ctx),
 				.cra_alignmask = 0xf,
 				.cra_module = THIS_MODULE,
 			},
@@ -1942,10 +1944,11 @@ static struct tegra_se_alg tegra_aead_algs[] = {
 			.chunksize = AES_BLOCK_SIZE,
 			.base = {
 				.cra_name = "ccm(aes)",
 				.cra_driver_name = "ccm-aes-tegra",
 				.cra_priority = 500,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = 1,
 				.cra_ctxsize = sizeof(struct tegra_aead_ctx),
 				.cra_alignmask = 0xf,
 				.cra_module = THIS_MODULE,
 			},
@@ -1969,11 +1972,11 @@ static struct tegra_se_alg tegra_cmac_algs[] = {
 			.halg.statesize = sizeof(struct tegra_cmac_reqctx),
 			.halg.base = {
 				.cra_name = "cmac(aes)",
 				.cra_driver_name = "tegra-se-cmac",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = AES_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_cmac_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_cmac_cra_init,
diff --git a/drivers/crypto/tegra/tegra-se-hash.c b/drivers/crypto/tegra/tegra-se-hash.c
index 4a298ace6e9f..06bb5bf0fa33 100644
--- a/drivers/crypto/tegra/tegra-se-hash.c
+++ b/drivers/crypto/tegra/tegra-se-hash.c
@@ -759,11 +759,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha1",
 				.cra_driver_name = "tegra-se-sha1",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA1_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -784,11 +784,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha224",
 				.cra_driver_name = "tegra-se-sha224",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA224_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -809,11 +809,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha256",
 				.cra_driver_name = "tegra-se-sha256",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA256_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -834,11 +834,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha384",
 				.cra_driver_name = "tegra-se-sha384",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA384_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -859,11 +859,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha512",
 				.cra_driver_name = "tegra-se-sha512",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA512_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -884,11 +884,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha3-224",
 				.cra_driver_name = "tegra-se-sha3-224",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA3_224_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -909,11 +909,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha3-256",
 				.cra_driver_name = "tegra-se-sha3-256",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA3_256_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -934,11 +934,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha3-384",
 				.cra_driver_name = "tegra-se-sha3-384",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA3_384_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -959,11 +959,11 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "sha3-512",
 				.cra_driver_name = "tegra-se-sha3-512",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH,
+				.cra_flags = CRYPTO_ALG_ASYNC,
 				.cra_blocksize = SHA3_512_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -986,11 +986,12 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "hmac(sha224)",
 				.cra_driver_name = "tegra-se-hmac-sha224",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_NEED_FALLBACK,
+				.cra_flags = CRYPTO_ALG_ASYNC |
+					     CRYPTO_ALG_NEED_FALLBACK,
 				.cra_blocksize = SHA224_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -1013,11 +1014,12 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "hmac(sha256)",
 				.cra_driver_name = "tegra-se-hmac-sha256",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_NEED_FALLBACK,
+				.cra_flags = CRYPTO_ALG_ASYNC |
+					     CRYPTO_ALG_NEED_FALLBACK,
 				.cra_blocksize = SHA256_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -1040,11 +1042,12 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "hmac(sha384)",
 				.cra_driver_name = "tegra-se-hmac-sha384",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_NEED_FALLBACK,
+				.cra_flags = CRYPTO_ALG_ASYNC |
+					     CRYPTO_ALG_NEED_FALLBACK,
 				.cra_blocksize = SHA384_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,
@@ -1067,11 +1070,12 @@ static struct tegra_se_alg tegra_hash_algs[] = {
 			.halg.statesize = sizeof(struct tegra_sha_reqctx),
 			.halg.base = {
 				.cra_name = "hmac(sha512)",
 				.cra_driver_name = "tegra-se-hmac-sha512",
 				.cra_priority = 300,
-				.cra_flags = CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_NEED_FALLBACK,
+				.cra_flags = CRYPTO_ALG_ASYNC |
+					     CRYPTO_ALG_NEED_FALLBACK,
 				.cra_blocksize = SHA512_BLOCK_SIZE,
 				.cra_ctxsize = sizeof(struct tegra_sha_ctx),
 				.cra_alignmask = 0,
 				.cra_module = THIS_MODULE,
 				.cra_init = tegra_sha_cra_init,

base-commit: 5c52607c43c397b79a9852ce33fc61de58c3645c
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH 46/61] vfio: Prefer IS_ERR_OR_NULL over manual NULL check
From: Alex Williamson @ 2026-03-16 22:10 UTC (permalink / raw)
  To: Philipp Hahn
  Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
	gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
	linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
	linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
	linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
	linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
	linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
	linux-sctp, linux-security-module, linux-sh, linux-sound,
	linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
	netdev, ntfs3, samba-technical, sched-ext, target-devel,
	tipc-discussion, v9fs, alex
In-Reply-To: <20260310-b4-is_err_or_null-v1-46-bd63b656022d@avm.de>

On Tue, 10 Mar 2026 12:49:12 +0100
Philipp Hahn <phahn-oss@avm.de> wrote:

> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
> 
> Change generated with coccinelle.
> 
> To: Alex Williamson <alex@shazbot.org>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
>  drivers/vfio/vfio_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 742477546b15d4dbaf9ebcfb2e67627db71521e0..d71922dfde5885967398deddec3e9e04b05adfec 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -923,7 +923,7 @@ vfio_ioctl_device_feature_mig_device_state(struct vfio_device *device,
>  
>  	/* Handle the VFIO_DEVICE_FEATURE_SET */
>  	filp = device->mig_ops->migration_set_state(device, mig.device_state);
> -	if (IS_ERR(filp) || !filp)
> +	if (IS_ERR_OR_NULL(filp))
>  		goto out_copy;
>  
>  	return vfio_ioct_mig_return_fd(filp, arg, &mig);
> 

As others have expressed in general, this doesn't seem to be cleaner
and tends to mask that we consider IS_ERR() and NULL as separate cases
in the goto.  This code looks like it could use some refactoring, and
likely that refactoring should handle the IS_ERR() and NULL cases
separately, but conflating them here is not an improvement.  Thanks,

Alex

^ permalink raw reply

* Re: [PATCH] jbd2: gracefully abort on checkpointing state corruptions
From: Milos Nikic @ 2026-03-16 22:17 UTC (permalink / raw)
  To: Jan Kara; +Cc: tytso, linux-ext4, linux-kernel
In-Reply-To: <3rvkzexi3tgx7cj3kusm2fh6yioake7j2qkh53vwkue7l3x3j5@4ztco37p2pvl>

On Mon, Mar 16, 2026 at 10:34 AM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 09-03-26 16:08:38, Milos Nikic wrote:
> > This patch targets two internal state machine invariants in checkpoint.c
> > residing inside functions that natively return integer error codes.
> >
> > - In jbd2_cleanup_journal_tail(): A blocknr of 0 indicates a severely
> > corrupted journal superblock. Replaced the J_ASSERT with a WARN_ON_ONCE
> > and a graceful journal abort, returning -EUCLEAN.
> >
> > - In jbd2_log_do_checkpoint(): Replaced the J_ASSERT_BH checking for
> > an unexpected buffer_jwrite state. If the warning triggers, we
> > explicitly drop the just-taken get_bh() reference and call __flush_batch()
> > to safely clean up any previously queued buffers in the j_chkpt_bhs array,
> > preventing a memory leak before returning -EUCLEAN.
> >
> > Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
>
> Looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
>                                                                 Honza

Hi Jan,

Thank you for the review!
Just a quick heads-up: I recently sent a v2 of this patch to the list
to address some minor feedback from Baokun (specifically, changing
-EUCLEAN to -EFSCORRUPTED, and ensuring jbd2_journal_abort is called
after __flush_batch).
Does your Reviewed-by still apply to the v2? If so, I can spin up a
quick v3 just to formally collect your tag, or if you prefer, you can
just grab v2 from the list and append it there.

Thanks, Milos


>
> > ---
> >  fs/jbd2/checkpoint.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> > index de89c5bef607..cdfbfd27afae 100644
> > --- a/fs/jbd2/checkpoint.c
> > +++ b/fs/jbd2/checkpoint.c
> > @@ -267,7 +267,17 @@ int jbd2_log_do_checkpoint(journal_t *journal)
> >                        */
> >                       BUFFER_TRACE(bh, "queue");
> >                       get_bh(bh);
> > -                     J_ASSERT_BH(bh, !buffer_jwrite(bh));
> > +                     if (WARN_ON_ONCE(buffer_jwrite(bh))) {
> > +                             put_bh(bh); /* drop the ref we just took */
> > +                             spin_unlock(&journal->j_list_lock);
> > +                             jbd2_journal_abort(journal, -EUCLEAN);
> > +
> > +                             /* Clean up any previously batched buffers */
> > +                             if (batch_count)
> > +                                     __flush_batch(journal, &batch_count);
> > +
> > +                             return -EUCLEAN;
> > +                     }
> >                       journal->j_chkpt_bhs[batch_count++] = bh;
> >                       transaction->t_chp_stats.cs_written++;
> >                       transaction->t_checkpoint_list = jh->b_cpnext;
> > @@ -325,7 +335,10 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
> >
> >       if (!jbd2_journal_get_log_tail(journal, &first_tid, &blocknr))
> >               return 1;
> > -     J_ASSERT(blocknr != 0);
> > +     if (WARN_ON_ONCE(blocknr == 0)) {
> > +             jbd2_journal_abort(journal, -EUCLEAN);
> > +             return -EUCLEAN;
> > +     }
> >
> >       /*
> >        * We need to make sure that any blocks that were recently written out
> > --
> > 2.53.0
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

^ permalink raw reply

* Re: [PATCHBLIZZARD v7] fuse/libfuse/e2fsprogs: containerize ext4 for safer operation
From: Joanne Koong @ 2026-03-16 23:08 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: linux-fsdevel, bpf, linux-ext4, Miklos Szeredi, Bernd Schubert,
	Theodore Ts'o, Neal Gompa, Amir Goldstein, Christian Brauner,
	Jeff Layton, John, demiobenour
In-Reply-To: <20260316180408.GN6069@frogsfrogsfrogs>

On Mon, Mar 16, 2026 at 11:04 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Mon, Mar 16, 2026 at 10:56:21AM -0700, Joanne Koong wrote:
> > On Mon, Feb 23, 2026 at 2:46 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > There are some warts remaining:
> > >
> > > a. I would like to continue the discussion about how the design review
> > >    of this code should be structured, and how might I go about creating
> > >    new userspace filesystem servers -- lightweight new ones based off
> > >    the existing userspace tools?  Or by merging lklfuse?
> >
> > What do you mean by "merging lklfuse"?
>
> Merging the lklfuse project into upstream Linux, which involves running
> the whole kit and caboodle through our review process, and then fixing

Gotcha, so it would basically be having to port this arch/lkl
directory [1] into the linux tree

> user-mode-linux to work anywhere other than x86.

Are lklfuse and user-mode-linux (UML) two separate things or is
lklfuse dependent on user-mode-linux?

>
> > Could you explain what the limitations of lklfuse are compared to the
> > fuse iomap approach in this patchset?
>
> The ones I know about are:
>
> 1> There's no support for vmapped kernel memory in UML mode, so anyone
> who requires a large contiguous memory buffer cannot assemble them out
> of "physical" pages.  This has been a stumbling block for XFS in the
> past.
>
> 2> LKLFUSE still uses the classic fuse IO paths, which means that at
> best you can directio the IO through the lklfuse kernel.  At worst you
> have to use the pagecache inside the lklfuse kernel, which is very
> wasteful.

For the security / isolation use cases you've described, is
near-native performance a hard requirement? As I understand it, the
main use cases of this will be for mounting untrusted disk images and
CI/filesystem testing, or are there broader use cases beyond this?

>
> 3> lklfuse hasn't been updated since 6.6.

Gotcha. So if I'm understanding it correctly, the pros/cons come down to:
lklfuse pros:
- (arguably) easier setup cost. once it's setup (assuming it's
possible to add support for the vmapped kernel memory thing you
mentioned above), it'll automatically work for every filesystem vs.
having to implement a fuse-iomap server for every filesystem
- easier to maintain vs. having to maintain each filesystem's
userspace server implementation

lklfuse cons:
- worse (not sure by how much) performance
- once it's merged into the kernel, we can't choose to not
maintain/support it in the future

Am I understanding this correctly?

In my opinion, if near-native performance is not a hard requirement,
it seems like less pain overall to go with lklfuse. lklfuse seems a
lot easier to maintain and I'm not sure if some complexities like
btrfs's copy-on-write could be handled properly with fuse-iomap.

What are your thoughts on this?

Thanks,
Joanne

[1] https://github.com/lkl/linux/tree/master/arch/lkl

>
> --D
>
> > Thanks,
> > Joanne
> >
> > >
> > > b. ext4 doesn't support out of place writes so I don't know if that
> > >    actually works correctly.
> > >
> > > c. fuse2fs doesn't support the ext4 journal.  Urk.
> > >
> > > d. There's a VERY large quantity of fuse2fs improvements that need to be
> > >    applied before we get to the fuse-iomap parts.  I'm not sending these
> > >    (or the fstests changes) to keep the size of the patchbomb at
> > >    "unreasonably large". :P  As a result, the fstests and e2fsprogs
> > >    postings are very targeted.
> > >
> > > e. I've dropped the fstests part of the patchbomb because v6 was just
> > >    way too long.
> > >
> > > I would like to get the main parts of this submission reviewed for 7.1
> > > now that this has been collecting comments and tweaks in non-rfc status
> > > for 3.5 months.
> > >
> > > Kernel:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=fuse-iomap-bpf
> > >
> > > libfuse:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/libfuse.git/log/?h=fuse-iomap-bpf
> > >
> > > e2fsprogs:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/e2fsprogs.git/log/?h=fuse-iomap-bpf
> > >
> > > fstests:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fuse2fs
> > >
> > > --Darrick

^ permalink raw reply

* Re: [PATCH] ext4: Fix possible NULL pointer dereference in ext4_group_desc_free()
From: Zqiang @ 2026-03-16 23:33 UTC (permalink / raw)
  To: Baokun Li; +Cc: linux-ext4, linux-kernel, tytso, adilger.kernel, libaokun
In-Reply-To: <e275a80b-df14-4d05-9334-b10d2e57fbbc@linux.alibaba.com>

> 
> On 3/16/26 4:20 PM, Zqiang wrote:
> 
> > 
> > This can happen if the kvmalloc_objs() fails and sbi->s_group_desc pointer
> >  is NULL in the ext4_group_desc_init(), and then the ext4_group_desc_free()
> >  is called, leading to a NULL group_desc pointer dereference.
> > 
> >  This commit therefore adds a NULL check for sbi->s_group_desc before
> >  accessing its internal members.
> > 
> >  Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> >  ---
> >  fs/ext4/super.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> >  diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> >  index 43f680c750ae..c4307dc04687 100644
> >  --- a/fs/ext4/super.c
> >  +++ b/fs/ext4/super.c
> >  @@ -1256,9 +1256,11 @@ static void ext4_group_desc_free(struct ext4_sb_info *sbi)
> >  
> >  rcu_read_lock();
> >  group_desc = rcu_dereference(sbi->s_group_desc);
> >  - for (i = 0; i < sbi->s_gdb_count; i++)
> > 
> In ext4_group_desc_init(), s_gdb_count is only assigned after kvmalloc_array
> allocation succeeds. Therefore, when kvmalloc_array fails, the
> brelse(group_desc[i]) in ext4_group_desc_free() will not actually be
> executed,
> and thus this NULL pointer dereference issue will not be triggered.


Thanks for replay, got it, sorry for make noise.

Just then, I find that warning may be trigger:

the kvfree() is called in RCU read critical section, if
the sbi->s_group_desc pointer comes from vmalloc(),
the vfree() is called to release it, but the might_sleep()
is called in the vfree(), this may be trigger warnings in
rcu_sleep_check() when the enable CONFIG_DEBUG_ATOMIC_SLEEP.

May be use rcu_access_pointer() to access sbi->s_group_desc
is enough.

Thanks
Zqiang


> 
> Cheers,
> Baokun
> 
> > 
> > - brelse(group_desc[i]);
> >  - kvfree(group_desc);
> >  + if (group_desc) {
> >  + for (i = 0; i < sbi->s_gdb_count; i++)
> >  + brelse(group_desc[i]);
> >  + kvfree(group_desc);
> >  + }
> >  rcu_read_unlock();
> >  }
> >
>

^ permalink raw reply

* Re: [PATCHBLIZZARD v7] fuse/libfuse/e2fsprogs: containerize ext4 for safer operation
From: Darrick J. Wong @ 2026-03-16 23:41 UTC (permalink / raw)
  To: Joanne Koong
  Cc: linux-fsdevel, bpf, linux-ext4, Miklos Szeredi, Bernd Schubert,
	Theodore Ts'o, Neal Gompa, Amir Goldstein, Christian Brauner,
	Jeff Layton, John, demiobenour
In-Reply-To: <CAJnrk1bmJYMt27xedN2Xza0KPXfYEbh-9g=3fNWru-HRM9WFEw@mail.gmail.com>

On Mon, Mar 16, 2026 at 04:08:55PM -0700, Joanne Koong wrote:
> On Mon, Mar 16, 2026 at 11:04 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Mar 16, 2026 at 10:56:21AM -0700, Joanne Koong wrote:
> > > On Mon, Feb 23, 2026 at 2:46 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > > >
> > > > There are some warts remaining:
> > > >
> > > > a. I would like to continue the discussion about how the design review
> > > >    of this code should be structured, and how might I go about creating
> > > >    new userspace filesystem servers -- lightweight new ones based off
> > > >    the existing userspace tools?  Or by merging lklfuse?
> > >
> > > What do you mean by "merging lklfuse"?
> >
> > Merging the lklfuse project into upstream Linux, which involves running
> > the whole kit and caboodle through our review process, and then fixing
> 
> Gotcha, so it would basically be having to port this arch/lkl
> directory [1] into the linux tree

Right.

> > user-mode-linux to work anywhere other than x86.
> 
> Are lklfuse and user-mode-linux (UML) two separate things or is
> lklfuse dependent on user-mode-linux?

I was under the impression that lklfuse uses UML.  Given the weird
things in arch/lkl/Kconfig:

config 64BIT
	bool "64bit kernel"
	default y if OUTPUT_FORMAT = "pe-x86-64"
	default $(success,$(srctree)/arch/lkl/scripts/cc-objdump-file-format.sh|grep -q '^elf64-') if OUTPUT_FORMAT != "pe-x86-64"

I was kinda guessing x86_64 was the primary target of the developers?

/me notes that he's now looked into libguestfs per Demi Marie's comments
and some curiosity on the part of ngompa and i>

Whatever it is that libguestfs does to stand up unprivileged fs mounts
also could fit this bill.  It's *really* slow to start because it takes
the booted kernel, creates a largeish initramfs, boots that combo via
libvirt, and then fires up a fuse server to talk to the vm kernel.

I think all you'd have to do is change libguestfs to start the VM and
run the fuse server inside a systemd container instead of directly from
the CLI.

> > > Could you explain what the limitations of lklfuse are compared to the
> > > fuse iomap approach in this patchset?
> >
> > The ones I know about are:
> >
> > 1> There's no support for vmapped kernel memory in UML mode, so anyone
> > who requires a large contiguous memory buffer cannot assemble them out
> > of "physical" pages.  This has been a stumbling block for XFS in the
> > past.
> >
> > 2> LKLFUSE still uses the classic fuse IO paths, which means that at
> > best you can directio the IO through the lklfuse kernel.  At worst you
> > have to use the pagecache inside the lklfuse kernel, which is very
> > wasteful.
> 
> For the security / isolation use cases you've described, is
> near-native performance a hard requirement?

Not a hard requirement, just a means to convince people that they can
choose containment without completely collapsing performance.

> As I understand it, the main use cases of this will be for mounting
> untrusted disk images and CI/filesystem testing, or are there broader
> use cases beyond this?

That covers nearly all of it.

> >
> > 3> lklfuse hasn't been updated since 6.6.
> 
> Gotcha. So if I'm understanding it correctly, the pros/cons come down to:
> lklfuse pros:
> - (arguably) easier setup cost. once it's setup (assuming it's
> possible to add support for the vmapped kernel memory thing you
> mentioned above), it'll automatically work for every filesystem vs.
> having to implement a fuse-iomap server for every filesystem

Or even a good non-iomap fuse server for every filesystem.  Admittedly
the weak part of fuse4fs is that libext2fs is not as robust as the
kernel is.

> - easier to maintain vs. having to maintain each filesystem's
> userspace server implementation

Yeah.

> lklfuse cons:
> - worse (not sure by how much) performance

Probably a lot, because now you have to run a full IO stack all the way
through lklfuse.

> - once it's merged into the kernel, we can't choose to not
> maintain/support it in the future

Correct.

> Am I understanding this correctly?
> 
> In my opinion, if near-native performance is not a hard requirement,
> it seems like less pain overall to go with lklfuse. lklfuse seems a
> lot easier to maintain and I'm not sure if some complexities like
> btrfs's copy-on-write could be handled properly with fuse-iomap.

btrfs cow can be done with iomap, at least on the directio end.  It's
the other features like fsverity/fscrypt/data checksumming that aren't
currently supported by iomap.

> What are your thoughts on this?

"Gee, what if I could simplify most of my own work out of existence?"

--D

> Thanks,
> Joanne
> 
> [1] https://github.com/lkl/linux/tree/master/arch/lkl
> 
> >
> > --D
> >
> > > Thanks,
> > > Joanne
> > >
> > > >
> > > > b. ext4 doesn't support out of place writes so I don't know if that
> > > >    actually works correctly.
> > > >
> > > > c. fuse2fs doesn't support the ext4 journal.  Urk.
> > > >
> > > > d. There's a VERY large quantity of fuse2fs improvements that need to be
> > > >    applied before we get to the fuse-iomap parts.  I'm not sending these
> > > >    (or the fstests changes) to keep the size of the patchbomb at
> > > >    "unreasonably large". :P  As a result, the fstests and e2fsprogs
> > > >    postings are very targeted.
> > > >
> > > > e. I've dropped the fstests part of the patchbomb because v6 was just
> > > >    way too long.
> > > >
> > > > I would like to get the main parts of this submission reviewed for 7.1
> > > > now that this has been collecting comments and tweaks in non-rfc status
> > > > for 3.5 months.
> > > >
> > > > Kernel:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=fuse-iomap-bpf
> > > >
> > > > libfuse:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/libfuse.git/log/?h=fuse-iomap-bpf
> > > >
> > > > e2fsprogs:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/e2fsprogs.git/log/?h=fuse-iomap-bpf
> > > >
> > > > fstests:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fuse2fs
> > > >
> > > > --Darrick
> 

^ permalink raw reply

* Re: [PATCHBLIZZARD v7] fuse/libfuse/e2fsprogs: containerize ext4 for safer operation
From: Demi Marie Obenour @ 2026-03-17  0:10 UTC (permalink / raw)
  To: Joanne Koong, Darrick J. Wong
  Cc: linux-fsdevel, bpf, linux-ext4, Miklos Szeredi, Bernd Schubert,
	Theodore Ts'o, Neal Gompa, Amir Goldstein, Christian Brauner,
	Jeff Layton, John
In-Reply-To: <CAJnrk1bmJYMt27xedN2Xza0KPXfYEbh-9g=3fNWru-HRM9WFEw@mail.gmail.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 3067 bytes --]

On 3/16/26 19:08, Joanne Koong wrote:
> On Mon, Mar 16, 2026 at 11:04 AM Darrick J. Wong <djwong@kernel.org> wrote:
>>
>> On Mon, Mar 16, 2026 at 10:56:21AM -0700, Joanne Koong wrote:
>>> On Mon, Feb 23, 2026 at 2:46 PM Darrick J. Wong <djwong@kernel.org> wrote:
>>>>
>>>> There are some warts remaining:
>>>>
>>>> a. I would like to continue the discussion about how the design review
>>>>    of this code should be structured, and how might I go about creating
>>>>    new userspace filesystem servers -- lightweight new ones based off
>>>>    the existing userspace tools?  Or by merging lklfuse?
>>>
>>> What do you mean by "merging lklfuse"?
>>
>> Merging the lklfuse project into upstream Linux, which involves running
>> the whole kit and caboodle through our review process, and then fixing
> 
> Gotcha, so it would basically be having to port this arch/lkl
> directory [1] into the linux tree
> 
>> user-mode-linux to work anywhere other than x86.
> 
> Are lklfuse and user-mode-linux (UML) two separate things or is
> lklfuse dependent on user-mode-linux?
> 
>>
>>> Could you explain what the limitations of lklfuse are compared to the
>>> fuse iomap approach in this patchset?
>>
>> The ones I know about are:
>>
>> 1> There's no support for vmapped kernel memory in UML mode, so anyone
>> who requires a large contiguous memory buffer cannot assemble them out
>> of "physical" pages.  This has been a stumbling block for XFS in the
>> past.
>>
>> 2> LKLFUSE still uses the classic fuse IO paths, which means that at
>> best you can directio the IO through the lklfuse kernel.  At worst you
>> have to use the pagecache inside the lklfuse kernel, which is very
>> wasteful.
> 
> For the security / isolation use cases you've described, is
> near-native performance a hard requirement?

It should be good enough that distros are willing to use it by default.
Otherwise, there is no way to break out of the loop where kernel devs
say to not mount untrusted filesystems, yet the distros keep doing
that because people have stuff they need to get done.

I know from talking to Neal Gompa that libguestfs is too slow for this.

> As I understand it, the
> main use cases of this will be for mounting untrusted disk images and
> CI/filesystem testing, or are there broader use cases beyond this?

Anyone with strong verified boot requirements is going to want this.
The threat model used by Android and Chromium OS is that all on-disk
state is *never* trusted.  The root filesystem is on a dm-verity
protected partition, so mounting it in the kernel is fine.  However,
the writable partition should be mounted in userspace.

I checked with the ChromeOS security team, and they do consider being
able to write corrupted metadata to disk that compromises the system
on the next reboot to be a vulnerability.

This is also a case where performance actually matters, including
for F2FS on zoned UFS which is always copy-on-write for user data.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCHBLIZZARD v7] fuse/libfuse/e2fsprogs: containerize ext4 for safer operation
From: Demi Marie Obenour @ 2026-03-17  0:20 UTC (permalink / raw)
  To: Darrick J. Wong, Joanne Koong
  Cc: linux-fsdevel, bpf, linux-ext4, Miklos Szeredi, Bernd Schubert,
	Theodore Ts'o, Neal Gompa, Amir Goldstein, Christian Brauner,
	Jeff Layton, John
In-Reply-To: <20260316234137.GJ1742010@frogsfrogsfrogs>


[-- Attachment #1.1.1: Type: text/plain, Size: 7274 bytes --]

On 3/16/26 19:41, Darrick J. Wong wrote:
> On Mon, Mar 16, 2026 at 04:08:55PM -0700, Joanne Koong wrote:
>> On Mon, Mar 16, 2026 at 11:04 AM Darrick J. Wong <djwong@kernel.org> wrote:
>>>
>>> On Mon, Mar 16, 2026 at 10:56:21AM -0700, Joanne Koong wrote:
>>>> On Mon, Feb 23, 2026 at 2:46 PM Darrick J. Wong <djwong@kernel.org> wrote:
>>>>>
>>>>> There are some warts remaining:
>>>>>
>>>>> a. I would like to continue the discussion about how the design review
>>>>>    of this code should be structured, and how might I go about creating
>>>>>    new userspace filesystem servers -- lightweight new ones based off
>>>>>    the existing userspace tools?  Or by merging lklfuse?
>>>>
>>>> What do you mean by "merging lklfuse"?
>>>
>>> Merging the lklfuse project into upstream Linux, which involves running
>>> the whole kit and caboodle through our review process, and then fixing
>>
>> Gotcha, so it would basically be having to port this arch/lkl
>> directory [1] into the linux tree
> 
> Right.
> 
>>> user-mode-linux to work anywhere other than x86.
>>
>> Are lklfuse and user-mode-linux (UML) two separate things or is
>> lklfuse dependent on user-mode-linux?
> 
> I was under the impression that lklfuse uses UML.  Given the weird
> things in arch/lkl/Kconfig:
> 
> config 64BIT
> 	bool "64bit kernel"
> 	default y if OUTPUT_FORMAT = "pe-x86-64"
> 	default $(success,$(srctree)/arch/lkl/scripts/cc-objdump-file-format.sh|grep -q '^elf64-') if OUTPUT_FORMAT != "pe-x86-64"
> 
> I was kinda guessing x86_64 was the primary target of the developers?
> 
> /me notes that he's now looked into libguestfs per Demi Marie's comments
> and some curiosity on the part of ngompa and i>
> 
> Whatever it is that libguestfs does to stand up unprivileged fs mounts
> also could fit this bill.  It's *really* slow to start because it takes
> the booted kernel, creates a largeish initramfs, boots that combo via
> libvirt, and then fires up a fuse server to talk to the vm kernel.
> 
> I think all you'd have to do is change libguestfs to start the VM and
> run the fuse server inside a systemd container instead of directly from
> the CLI.

The feedback I have gotten from ngompa is that libguestfs is just
too slow for distros to use it to mount stuff.

>>>> Could you explain what the limitations of lklfuse are compared to the
>>>> fuse iomap approach in this patchset?
>>>
>>> The ones I know about are:
>>>
>>> 1> There's no support for vmapped kernel memory in UML mode, so anyone
>>> who requires a large contiguous memory buffer cannot assemble them out
>>> of "physical" pages.  This has been a stumbling block for XFS in the
>>> past.
>>>
>>> 2> LKLFUSE still uses the classic fuse IO paths, which means that at
>>> best you can directio the IO through the lklfuse kernel.  At worst you
>>> have to use the pagecache inside the lklfuse kernel, which is very
>>> wasteful.
>>
>> For the security / isolation use cases you've described, is
>> near-native performance a hard requirement?
> 
> Not a hard requirement, just a means to convince people that they can
> choose containment without completely collapsing performance.
> 
>> As I understand it, the main use cases of this will be for mounting
>> untrusted disk images and CI/filesystem testing, or are there broader
>> use cases beyond this?
> 
> That covers nearly all of it.

It's worth noting that on ChromeOS and Android, the only trusted
disk images are those that are read-only and protected by dm-verity.
*Every* writable image is considered untrusted.

I don't know if doing a full fsck at each boot is considered
acceptable, but I suspect it would slow boot far too much.

Yes, Google ought to be paying for the kernel changes to fix this mess.

>>> 3> lklfuse hasn't been updated since 6.6.
>>
>> Gotcha. So if I'm understanding it correctly, the pros/cons come down to:
>> lklfuse pros:
>> - (arguably) easier setup cost. once it's setup (assuming it's
>> possible to add support for the vmapped kernel memory thing you
>> mentioned above), it'll automatically work for every filesystem vs.
>> having to implement a fuse-iomap server for every filesystem
> 
> Or even a good non-iomap fuse server for every filesystem.  Admittedly
> the weak part of fuse4fs is that libext2fs is not as robust as the
> kernel is.
> 
>> - easier to maintain vs. having to maintain each filesystem's
>> userspace server implementation
> 
> Yeah.
> 
>> lklfuse cons:
>> - worse (not sure by how much) performance
> 
> Probably a lot, because now you have to run a full IO stack all the way
> through lklfuse.

How much is "a lot"?  Is it "this is only useful for non-interactive
overnight backups", "you will notice this in benchmarks but it's okay
for normal use", or somewhere in between?

Could lklfuse and iomap be combined?

>> - once it's merged into the kernel, we can't choose to not
>> maintain/support it in the future
> 
> Correct.
> 
>> Am I understanding this correctly?
>>
>> In my opinion, if near-native performance is not a hard requirement,
>> it seems like less pain overall to go with lklfuse. lklfuse seems a
>> lot easier to maintain and I'm not sure if some complexities like
>> btrfs's copy-on-write could be handled properly with fuse-iomap.
> 
> btrfs cow can be done with iomap, at least on the directio end.  It's
> the other features like fsverity/fscrypt/data checksumming that aren't
> currently supported by iomap.

Pretty much everyone on btrfs uses data checksumming.

>> What are your thoughts on this?
> 
> "Gee, what if I could simplify most of my own work out of existence?"

What is that work?

> --D
> 
>> Thanks,
>> Joanne
>>
>> [1] https://github.com/lkl/linux/tree/master/arch/lkl
>>
>>>
>>> --D
>>>
>>>> Thanks,
>>>> Joanne
>>>>
>>>>>
>>>>> b. ext4 doesn't support out of place writes so I don't know if that
>>>>>    actually works correctly.
>>>>>
>>>>> c. fuse2fs doesn't support the ext4 journal.  Urk.
>>>>>
>>>>> d. There's a VERY large quantity of fuse2fs improvements that need to be
>>>>>    applied before we get to the fuse-iomap parts.  I'm not sending these
>>>>>    (or the fstests changes) to keep the size of the patchbomb at
>>>>>    "unreasonably large". :P  As a result, the fstests and e2fsprogs
>>>>>    postings are very targeted.
>>>>>
>>>>> e. I've dropped the fstests part of the patchbomb because v6 was just
>>>>>    way too long.
>>>>>
>>>>> I would like to get the main parts of this submission reviewed for 7.1
>>>>> now that this has been collecting comments and tweaks in non-rfc status
>>>>> for 3.5 months.
>>>>>
>>>>> Kernel:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=fuse-iomap-bpf
>>>>>
>>>>> libfuse:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/libfuse.git/log/?h=fuse-iomap-bpf
>>>>>
>>>>> e2fsprogs:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/e2fsprogs.git/log/?h=fuse-iomap-bpf
>>>>>
>>>>> fstests:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fuse2fs
>>>>>
>>>>> --Darrick
>>


-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v18 00/42] DEPT(DEPendency Tracker)
From: Byungchul Park @ 2026-03-17  4:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel_team, torvalds, damien.lemoal, linux-ide, adilger.kernel,
	linux-ext4, mingo, peterz, will, tglx, rostedt, joel, sashal,
	daniel.vetter, duyuyang, johannes.berg, tj, tytso, willy, david,
	amir73il, gregkh, kernel-team, linux-mm, akpm, mhocko, minchan,
	hannes, vdavydov.dev, sj, jglisse, dennis, cl, penberg, rientjes,
	vbabka, ngupta, linux-block, josef, linux-fsdevel, jack, jlayton,
	dan.j.williams, hch, djwong, dri-devel, rodrigosiqueiramelo,
	melissa.srw, hamohammed.sa, harry.yoo, chris.p.wilson,
	gwan-gyeong.mun, max.byungchul.park, boqun.feng, longman,
	yunseong.kim, ysk, yeoreum.yun, netdev, matthew.brost, her0gyugyu,
	corbet, catalin.marinas, bp, x86, hpa, luto, sumit.semwal,
	gustavo, christian.koenig, andi.shyti, arnd, lorenzo.stoakes,
	Liam.Howlett, rppt, surenb, mcgrof, petr.pavlu, da.gomez,
	samitolvanen, paulmck, frederic, neeraj.upadhyay, joelagnelf,
	josh, urezki, mathieu.desnoyers, jiangshanlai, qiang.zhang,
	juri.lelli, vincent.guittot, dietmar.eggemann, bsegall, mgorman,
	vschneid, chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy,
	anna, kees, bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
	kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
	shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
	joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
	alexander.shishkin, lillian, chenhuacai, francesco,
	guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
	thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
	linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
	linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel,
	2407018371, dakr, miguel.ojeda.sandonis, neilb, bagasdotme,
	wsa+renesas, dave.hansen, geert, ojeda, alex.gaynor, gary,
	bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux
In-Reply-To: <20251205071855.72743-1-byungchul@sk.com>

On Fri, Dec 05, 2025 at 04:18:13PM +0900, Byungchul Park wrote:
> I'm happy to see that DEPT reported real problems in practice:
> 
>    https://lore.kernel.org/lkml/6383cde5-cf4b-facf-6e07-1378a485657d@I-love.SAKURA.ne.jp/
>    https://lore.kernel.org/lkml/1674268856-31807-1-git-send-email-byungchul.park@lge.com/
>    https://lore.kernel.org/all/b6e00e77-4a8c-4e05-ab79-266bf05fcc2d@igalia.com/
> 
> I’ve added documentation describing DEPT — this should help you
> understand what DEPT is and how it works.  You can use DEPT simply by
> enabling CONFIG_DEPT and checking dmesg at runtime.
> ---
> 
> Hi Linus and folks,

Hi Linus and Ingo,

Although the DEPT tool still has areas for improvement, I am confident
it employs the most appropriate method for tracking dependencies within
Linux kernel.

If it is to be maintained in the source tree, which subsystem would be
the most suitable for its management?  Personally, I believe introducing
a dedicated dependency subsystem under 'kernel/' would be ideal, though
managing it within the already well-maintained scheduler or locking
subsystems might be more practical.

Since DEPT tracks not only locking mechanisms but also general sleep and
wake events, I have avoided placing it under the locking subsystem thus
far.  If you have alternative or more fitting suggestions, I would
appreciate your input.

Thanks.

	Byungchul
> 
> I’ve been developing a tool to detect deadlock possibilities by tracking
> waits/events — rather than lock acquisition order — to cover all the
> synchronization mechanisms.  To summarize the design rationale, starting
> from the problem statement, through analysis, to the solution:
> 
>    CURRENT STATUS
>    --------------
>    Lockdep tracks lock acquisition order to identify deadlock conditions.
>    Additionally, it tracks IRQ state changes — via {en,dis}able — to
>    detect cases where locks are acquired unintentionally during
>    interrupt handling.
>    
>    PROBLEM
>    -------
>    Waits and their associated events that are never reachable can
>    eventually lead to deadlocks.  However, since Lockdep focuses solely
>    on lock acquisition order, it has inherent limitations when handling
>    waits and events.
>    
>    Moreover, by tracking only lock acquisition order, Lockdep cannot
>    properly handle read locks or cross-event scenarios — such as
>    wait_for_completion() and complete() — making it increasingly
>    inadequate as a general-purpose deadlock detection tool.
>    
>    SOLUTION
>    --------
>    Once again, waits and their associated events that are never
>    reachable can eventually lead to deadlocks.  The new solution, DEPT,
>    focuses directly on waits and events.  DEPT monitors waits and events,
>    and reports them when any become unreachable.
> 
> DEPT provides:
> 
>    * Correct handling of read locks.
>    * Support for general waits and events.
>    * Continuous operation, even after multiple reports.
>    * Simple, intuitive annotation APIs.
> 
> There are still false positives, and some are already being worked on
> for suppression.  Especially splitting the folio class into several
> appropriate classes e.g. block device mapping class and regular file
> mapping class, is currently under active development by me and Yeoreum
> Yun.
> 
> Anyway, these efforts will need to continue for a while, as we’ve seen
> with lockdep over two decades.  DEPT is tagged as EXPERIMENTAL in
> Kconfig — meaning it’s not yet suitable for use as an automation tool.
> 
> However, for those who are interested in using DEPT to analyze complex
> synchronization patterns and extract dependency insights, DEPT would be
> a great tool for the purpose.
> 
> Thanks for your support and contributions to:
> 
>    Harry Yoo <harry.yoo@oracle.com>
>    Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>    Yunseong Kim <ysk@kzalloc.com>
>    Yeoreum Yun <yeoreum.yun@arm.com>
> 
> FAQ
> ---
> Q. Is this the first attempt to solve this problem?
> A. No. The cross-release feature (commit b09be676e0ff2) attempted to
>    address it — as a Lockdep extension.  It was merged, but quickly
>    reverted, because:
> 
>    While it uncovered valuable hidden issues, it also introduced false
>    positives.  Since these false positives mask further real problems
>    with Lockdep — and developers strongly dislike them — the feature was
>    rolled back.
> 
> Q. Why wasn’t DEPT built as a Lockdep extension?
> A. Lockdep is the result of years of work by kernel developers — and is
>    now very stable. But I chose to build DEPT separately, because:
> 
>    While reusing BFS(Breadth First Search) and Lockdep’s hashing is
>    beneficial, the rest of the system must be rebuilt from scratch to
>    align with DEPT’s wait-event model — since Lockdep was originally
>    designed for tracking lock acquisition orders, not wait-event
>    dependencies.
> 
> Q. Do you plan to replace Lockdep entirely?
> A. Not at all — Lockdep still plays a vital role in validating correct
>    lock usage.  While its dependency-checking logic should eventually be
>    superseded by DEPT, the rest of its functionality should stay.
> 
> Q. Should we replace the dependency check immediately?
> A. Absolutely not.  Lockdep’s stability is the result of years of hard
>    work by kernel developers.  Lockdep and DEPT should run side by side
>    until DEPT matures.
> 
> Q. Stronger detection often leads to more false positives — which was a
>    major pain point when cross-release was added.  Is DEPT designed to
>    handle this?
> A. Yes.  DEPT’s simple, generalized design enables flexible reporting —
>    so while false positives still need fixing, they’re far less
>    disruptive than they were under the Lockdep extension, cross-release.
> 
> Q. Why not fix all false positives out-of-tree before merging?
> A. Since the affected subsystems span the entire kernel, like Lockdep,
>    which has relied on annotations to avoid false positives over the
>    last two decades, DEPT too will require the annotation efforts.
> 
>    Performing annotation work within the mainline will help us add
>    annotations more appropriately and will also make DEPT a useful tool
>    for a wider range of users more quickly.
> 
>    CONFIG_DEPT is marked EXPERIMENTAL, so it’s opt-in. Some users are
>    already interested in using DEPT to analyze complex synchronization
>    patterns and extract dependency insights.
> 
> 	Byungchul
> ---
> Changes from v17:
> 
> 	1. Rebase on the mainline as of 2025 Dec 5.
> 	2. Convert the documents' format from txt to rst. (feedbacked
> 	   by Jonathan Corbet and Bagas Sanjaya)
> 	3. Move the documents from 'Documentation/dependency' to
> 	   'Documentation/dev-tools'. (feedbakced by Jonathan Corbet)
> 	4. Improve the documentation. (feedbacked by NeilBrown)
> 	5. Use a common function, enter_from_user_mode(), instead of
> 	   arch specific code, to notice context switch from user mode.
> 	   (feedbacked by Dave Hansen, Mark Rutland, and Mark Brown)
> 	6. Resolve the header dependency issue by using dept's internal
> 	   header, instead of relocating 'struct llist_{head,node}' to
> 	   another header. (feedbacked by Greg KH)
> 	7. Improve page(or folio) usage type APIs.
> 	8. Add rust helper for wait_for_completion(). (feedbacked by
> 	   Guangbo Cui, Boqun Feng, and Danilo Krummrich)
> 	9. Refine some commit messages.
> 
> Changes from v16:
> 
> 	1. Rebase on v6.17.
> 	2. Fix a false positive from rcu (by Yunseong Kim)
> 	3. Introduce APIs to set page's usage, dept_set_page_usage() and
> 	   dept_reset_page_usage() to avoid false positives.
> 	4. Consider lock_page() as a potential wait unconditionally.
> 	5. Consider folio_lock_killable() as a potential wait
> 	   unconditionally.
> 	6. Add support for tracking PG_writeback waits and events.
> 	7. Fix two build errors due to the additional debug information
> 	   added by dept. (by Yunseong Kim)
> 
> Changes from v15:
> 
> 	1. Fix typo and improve comments and commit messages (feedbacked
> 	   by ALOK TIWARI, Waiman Long, and kernel test robot).
> 	2. Do not stop dept on detection of cicular dependency of
> 	   recover event, allowing to keep reporting.
> 	3. Add SK hynix to copyright.
> 	4. Consider folio_lock() as a potential wait unconditionally.
> 	5. Fix Kconfig dependency bug (feedbacked by kernel test rebot).
> 	6. Do not suppress reports that involve classes even that have
> 	   already involved in other reports, allowing to keep
> 	   reporting.
> 
> Changes from v14:
> 
> 	1. Rebase on the current latest, v6.15-rc6.
> 	2. Refactor dept code.
> 	3. With multi event sites for a single wait, even if an event
> 	   forms a circular dependency, the event can be recovered by
> 	   other event(or wake up) paths.  Even though informing the
> 	   circular dependency is worthy but it should be suppressed
> 	   once informing it, if it doesn't lead an actual deadlock.  So
> 	   introduce APIs to annotate the relationship between event
> 	   site and recover site, that are, event_site() and
> 	   dept_recover_event().
> 	4. wait_for_completion() worked with dept map embedded in struct
> 	   completion.  However, it generates a few false positves since
> 	   all the waits using the instance of struct completion, share
> 	   the map and key.  To avoid the false positves, make it not to
> 	   share the map and key but each wait_for_completion() caller
> 	   have its own key by default.  Of course, external maps also
> 	   can be used if needed.
> 	5. Fix a bug about hardirq on/off tracing.
> 	6. Implement basic unit test for dept.
> 	7. Add more supports for dma fence synchronization.
> 	8. Add emergency stop of dept e.g. on panic().
> 	9. Fix false positives by mmu_notifier_invalidate_*().
> 	10. Fix recursive call bug by DEPT_WARN_*() and DEPT_STOP().
> 	11. Fix trivial bugs in DEPT_WARN_*() and DEPT_STOP().
> 	12. Fix a bug that a spin lock, dept_pool_spin, is used in
> 	    both contexts of irq disabled and enabled without irq
> 	    disabled.
> 	13. Suppress reports with classes, any of that already have
> 	    been reported, even though they have different chains but
> 	    being barely meaningful.
> 	14. Print stacktrace of the wait that an event is now waking up,
> 	    not only stacktrace of the event.
> 	15. Make dept aware of lockdep_cmp_fn() that is used to avoid
> 	    false positives in lockdep so that dept can also avoid them.
> 	16. Do do_event() only if there are no ecxts have been
> 	    delimited.
> 	17. Fix a bug that was not synchronized for stage_m in struct
> 	    dept_task, using a spin lock, dept_task()->stage_lock.
> 	18. Fix a bug that dept didn't handle the case that multiple
> 	    ttwus for a single waiter can be called at the same time
> 	    e.i. a race issue.
> 	19. Distinguish each kernel context from others, not only by
> 	    system call but also by user oriented fault so that dept can
> 	    work with more accuracy information about kernel context.
> 	    That helps to avoid a few false positives.
> 	20. Limit dept's working to x86_64 and arm64.
> 
> Changes from v13:
> 
> 	1. Rebase on the current latest version, v6.9-rc7.
> 	2. Add 'dept' documentation describing dept APIs.
> 
> Changes from v12:
> 
> 	1. Refine the whole document for dept.
> 	2. Add 'Interpret dept report' section in the document, using a
> 	   deadlock report obtained in practice. Hope this version of
> 	   document helps guys understand dept better.
> 
> 	   https://lore.kernel.org/lkml/6383cde5-cf4b-facf-6e07-1378a485657d@I-love.SAKURA.ne.jp/#t
> 	   https://lore.kernel.org/lkml/1674268856-31807-1-git-send-email-byungchul.park@lge.com/
> 
> Changes from v11:
> 
> 	1. Add 'dept' documentation describing the concept of dept.
> 	2. Rewrite the commit messages of the following commits for
> 	   using weaker lockdep annotation, for better description.
> 
> 	   fs/jbd2: Use a weaker annotation in journal handling
> 	   cpu/hotplug: Use a weaker annotation in AP thread
> 
> 	   (feedbacked by Thomas Gleixner)
> 
> Changes from v10:
> 
> 	1. Fix noinstr warning when building kernel source.
> 	2. dept has been reporting some false positives due to the folio
> 	   lock's unfairness. Reflect it and make dept work based on
> 	   dept annotaions instead of just wait and wake up primitives.
> 	3. Remove the support for PG_writeback while working on 2. I
> 	   will add the support later if needed.
> 	4. dept didn't print stacktrace for [S] if the participant of a
> 	   deadlock is not lock mechanism but general wait and event.
> 	   However, it made hard to interpret the report in that case.
> 	   So add support to print stacktrace of the requestor who asked
> 	   the event context to run - usually a waiter of the event does
> 	   it just before going to wait state.
> 	5. Give up tracking raw_local_irq_{disable,enable}() since it
> 	   totally messed up dept's irq tracking. So make it work in the
> 	   same way as lockdep does. I will consider it once any false
> 	   positives by those are observed again.
> 	6. Change the manual rwsem_acquire_read(->j_trans_commit_map)
> 	   annotation in fs/jbd2/transaction.c to the try version so
> 	   that it works as much as it exactly needs.
> 	7. Remove unnecessary 'inline' keyword in dept.c and add
> 	   '__maybe_unused' to a needed place.
> 
> Changes from v9:
> 
> 	1. Fix a bug. SDT tracking didn't work well because of my big
> 	   mistake that I should've used waiter's map to indentify its
> 	   class but it had been working with waker's one. FYI,
> 	   PG_locked and PG_writeback weren't affected. They still
> 	   worked well. (reported by YoungJun)
> 	
> Changes from v8:
> 
> 	1. Fix build error by adding EXPORT_SYMBOL(PG_locked_map) and
> 	   EXPORT_SYMBOL(PG_writeback_map) for kernel module build -
> 	   appologize for that. (reported by kernel test robot)
> 	2. Fix build error by removing header file's circular dependency
> 	   that was caused by "atomic.h", "kernel.h" and "irqflags.h",
> 	   which I introduced - appolgize for that. (reported by kernel
> 	   test robot)
> 
> Changes from v7:
> 
> 	1. Fix a bug that cannot track rwlock dependency properly,
> 	   introduced in v7. (reported by Boqun and lockdep selftest)
> 	2. Track wait/event of PG_{locked,writeback} more aggressively
> 	   assuming that when a bit of PG_{locked,writeback} is cleared
> 	   there might be waits on the bit. (reported by Linus, Hillf
> 	   and syzbot)
> 	3. Fix and clean bad style code e.i. unnecessarily introduced
> 	   a randome pattern and so on. (pointed out by Linux)
> 	4. Clean code for applying dept to wait_for_completion().
> 
> Changes from v6:
> 
> 	1. Tie to task scheduler code to track sleep and try_to_wake_up()
> 	   assuming sleeps cause waits, try_to_wake_up()s would be the
> 	   events that those are waiting for, of course with proper dept
> 	   annotations, sdt_might_sleep_weak(), sdt_might_sleep_strong()
> 	   and so on. For these cases, class is classified at sleep
> 	   entrance rather than the synchronization initialization code.
> 	   Which would extremely reduce false alarms.
> 	2. Remove the dept associated instance in each page struct for
> 	   tracking dependencies by PG_locked and PG_writeback thanks to
> 	   the 1. work above.
> 	3. Introduce CONFIG_dept_AGGRESIVE_TIMEOUT_WAIT to suppress
> 	   reports that waits with timeout set are involved, for those
> 	   who don't like verbose reporting.
> 	4. Add a mechanism to refill the internal memory pools on
> 	   running out so that dept could keep working as long as free
> 	   memory is available in the system.
> 	5. Re-enable tracking hashed-waitqueue wait. That's going to no
> 	   longer generate false positives because class is classified
> 	   at sleep entrance rather than the waitqueue initailization.
> 	6. Refactor to make it easier to port onto each new version of
> 	   the kernel.
> 	7. Apply dept to dma fence.
> 	8. Do trivial optimizaitions.
> 
> Changes from v5:
> 
> 	1. Use just pr_warn_once() rather than WARN_ONCE() on the lack
> 	   of internal resources because WARN_*() printing stacktrace is
> 	   too much for informing the lack. (feedback from Ted, Hyeonggon)
> 	2. Fix trivial bugs like missing initializing a struct before
> 	   using it.
> 	3. Assign a different class per task when handling onstack
> 	   variables for waitqueue or the like. Which makes dept
> 	   distinguish between onstack variables of different tasks so
> 	   as to prevent false positives. (reported by Hyeonggon)
> 	4. Make dept aware of even raw_local_irq_*() to prevent false
> 	   positives. (reported by Hyeonggon)
> 	5. Don't consider dependencies between the events that might be
> 	   triggered within __schedule() and the waits that requires
> 	    __schedule(), real ones. (reported by Hyeonggon)
> 	6. Unstage the staged wait that has prepare_to_wait_event()'ed
> 	   *and* yet to get to __schedule(), if we encounter __schedule()
> 	   in-between for another sleep, which is possible if e.g. a
> 	   mutex_lock() exists in 'condition' of ___wait_event().
> 	7. Turn on CONFIG_PROVE_LOCKING when CONFIG_DEPT is on, to rely
> 	   on the hardirq and softirq entrance tracing to make dept more
> 	   portable for now.
> 
> Changes from v4:
> 
> 	1. Fix some bugs that produce false alarms.
> 	2. Distinguish each syscall context from another *for arm64*.
> 	3. Make it not warn it but just print it in case dept ring
> 	   buffer gets exhausted. (feedback from Hyeonggon)
> 	4. Explicitely describe "EXPERIMENTAL" and "dept might produce
> 	   false positive reports" in Kconfig. (feedback from Ted)
> 
> Changes from v3:
> 
> 	1. dept shouldn't create dependencies between different depths
> 	   of a class that were indicated by *_lock_nested(). dept
> 	   normally doesn't but it does once another lock class comes
> 	   in. So fixed it. (feedback from Hyeonggon)
> 	2. dept considered a wait as a real wait once getting to
> 	   __schedule() even if it has been set to TASK_RUNNING by wake
> 	   up sources in advance. Fixed it so that dept doesn't consider
> 	   the case as a real wait. (feedback from Jan Kara)
> 	3. Stop tracking dependencies with a map once the event
> 	   associated with the map has been handled. dept will start to
> 	   work with the map again, on the next sleep.
> 
> Changes from v2:
> 
> 	1. Disable dept on bit_wait_table[] in sched/wait_bit.c
> 	   reporting a lot of false positives, which is my fault.
> 	   Wait/event for bit_wait_table[] should've been tagged in a
> 	   higher layer for better work, which is a future work.
> 	   (feedback from Jan Kara)
> 	2. Disable dept on crypto_larval's completion to prevent a false
> 	   positive.
> 
> Changes from v1:
> 
> 	1. Fix coding style and typo. (feedback from Steven)
> 	2. Distinguish each work context from another in workqueue.
> 	3. Skip checking lock acquisition with nest_lock, which is about
> 	   correct lock usage that should be checked by lockdep.
> 
> Changes from RFC(v0):
> 
> 	1. Prevent adding a wait tag at prepare_to_wait() but __schedule().
> 	   (feedback from Linus and Matthew)
> 	2. Use try version at lockdep_acquire_cpus_lock() annotation.
> 	3. Distinguish each syscall context from another.
> 
> Byungchul Park (41):
>   dept: implement DEPT(DEPendency Tracker)
>   dept: add single event dependency tracker APIs
>   dept: add lock dependency tracker APIs
>   dept: tie to lockdep and IRQ tracing
>   dept: add proc knobs to show stats and dependency graph
>   dept: distinguish each kernel context from another
>   dept: distinguish each work from another
>   dept: add a mechanism to refill the internal memory pools on running
>     out
>   dept: record the latest one out of consecutive waits of the same class
>   dept: apply sdt_might_sleep_{start,end}() to
>     wait_for_completion()/complete()
>   dept: apply sdt_might_sleep_{start,end}() to swait
>   dept: apply sdt_might_sleep_{start,end}() to waitqueue wait
>   dept: apply sdt_might_sleep_{start,end}() to hashed-waitqueue wait
>   dept: apply sdt_might_sleep_{start,end}() to dma fence
>   dept: track timeout waits separately with a new Kconfig
>   dept: apply timeout consideration to wait_for_completion()/complete()
>   dept: apply timeout consideration to swait
>   dept: apply timeout consideration to waitqueue wait
>   dept: apply timeout consideration to hashed-waitqueue wait
>   dept: apply timeout consideration to dma fence wait
>   dept: make dept able to work with an external wgen
>   dept: track PG_locked with dept
>   dept: print staged wait's stacktrace on report
>   locking/lockdep: prevent various lockdep assertions when
>     lockdep_off()'ed
>   dept: add documents for dept
>   cpu/hotplug: use a weaker annotation in AP thread
>   dept: assign dept map to mmu notifier invalidation synchronization
>   dept: assign unique dept_key to each distinct dma fence caller
>   dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
>   dept: make dept stop from working on debug_locks_off()
>   dept: assign unique dept_key to each distinct wait_for_completion()
>     caller
>   completion, dept: introduce init_completion_dmap() API
>   dept: introduce a new type of dependency tracking between multi event
>     sites
>   dept: add module support for struct dept_event_site and
>     dept_event_site_dep
>   dept: introduce event_site() to disable event tracking if it's
>     recoverable
>   dept: implement a basic unit test for dept
>   dept: call dept_hardirqs_off() in local_irq_*() regardless of irq
>     state
>   dept: introduce APIs to set page usage and use subclasses_evt for the
>     usage
>   dept: track PG_writeback with dept
>   SUNRPC: relocate struct rcu_head to the first field of struct rpc_xprt
>   mm: percpu: increase PERCPU_DYNAMIC_SIZE_SHIFT on DEPT and large
>     PAGE_SIZE
> 
> Yunseong Kim (1):
>   rcu/update: fix same dept key collision between various types of RCU
> 
>  Documentation/dev-tools/dept.rst     |  778 ++++++
>  Documentation/dev-tools/dept_api.rst |  125 +
>  drivers/dma-buf/dma-fence.c          |   23 +-
>  include/asm-generic/vmlinux.lds.h    |   13 +-
>  include/linux/completion.h           |  124 +-
>  include/linux/dept.h                 |  402 +++
>  include/linux/dept_ldt.h             |   78 +
>  include/linux/dept_sdt.h             |   68 +
>  include/linux/dept_unit_test.h       |   67 +
>  include/linux/dma-fence.h            |   74 +-
>  include/linux/hardirq.h              |    3 +
>  include/linux/irq-entry-common.h     |    4 +
>  include/linux/irqflags.h             |   21 +-
>  include/linux/local_lock_internal.h  |    1 +
>  include/linux/lockdep.h              |  105 +-
>  include/linux/lockdep_types.h        |    3 +
>  include/linux/mm_types.h             |    4 +
>  include/linux/mmu_notifier.h         |   26 +
>  include/linux/module.h               |    5 +
>  include/linux/mutex.h                |    1 +
>  include/linux/page-flags.h           |  217 +-
>  include/linux/pagemap.h              |   37 +-
>  include/linux/percpu-rwsem.h         |    2 +-
>  include/linux/percpu.h               |    4 +
>  include/linux/rcupdate_wait.h        |   13 +-
>  include/linux/rtmutex.h              |    1 +
>  include/linux/rwlock_types.h         |    1 +
>  include/linux/rwsem.h                |    1 +
>  include/linux/sched.h                |  118 +
>  include/linux/seqlock.h              |    2 +-
>  include/linux/spinlock_types_raw.h   |    3 +
>  include/linux/srcu.h                 |    2 +-
>  include/linux/sunrpc/xprt.h          |    9 +-
>  include/linux/swait.h                |    3 +
>  include/linux/wait.h                 |    3 +
>  include/linux/wait_bit.h             |    3 +
>  init/init_task.c                     |    2 +
>  init/main.c                          |    2 +
>  kernel/Makefile                      |    1 +
>  kernel/cpu.c                         |    2 +-
>  kernel/dependency/Makefile           |    5 +
>  kernel/dependency/dept.c             | 3499 ++++++++++++++++++++++++++
>  kernel/dependency/dept_hash.h        |   10 +
>  kernel/dependency/dept_internal.h    |  314 +++
>  kernel/dependency/dept_object.h      |   13 +
>  kernel/dependency/dept_proc.c        |   94 +
>  kernel/dependency/dept_unit_test.c   |  173 ++
>  kernel/exit.c                        |    1 +
>  kernel/fork.c                        |    2 +
>  kernel/locking/lockdep.c             |   33 +
>  kernel/module/main.c                 |   19 +
>  kernel/rcu/rcu.h                     |    1 +
>  kernel/rcu/update.c                  |    5 +-
>  kernel/sched/completion.c            |   62 +-
>  kernel/sched/core.c                  |    9 +
>  kernel/workqueue.c                   |    3 +
>  lib/Kconfig.debug                    |   48 +
>  lib/debug_locks.c                    |    2 +
>  lib/locking-selftest.c               |    2 +
>  mm/filemap.c                         |   38 +
>  mm/mm_init.c                         |    3 +
>  mm/mmu_notifier.c                    |   31 +-
>  rust/helpers/completion.c            |    5 +
>  63 files changed, 6602 insertions(+), 121 deletions(-)
>  create mode 100644 Documentation/dev-tools/dept.rst
>  create mode 100644 Documentation/dev-tools/dept_api.rst
>  create mode 100644 include/linux/dept.h
>  create mode 100644 include/linux/dept_ldt.h
>  create mode 100644 include/linux/dept_sdt.h
>  create mode 100644 include/linux/dept_unit_test.h
>  create mode 100644 kernel/dependency/Makefile
>  create mode 100644 kernel/dependency/dept.c
>  create mode 100644 kernel/dependency/dept_hash.h
>  create mode 100644 kernel/dependency/dept_internal.h
>  create mode 100644 kernel/dependency/dept_object.h
>  create mode 100644 kernel/dependency/dept_proc.c
>  create mode 100644 kernel/dependency/dept_unit_test.c
> 
> 
> base-commit: 43dfc13ca972988e620a6edb72956981b75ab6b0
> -- 
> 2.17.1

^ permalink raw reply

* Re: [PATCH] ext4: Fix possible NULL pointer dereference in ext4_group_desc_free()
From: Baokun Li @ 2026-03-17  6:32 UTC (permalink / raw)
  To: Zqiang; +Cc: linux-ext4, linux-kernel, tytso, adilger.kernel, libaokun
In-Reply-To: <584a552d5b5cf7554f4a1c607e62f308436ff3eb@linux.dev>


On 3/17/26 7:33 AM, Zqiang wrote:
>> On 3/16/26 4:20 PM, Zqiang wrote:
>>
>>> This can happen if the kvmalloc_objs() fails and sbi->s_group_desc pointer
>>>  is NULL in the ext4_group_desc_init(), and then the ext4_group_desc_free()
>>>  is called, leading to a NULL group_desc pointer dereference.
>>>
>>>  This commit therefore adds a NULL check for sbi->s_group_desc before
>>>  accessing its internal members.
>>>
>>>  Signed-off-by: Zqiang <qiang.zhang@linux.dev>
>>>  ---
>>>  fs/ext4/super.c | 8 +++++---
>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>>  diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>>>  index 43f680c750ae..c4307dc04687 100644
>>>  --- a/fs/ext4/super.c
>>>  +++ b/fs/ext4/super.c
>>>  @@ -1256,9 +1256,11 @@ static void ext4_group_desc_free(struct ext4_sb_info *sbi)
>>>  
>>>  rcu_read_lock();
>>>  group_desc = rcu_dereference(sbi->s_group_desc);
>>>  - for (i = 0; i < sbi->s_gdb_count; i++)
>>>
>> In ext4_group_desc_init(), s_gdb_count is only assigned after kvmalloc_array
>> allocation succeeds. Therefore, when kvmalloc_array fails, the
>> brelse(group_desc[i]) in ext4_group_desc_free() will not actually be
>> executed,
>> and thus this NULL pointer dereference issue will not be triggered.
>
> Thanks for replay, got it, sorry for make noise.
>
> Just then, I find that warning may be trigger:
>
> the kvfree() is called in RCU read critical section, if
> the sbi->s_group_desc pointer comes from vmalloc(),
> the vfree() is called to release it, but the might_sleep()
> is called in the vfree(), this may be trigger warnings in
> rcu_sleep_check() when the enable CONFIG_DEBUG_ATOMIC_SLEEP.

Indeed, vfree triggers the following warning: ```
===========================================================================
EXT4-fs (vdc): unmounting filesystem
c478da00-c52c-4dd4-81c1-d4f93e12ab50. BUG: sleeping function called from
invalid context at mm/vmalloc.c:3441 in_atomic(): 0, irqs_disabled(): 0,
non_block: 0, pid: 457, name: umount preempt_count: 0, expected: 0 RCU
nest depth: 1, expected: 0 CPU: 0 UID: 0 PID: 457 Comm: umount Tainted:
G W 6.19.0-rc4-g4f5e8e6f0123-dirty #10 PREEMPT(none) Tainted: [W]=WARN
Call Trace: <TASK> dump_stack_lvl+0x55/0x70 __might_resched+0x116/0x160
vfree+0x38/0x60 ext4_put_super+0x1ac/0x490
generic_shutdown_super+0x81/0x180 kill_block_super+0x1a/0x40
ext4_kill_sb+0x22/0x40 deactivate_locked_super+0x35/0xb0
cleanup_mnt+0x101/0x170 task_work_run+0x5c/0xa0
exit_to_user_mode_loop+0xe2/0x460 do_syscall_64+0x1de/0x1f0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
===========================================================================
``` And ext4_mb_init_backend, ext4_mb_release, ext4_flex_groups_free
have similar issues.Indeed, vfree triggers the following warning:

===========================================================================
EXT4-fs (vdc): unmounting filesystem c478da00-c52c-4dd4-81c1-d4f93e12ab50.
BUG: sleeping function called from invalid context at mm/vmalloc.c:3441
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 457, name: umount
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
CPU: 0 UID: 0 PID: 457 Comm: umount Tainted: G W
               6.19.0-rc4-g4f5e8e6f0123-dirty #10 PREEMPT(none)
Tainted: [W]=WARN
Call Trace:
 <TASK>
 dump_stack_lvl+0x55/0x70
 __might_resched+0x116/0x160
 vfree+0x38/0x60
 ext4_put_super+0x1ac/0x490
 generic_shutdown_super+0x81/0x180
 kill_block_super+0x1a/0x40
 ext4_kill_sb+0x22/0x40
 deactivate_locked_super+0x35/0xb0
 cleanup_mnt+0x101/0x170
 task_work_run+0x5c/0xa0
 exit_to_user_mode_loop+0xe2/0x460
 do_syscall_64+0x1de/0x1f0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
===========================================================================

And ext4_mb_init_backend, ext4_mb_release, ext4_flex_groups_free have
similar issues.

> May be use rcu_access_pointer() to access sbi->s_group_desc
> is enough.
>
Yes, these are all initialization or teardown paths and cannot run
concurrently with
online resize, so using rcu_access_pointer() seems sufficient.

Also, should we add might_sleep() to kvfree() to prevent similar issues?


Cheers,
Baokun


^ permalink raw reply

* [RFC v5 0/7] ext4: fast commit: snapshot inode state for FC log
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-ext4,
	linux-trace-kernel, linux-kernel

Hi,

(This RFC v5 series is based on linux-next tag next-20260106 plus the
prerequisite patch "ext4: fast commit: make s_fc_lock reclaim-safe":
https://lore.kernel.org/all/20260106120621.440126-1-me@linux.beauty/)

Zhang Yi in RFC v3 review pointed out that postponing lockdep assertions only
masks the issue, and that sleeping in ext4_fc_track_inode() while holding
i_data_sem can form a real ABBA deadlock if the fast commit writer also needs
i_data_sem while the inode is in FC_COMMITTING.

Zhang Yi suggested two possible directions to address the root cause:

1. "Ha, the solution seems to have already been listed in the TODOs in
fast_commit.c.

  Change ext4_fc_commit() to lookup logical to physical mapping using extent
  status tree. This would get rid of the need to call ext4_fc_track_inode()
  before acquiring i_data_sem. To do that we would need to ensure that
  modified extents from the extent status tree are not evicted from memory."

2. "Alternatively, recording the mapped range of tracking might also be
feasible."

This series implements a hybrid way: it implements approach 2 by snapshotting inode image
and mapped ranges at commit time, and consuming only snapshots during log
writing.

Approach 2 still needs a mapping source while building the snapshot
(logical-to-physical and unwritten/hole semantics). Calling ext4_map_blocks()
there would take i_data_sem and can block inside the
jbd2_journal_lock_updates() window, which risks deadlocks or unbounded stalls.
So the snapshot path uses approach 1's extent status lookups as a best-effort
mapping source to avoid ext4_map_blocks().

I did not fully implement approach 1 (making extent status lookups
authoritative by preventing reclaim of needed entries) because that would need
additional pinning/integration under memory pressure and a larger correctness
surface. Instead, the extent status tree is treated as a cache and the
snapshot path falls back to full commit on cache misses or unstable mappings
(e.g. delayed allocation).

Lock inversion / deadlock model (before):

CPU0 (metadata update)               CPU1 (fast commit)
--------------------               -----------------
... hold i_data_sem (A)             mutex_lock(s_fc_lock) (B)
    ext4_fc_track_inode()             ext4_fc_write_inode_data()
      mutex_lock(s_fc_lock) (B)         ext4_map_blocks()
      wait FC_COMMITTING (sleep)          down_read(i_data_sem) (A)

This creates i_data_sem (A) -> s_fc_lock (B) on update paths, and
s_fc_lock (B) -> i_data_sem (A) on commit paths. Once CPU0 sleeps while
holding (A), CPU1 can block on (A) while holding (B), completing the ABBA
cycle.

New model (this series):

CPU0 (metadata update)               CPU1 (fast commit)
--------------------               -----------------
... maybe hold i_data_sem (A)        jbd2_journal_lock_updates()
    ext4_fc_track_*()                 snapshot inode + ranges (no map_blocks)
      mutex_lock(s_fc_lock) (B)       jbd2_journal_unlock_updates()
      if FC_COMMITTING: set FC_REQUEUE s_fc_lock (B)
      no sleep                         write FC log from snapshots only
                                    cleanup: clear COMMITTING, requeue if set

The commit path no longer takes i_data_sem while holding s_fc_lock, and
tracking no longer sleeps waiting for FC_COMMITTING. If an inode is updated
during a fast commit, EXT4_STATE_FC_REQUEUE records that fact and the inode
is moved to FC_Q_STAGING for the next commit.
The only remaining FC_COMMITTING waiter is ext4_fc_del(), which drops
s_fc_lock before sleeping.

This series snapshots the on-disk inode and tracked data ranges while journal
updates are locked and existing handles are drained. The log writing phase then
serializes only snapshots, so it no longer needs to call ext4_map_blocks() and
take i_data_sem under s_fc_lock. This is done in two steps: patch 1 drops
ext4_map_blocks() from log writing by introducing commit-time snapshots, and
patch 5 drops ext4_map_blocks() from the snapshot path by using the extent
status cache. The snapshot also records whether a mapped extent is unwritten,
so the ADD_RANGE records (and replay) preserve unwritten semantics.

Snapshotting runs under jbd2_journal_lock_updates(). Since a cache miss in
ext4_get_inode_loc() can start synchronous inode table I/O and stall handle
starts for milliseconds, patch 1 uses ext4_get_inode_loc_noio() and falls back
to full commit if the inode table block is not present or not uptodate.

ext4_fc_track_inode() also stops waiting for FC_COMMITTING. Updates during an
ongoing fast commit are marked with EXT4_STATE_FC_REQUEUE and are replayed in
the next fast commit, while ext4_fc_del() waits for FC_COMMITTING so an inode
cannot be removed while the commit thread is still using it.

The extent status tree is a cache, not an authoritative source, so the snapshot
path falls back to full commit on cache misses or unstable mappings (e.g.
delayed allocation). This includes cases where extent status entries are not
present (or have been reclaimed) under memory pressure. The snapshot path does
not try to rebuild mappings by calling ext4_map_blocks(); instead it simply
marks the transaction fast commit ineligible.

To keep the updates-locked window bounded, the snapshot path caps the number of
snapshotted inodes and ranges per fast commit (currently 1024 inodes and 2048
ranges) and falls back to full commit when the cap is exceeded. The series also
handles the journal inode i_data_sem lockdep false positive via subclassing;
journal inode mapping may still take i_data_sem even when data inode mapping is
avoided.

Patch 6 adds the ext4_fc_lock_updates tracepoint to quantify the updates-locked
window and snapshot fallback reasons. Patch 7 extends
/proc/fs/ext4/<sb_id>/fc_info with best-effort snapshot counters. If the /proc
interface is undesirable, I can drop patch 7 and keep the tracepoint only, or
drop even both.

Testing and measurement were done on a QEMU/KVM guest with virtio-pmem + dax
(ext4 -O fast_commit, mounted dax,noatime). The workload does python3 500x
{4K write + fsync}, fallocate 256M, and python3 500x {creat + fsync(dir)}.
Over 3 cold boots, ext4_fc_lock_updates reported locked_ns p50 2.88-2.92 us,
p99 <= 6.71 us, and max <= 102.71 us, with snap_err always 0. Under stress-ng
memory pressure (stress-ng --vm 4 --vm-bytes 75% --timeout 60s), locked_ns p50
2.94 us, p99 <= 4.97 us, and max <= 20.07 us. The fc_info snapshot failure
counters stayed at 0.
These hold times are in the low microseconds range, and the caps keep the
worst case bounded.

Comments and guidance are very welcome. Please let me know if there are any
concerns about correctness, corner cases, or better approaches.

RFC v4 -> RFC v5:
- Patch 6: Make ext4_fc_lock_updates snap_err human readable via
  TRACE_DEFINE_ENUM() + __print_symbolic(), using a single TRACE_SNAP_ERR
  mapping while keeping the enum values stable for tooling.

RFC v3 -> RFC v4:
- Replace lockdep_assert movement with removing the wait in
  ext4_fc_track_inode() and using EXT4_STATE_FC_REQUEUE to capture updates
  during an ongoing fast commit.
- Replace dropping s_fc_lock around log writing with commit-time snapshots of
  inode image and mapped ranges (recording the mapped range of tracking as
  suggested by Zhang Yi) so log writing consumes only snapshots.
- Avoid inode table I/O under jbd2_journal_lock_updates() via
  ext4_get_inode_loc_noio() and fallback to full commit on cache misses.
- Use the extent status cache for snapshot mappings and fall back to full
  commit on cache misses or unstable mappings (e.g. delayed allocation).
- Add tracepoint and /proc snapshot stats to quantify the updates-locked window
  and snapshot fallback reasons.

RFC v2 -> RFC v3:
- rebase on top of
  https://lore.kernel.org/linux-ext4/20251223131342.287864-1-me@linux.beauty/T/#u

RFC v1 -> RFC v2:
- patch 1: move comments to correct place
- patch 2: add it to patchset.
- add missing RFC prefix

RFC v1: https://lore.kernel.org/linux-ext4/20251222032655.87056-1-me@linux.beauty/T/#u
RFC v2: https://lore.kernel.org/linux-ext4/20251222151906.24607-1-me@linux.beauty/T/#t
RFC v3: https://lore.kernel.org/linux-ext4/20251224032943.134063-1-me@linux.beauty/
RFC v4: https://lore.kernel.org/all/20260120112538.132774-1-me@linux.beauty/t/#m9a6c8f2391c6dc67471e918a0577b130e7633e49

Thanks,
Li Chen (7):
  ext4: fast commit: snapshot inode state before writing log
  ext4: lockdep: handle i_data_sem subclassing for special inodes
  ext4: fast commit: avoid waiting for FC_COMMITTING
  ext4: fast commit: avoid self-deadlock in inode snapshotting
  ext4: fast commit: avoid i_data_sem by dropping ext4_map_blocks() in
    snapshots
  ext4: fast commit: add lock_updates tracepoint
  ext4: fast commit: export snapshot stats in fc_info

 fs/ext4/ext4.h              |  73 +++-
 fs/ext4/fast_commit.c       | 703 +++++++++++++++++++++++++++++-------
 fs/ext4/inode.c             |  51 +++
 fs/ext4/super.c             |   9 +
 include/trace/events/ext4.h |  61 ++++
 5 files changed, 763 insertions(+), 134 deletions(-)

-- 
2.53.0

^ permalink raw reply

* [RFC v5 1/7] ext4: fast commit: snapshot inode state before writing log
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

Fast commit writes inode metadata and data range updates after unlocking
journal updates. New handles can start at that point, so the log writing
path must not look at live inode state.

Add a commit-time per-inode snapshot and populate it while journal updates
are locked and existing handles are drained. Store the snapshot behind
ext4_inode_info->i_fc_snap so ext4_inode_info only grows by one pointer.
The snapshot contains a copy of the on-disk inode plus the data range
records needed for fast commit TLVs.

Snapshotting runs under jbd2_journal_lock_updates(). Avoid triggering I/O
there by using ext4_get_inode_loc_noio() and falling back to full commit
if the inode table block is not present or not uptodate.

Log writing then only serializes the snapshot, so it no longer needs to
call ext4_map_blocks() and take i_data_sem under s_fc_lock. The snapshot
is installed and freed under s_fc_lock and is released from fast commit
cleanup and inode eviction.

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/ext4.h        |  22 ++-
 fs/ext4/fast_commit.c | 330 +++++++++++++++++++++++++++++++++++-------
 fs/ext4/inode.c       |  51 +++++++
 3 files changed, 351 insertions(+), 52 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1524276aeac79..bd30c24d4f948 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1033,6 +1033,7 @@ enum {
 	I_DATA_SEM_EA
 };
 
+struct ext4_fc_inode_snap;
 
 /*
  * fourth extended file system inode data in memory
@@ -1089,6 +1090,22 @@ struct ext4_inode_info {
 	/* End of lblk range that needs to be committed in this fast commit */
 	ext4_lblk_t i_fc_lblk_len;
 
+	/*
+	 * Commit-time fast commit snapshots.
+	 *
+	 * i_fc_snap is installed and freed under sbi->s_fc_lock. The fast
+	 * commit log writing path reads the snapshot under sbi->s_fc_lock while
+	 * serializing fast commit TLVs.
+	 *
+	 * The snapshot lifetime is bounded by EXT4_STATE_FC_COMMITTING and the
+	 * corresponding cleanup / eviction paths.
+	 *
+	 * i_fc_snap points to per-inode snapshot data for fast commit:
+	 * - a raw inode snapshot for EXT4_FC_TAG_INODE
+	 * - data range records for EXT4_FC_TAG_{ADD,DEL}_RANGE
+	 */
+	struct ext4_fc_inode_snap *i_fc_snap;
+
 	spinlock_t i_raw_lock;	/* protects updates to the raw inode */
 
 	/* Fast commit wait queue for this inode */
@@ -3093,8 +3110,9 @@ extern int  ext4_file_getattr(struct mnt_idmap *, const struct path *,
 			      struct kstat *, u32, unsigned int);
 extern void ext4_dirty_inode(struct inode *, int);
 extern int ext4_change_inode_journal_flag(struct inode *, int);
-extern int ext4_get_inode_loc(struct inode *, struct ext4_iloc *);
-extern int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino,
+int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc);
+int ext4_get_inode_loc_noio(struct inode *inode, struct ext4_iloc *iloc);
+int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino,
 			  struct ext4_iloc *iloc);
 extern int ext4_inode_attach_jinode(struct inode *inode);
 extern int ext4_can_truncate(struct inode *inode);
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 5bd57d7f921b9..d5c28304e8181 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -55,21 +55,23 @@
  *     deleted while it is being flushed.
  * [2] Flush data buffers to disk and clear "EXT4_STATE_FC_FLUSHING_DATA"
  *     state.
- * [3] Lock the journal by calling jbd2_journal_lock_updates. This ensures that
- *     all the exsiting handles finish and no new handles can start.
- * [4] Mark all the fast commit eligible inodes as undergoing fast commit
- *     by setting "EXT4_STATE_FC_COMMITTING" state.
- * [5] Unlock the journal by calling jbd2_journal_unlock_updates. This allows
+ * [3] Lock the journal by calling jbd2_journal_lock_updates(). This ensures
+ *     that all the existing handles finish and no new handles can start.
+ * [4] Mark all the fast commit eligible inodes as undergoing fast commit by
+ *     setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state
+ *     needed for log writing.
+ * [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows
  *     starting of new handles. If new handles try to start an update on
  *     any of the inodes that are being committed, ext4_fc_track_inode()
  *     will block until those inodes have finished the fast commit.
  * [6] Commit all the directory entry updates in the fast commit space.
- * [7] Commit all the changed inodes in the fast commit space and clear
- *     "EXT4_STATE_FC_COMMITTING" for these inodes.
+ * [7] Commit all the changed inodes in the fast commit space.
  * [8] Write tail tag (this tag ensures the atomicity, please read the following
  *     section for more details).
+ * [9] Clear "EXT4_STATE_FC_COMMITTING" and wake up waiters in
+ *     ext4_fc_cleanup().
  *
- * All the inode updates must be enclosed within jbd2_jounrnal_start()
+ * All the inode updates must be enclosed within jbd2_journal_start()
  * and jbd2_journal_stop() similar to JBD2 journaling.
  *
  * Fast Commit Ineligibility
@@ -199,6 +201,8 @@ static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
 	unlock_buffer(bh);
 }
 
+static void ext4_fc_free_inode_snap(struct inode *inode);
+
 static inline void ext4_fc_reset_inode(struct inode *inode)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
@@ -215,6 +219,7 @@ void ext4_fc_init_inode(struct inode *inode)
 	ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING);
 	INIT_LIST_HEAD(&ei->i_fc_list);
 	INIT_LIST_HEAD(&ei->i_fc_dilist);
+	ei->i_fc_snap = NULL;
 	init_waitqueue_head(&ei->i_fc_wait);
 }
 
@@ -240,6 +245,7 @@ void ext4_fc_del(struct inode *inode)
 
 	alloc_ctx = ext4_fc_lock(inode->i_sb);
 	if (list_empty(&ei->i_fc_list) && list_empty(&ei->i_fc_dilist)) {
+		ext4_fc_free_inode_snap(inode);
 		ext4_fc_unlock(inode->i_sb, alloc_ctx);
 		return;
 	}
@@ -281,6 +287,7 @@ void ext4_fc_del(struct inode *inode)
 		}
 		finish_wait(wq, &wait.wq_entry);
 	}
+	ext4_fc_free_inode_snap(inode);
 	list_del_init(&ei->i_fc_list);
 
 	/*
@@ -845,6 +852,21 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
 	return true;
 }
 
+struct ext4_fc_range {
+	struct list_head list;
+	u16 tag;
+	ext4_lblk_t lblk;
+	ext4_lblk_t len;
+	ext4_fsblk_t pblk;
+	bool unwritten;
+};
+
+struct ext4_fc_inode_snap {
+	struct list_head data_list;
+	unsigned int inode_len;
+	u8 inode_buf[];
+};
+
 /*
  * Writes inode in the fast commit space under TLV with tag @tag.
  * Returns 0 on success, error on failure.
@@ -852,21 +874,21 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
 static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
-	int ret;
-	struct ext4_iloc iloc;
+	struct ext4_fc_inode_snap *snap = ei->i_fc_snap;
 	struct ext4_fc_inode fc_inode;
 	struct ext4_fc_tl tl;
 	u8 *dst;
+	u8 *src;
+	int inode_len;
+	int ret;
 
-	ret = ext4_get_inode_loc(inode, &iloc);
-	if (ret)
-		return ret;
+	if (!snap)
+		return -ECANCELED;
 
-	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
-		inode_len = EXT4_INODE_SIZE(inode->i_sb);
-	else if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE)
-		inode_len += ei->i_extra_isize;
+	src = snap->inode_buf;
+	inode_len = snap->inode_len;
+	if (!src || inode_len == 0)
+		return -ECANCELED;
 
 	fc_inode.fc_ino = cpu_to_le32(inode->i_ino);
 	tl.fc_tag = cpu_to_le16(EXT4_FC_TAG_INODE);
@@ -882,10 +904,9 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
 	dst += EXT4_FC_TAG_BASE_LEN;
 	memcpy(dst, &fc_inode, sizeof(fc_inode));
 	dst += sizeof(fc_inode);
-	memcpy(dst, (u8 *)ext4_raw_inode(&iloc), inode_len);
+	memcpy(dst, src, inode_len);
 	ret = 0;
 err:
-	brelse(iloc.bh);
 	return ret;
 }
 
@@ -895,12 +916,74 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
  */
 static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 {
-	ext4_lblk_t old_blk_size, cur_lblk_off, new_blk_size;
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	struct ext4_map_blocks map;
+	struct ext4_fc_inode_snap *snap = ei->i_fc_snap;
 	struct ext4_fc_add_range fc_ext;
 	struct ext4_fc_del_range lrange;
 	struct ext4_extent *ex;
+	struct ext4_fc_range *range;
+
+	if (!snap)
+		return -ECANCELED;
+
+	list_for_each_entry(range, &snap->data_list, list) {
+		if (range->tag == EXT4_FC_TAG_DEL_RANGE) {
+			lrange.fc_ino = cpu_to_le32(inode->i_ino);
+			lrange.fc_lblk = cpu_to_le32(range->lblk);
+			lrange.fc_len = cpu_to_le32(range->len);
+			if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_DEL_RANGE,
+					     sizeof(lrange), (u8 *)&lrange, crc))
+				return -ENOSPC;
+			continue;
+		}
+
+		fc_ext.fc_ino = cpu_to_le32(inode->i_ino);
+		ex = (struct ext4_extent *)&fc_ext.fc_ex;
+		ex->ee_block = cpu_to_le32(range->lblk);
+		ex->ee_len = cpu_to_le16(range->len);
+		ext4_ext_store_pblock(ex, range->pblk);
+		if (range->unwritten)
+			ext4_ext_mark_unwritten(ex);
+		else
+			ext4_ext_mark_initialized(ex);
+
+		if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_ADD_RANGE,
+				     sizeof(fc_ext), (u8 *)&fc_ext, crc))
+			return -ENOSPC;
+	}
+
+	return 0;
+}
+
+static void ext4_fc_free_ranges(struct list_head *head)
+{
+	struct ext4_fc_range *range, *range_n;
+
+	list_for_each_entry_safe(range, range_n, head, list) {
+		list_del(&range->list);
+		kfree(range);
+	}
+}
+
+static void ext4_fc_free_inode_snap(struct inode *inode)
+{
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_fc_inode_snap *snap = ei->i_fc_snap;
+
+	if (!snap)
+		return;
+
+	ext4_fc_free_ranges(&snap->data_list);
+	kfree(snap);
+	ei->i_fc_snap = NULL;
+}
+
+static int ext4_fc_snapshot_inode_data(struct inode *inode,
+				       struct list_head *ranges)
+{
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	ext4_lblk_t start_lblk, end_lblk, cur_lblk;
+	struct ext4_map_blocks map;
 	int ret;
 
 	spin_lock(&ei->i_fc_lock);
@@ -908,18 +991,20 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 		spin_unlock(&ei->i_fc_lock);
 		return 0;
 	}
-	old_blk_size = ei->i_fc_lblk_start;
-	new_blk_size = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1;
+	start_lblk = ei->i_fc_lblk_start;
+	end_lblk = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1;
 	ei->i_fc_lblk_len = 0;
 	spin_unlock(&ei->i_fc_lock);
 
-	cur_lblk_off = old_blk_size;
-	ext4_debug("will try writing %d to %d for inode %ld\n",
-		   cur_lblk_off, new_blk_size, inode->i_ino);
+	cur_lblk = start_lblk;
+	ext4_debug("snapshot data ranges %u-%u for inode %lu\n",
+		   start_lblk, end_lblk, inode->i_ino);
+
+	while (cur_lblk <= end_lblk) {
+		struct ext4_fc_range *range;
 
-	while (cur_lblk_off <= new_blk_size) {
-		map.m_lblk = cur_lblk_off;
-		map.m_len = new_blk_size - cur_lblk_off + 1;
+		map.m_lblk = cur_lblk;
+		map.m_len = end_lblk - cur_lblk + 1;
 		ret = ext4_map_blocks(NULL, inode, &map,
 				      EXT4_GET_BLOCKS_IO_SUBMIT |
 				      EXT4_EX_NOCACHE);
@@ -927,17 +1012,21 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 			return -ECANCELED;
 
 		if (map.m_len == 0) {
-			cur_lblk_off++;
+			cur_lblk++;
 			continue;
 		}
 
+		range = kmalloc(sizeof(*range), GFP_NOFS);
+		if (!range)
+			return -ENOMEM;
+
+		range->lblk = map.m_lblk;
+		range->len = map.m_len;
+		range->pblk = 0;
+		range->unwritten = false;
+
 		if (ret == 0) {
-			lrange.fc_ino = cpu_to_le32(inode->i_ino);
-			lrange.fc_lblk = cpu_to_le32(map.m_lblk);
-			lrange.fc_len = cpu_to_le32(map.m_len);
-			if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_DEL_RANGE,
-					    sizeof(lrange), (u8 *)&lrange, crc))
-				return -ENOSPC;
+			range->tag = EXT4_FC_TAG_DEL_RANGE;
 		} else {
 			unsigned int max = (map.m_flags & EXT4_MAP_UNWRITTEN) ?
 				EXT_UNWRITTEN_MAX_LEN : EXT_INIT_MAX_LEN;
@@ -945,26 +1034,67 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 			/* Limit the number of blocks in one extent */
 			map.m_len = min(max, map.m_len);
 
-			fc_ext.fc_ino = cpu_to_le32(inode->i_ino);
-			ex = (struct ext4_extent *)&fc_ext.fc_ex;
-			ex->ee_block = cpu_to_le32(map.m_lblk);
-			ex->ee_len = cpu_to_le16(map.m_len);
-			ext4_ext_store_pblock(ex, map.m_pblk);
-			if (map.m_flags & EXT4_MAP_UNWRITTEN)
-				ext4_ext_mark_unwritten(ex);
-			else
-				ext4_ext_mark_initialized(ex);
-			if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_ADD_RANGE,
-					    sizeof(fc_ext), (u8 *)&fc_ext, crc))
-				return -ENOSPC;
+			range->tag = EXT4_FC_TAG_ADD_RANGE;
+			range->len = map.m_len;
+			range->pblk = map.m_pblk;
+			range->unwritten = !!(map.m_flags & EXT4_MAP_UNWRITTEN);
 		}
 
-		cur_lblk_off += map.m_len;
+		INIT_LIST_HEAD(&range->list);
+		list_add_tail(&range->list, ranges);
+
+		cur_lblk += map.m_len;
 	}
 
 	return 0;
 }
 
+static int ext4_fc_snapshot_inode(struct inode *inode)
+{
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_fc_inode_snap *snap;
+	int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
+	struct ext4_iloc iloc;
+	LIST_HEAD(ranges);
+	int ret;
+	int alloc_ctx;
+
+	ret = ext4_get_inode_loc_noio(inode, &iloc);
+	if (ret)
+		return ret;
+
+	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
+		inode_len = EXT4_INODE_SIZE(inode->i_sb);
+	else if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE)
+		inode_len += ei->i_extra_isize;
+
+	snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS);
+	if (!snap) {
+		brelse(iloc.bh);
+		return -ENOMEM;
+	}
+	INIT_LIST_HEAD(&snap->data_list);
+	snap->inode_len = inode_len;
+
+	memcpy(snap->inode_buf, (u8 *)ext4_raw_inode(&iloc), inode_len);
+	brelse(iloc.bh);
+
+	ret = ext4_fc_snapshot_inode_data(inode, &ranges);
+	if (ret) {
+		kfree(snap);
+		ext4_fc_free_ranges(&ranges);
+		return ret;
+	}
+
+	alloc_ctx = ext4_fc_lock(inode->i_sb);
+	ext4_fc_free_inode_snap(inode);
+	ei->i_fc_snap = snap;
+	list_splice_tail_init(&ranges, &snap->data_list);
+	ext4_fc_unlock(inode->i_sb, alloc_ctx);
+
+	return 0;
+}
+
 
 /* Flushes data of all the inodes in the commit queue. */
 static int ext4_fc_flush_data(journal_t *journal)
@@ -1015,6 +1145,11 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
 		 */
 		if (list_empty(&fc_dentry->fcd_dilist))
 			continue;
+		/*
+		 * For EXT4_FC_TAG_CREAT, fcd_dilist is linked on the created
+		 * inode's i_fc_dilist list (kept singular), so we can recover the
+		 * inode through it.
+		 */
 		ei = list_first_entry(&fc_dentry->fcd_dilist,
 				struct ext4_inode_info, i_fc_dilist);
 		inode = &ei->vfs_inode;
@@ -1039,6 +1174,88 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
 	return 0;
 }
 
+static int ext4_fc_snapshot_inodes(journal_t *journal)
+{
+	struct super_block *sb = journal->j_private;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_inode_info *iter;
+	struct ext4_fc_dentry_update *fc_dentry;
+	struct inode **inodes;
+	unsigned int nr_inodes = 0;
+	unsigned int i = 0;
+	int ret = 0;
+	int alloc_ctx;
+
+	alloc_ctx = ext4_fc_lock(sb);
+	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list)
+		nr_inodes++;
+
+	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
+		struct ext4_inode_info *ei;
+
+		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
+			continue;
+		if (list_empty(&fc_dentry->fcd_dilist))
+			continue;
+
+		/* See the comment in ext4_fc_commit_dentry_updates(). */
+		ei = list_first_entry(&fc_dentry->fcd_dilist,
+				      struct ext4_inode_info, i_fc_dilist);
+		if (!list_empty(&ei->i_fc_list))
+			continue;
+
+		nr_inodes++;
+	}
+	ext4_fc_unlock(sb, alloc_ctx);
+
+	if (!nr_inodes)
+		return 0;
+
+	inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS);
+	if (!inodes)
+		return -ENOMEM;
+
+	alloc_ctx = ext4_fc_lock(sb);
+	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
+		inodes[i] = igrab(&iter->vfs_inode);
+		if (inodes[i])
+			i++;
+	}
+
+	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
+		struct ext4_inode_info *ei;
+
+		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
+			continue;
+		if (list_empty(&fc_dentry->fcd_dilist))
+			continue;
+
+		/* See the comment in ext4_fc_commit_dentry_updates(). */
+		ei = list_first_entry(&fc_dentry->fcd_dilist,
+				      struct ext4_inode_info, i_fc_dilist);
+		if (!list_empty(&ei->i_fc_list))
+			continue;
+
+		inodes[i] = igrab(&ei->vfs_inode);
+		if (inodes[i])
+			i++;
+	}
+	ext4_fc_unlock(sb, alloc_ctx);
+
+	for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
+		ret = ext4_fc_snapshot_inode(inodes[nr_inodes]);
+		if (ret)
+			break;
+	}
+
+	for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
+		if (inodes[nr_inodes])
+			iput(inodes[nr_inodes]);
+	}
+	kvfree(inodes);
+	return ret;
+}
+
 static int ext4_fc_perform_commit(journal_t *journal)
 {
 	struct super_block *sb = journal->j_private;
@@ -1111,7 +1328,11 @@ static int ext4_fc_perform_commit(journal_t *journal)
 				     EXT4_STATE_FC_COMMITTING);
 	}
 	ext4_fc_unlock(sb, alloc_ctx);
+
+	ret = ext4_fc_snapshot_inodes(journal);
 	jbd2_journal_unlock_updates(journal);
+	if (ret)
+		return ret;
 
 	/*
 	 * Step 5: If file system device is different from journal device,
@@ -1308,6 +1529,7 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 					struct ext4_inode_info,
 					i_fc_list);
 		list_del_init(&ei->i_fc_list);
+		ext4_fc_free_inode_snap(&ei->vfs_inode);
 		ext4_clear_inode_state(&ei->vfs_inode,
 				       EXT4_STATE_FC_COMMITTING);
 		if (tid_geq(tid, ei->i_sync_tid)) {
@@ -1343,6 +1565,14 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 					     struct ext4_fc_dentry_update,
 					     fcd_list);
 		list_del_init(&fc_dentry->fcd_list);
+		if (fc_dentry->fcd_op == EXT4_FC_TAG_CREAT &&
+		    !list_empty(&fc_dentry->fcd_dilist)) {
+			/* See the comment in ext4_fc_commit_dentry_updates(). */
+			ei = list_first_entry(&fc_dentry->fcd_dilist,
+					      struct ext4_inode_info,
+					      i_fc_dilist);
+			ext4_fc_free_inode_snap(&ei->vfs_inode);
+		}
 		list_del_init(&fc_dentry->fcd_dilist);
 
 		release_dentry_name_snapshot(&fc_dentry->fcd_name);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index a1c81ffdca2b9..385ff112d405e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4969,6 +4969,57 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc)
 	return ret;
 }
 
+/*
+ * ext4_get_inode_loc_noio() is a best-effort variant of ext4_get_inode_loc().
+ * It looks up the inode table block in the buffer cache and returns -EAGAIN if
+ * the block is not present or not uptodate, without starting any I/O.
+ */
+int ext4_get_inode_loc_noio(struct inode *inode, struct ext4_iloc *iloc)
+{
+	struct super_block *sb = inode->i_sb;
+	struct ext4_group_desc *gdp;
+	struct buffer_head *bh;
+	ext4_fsblk_t block;
+	int inodes_per_block, inode_offset;
+	unsigned long ino = inode->i_ino;
+
+	iloc->bh = NULL;
+	if (ino < EXT4_ROOT_INO ||
+	    ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count))
+		return -EFSCORRUPTED;
+
+	iloc->block_group = (ino - 1) / EXT4_INODES_PER_GROUP(sb);
+	gdp = ext4_get_group_desc(sb, iloc->block_group, NULL);
+	if (!gdp)
+		return -EIO;
+
+	/* Figure out the offset within the block group inode table. */
+	inodes_per_block = EXT4_SB(sb)->s_inodes_per_block;
+	inode_offset = ((ino - 1) % EXT4_INODES_PER_GROUP(sb));
+	iloc->offset = (inode_offset % inodes_per_block) * EXT4_INODE_SIZE(sb);
+
+	block = ext4_inode_table(sb, gdp);
+	if (block <= le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block) ||
+	    block >= ext4_blocks_count(EXT4_SB(sb)->s_es)) {
+		ext4_error(sb,
+			   "Invalid inode table block %llu in block_group %u",
+			   block, iloc->block_group);
+		return -EFSCORRUPTED;
+	}
+	block += inode_offset / inodes_per_block;
+
+	bh = sb_find_get_block(sb, block);
+	if (!bh)
+		return -EAGAIN;
+	if (!ext4_buffer_uptodate(bh)) {
+		brelse(bh);
+		return -EAGAIN;
+	}
+
+	iloc->bh = bh;
+	return 0;
+}
+
 
 int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino,
 			  struct ext4_iloc *iloc)
-- 
2.53.0


^ permalink raw reply related

* [RFC v5 2/7] ext4: lockdep: handle i_data_sem subclassing for special inodes
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

Fast commit can hold s_fc_lock while writing journal blocks. Mapping the
journal inode can take its i_data_sem. Normal inode update paths can take a
data inode i_data_sem and then s_fc_lock, which makes lockdep report a
circular dependency.

lockdep treats all i_data_sem instances as one lock class and cannot
distinguish the journal inode i_data_sem from a regular inode i_data_sem.
The journal inode is not tracked by fast commit and no FC waiters ever
depend on it, so this is not a real ABBA deadlock. Assign the journal inode
a dedicated i_data_sem lockdep subclass to avoid the false positive.

Inode cache objects can be recycled, so also reset i_data_sem to
I_DATA_SEM_NORMAL when allocating an ext4 inode. Otherwise a new inode may
inherit an old subclass (journal/quota/ea) and trigger lockdep warnings.

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/ext4.h  | 4 +++-
 fs/ext4/super.c | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bd30c24d4f948..2e1681057196a 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1025,12 +1025,14 @@ do {										\
  *			  than the first
  *  I_DATA_SEM_QUOTA  - Used for quota inodes only
  *  I_DATA_SEM_EA     - Used for ea_inodes only
+ *  I_DATA_SEM_JOURNAL - Used for journal inode only
  */
 enum {
 	I_DATA_SEM_NORMAL = 0,
 	I_DATA_SEM_OTHER,
 	I_DATA_SEM_QUOTA,
-	I_DATA_SEM_EA
+	I_DATA_SEM_EA,
+	I_DATA_SEM_JOURNAL
 };
 
 struct ext4_fc_inode_snap;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 79762c3e0dff3..4f5f0c21d436f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1423,6 +1423,9 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
 	INIT_WORK(&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);
 	ext4_fc_init_inode(&ei->vfs_inode);
 	spin_lock_init(&ei->i_fc_lock);
+#ifdef CONFIG_LOCKDEP
+	lockdep_set_subclass(&ei->i_data_sem, I_DATA_SEM_NORMAL);
+#endif
 	return &ei->vfs_inode;
 }
 
@@ -5863,6 +5866,11 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
 		return ERR_PTR(-EFSCORRUPTED);
 	}
 
+#ifdef CONFIG_LOCKDEP
+	lockdep_set_subclass(&EXT4_I(journal_inode)->i_data_sem,
+			     I_DATA_SEM_JOURNAL);
+#endif
+
 	ext4_debug("Journal inode found at %p: %lld bytes\n",
 		  journal_inode, journal_inode->i_size);
 	return journal_inode;
-- 
2.53.0


^ permalink raw reply related

* [RFC v5 3/7] ext4: fast commit: avoid waiting for FC_COMMITTING
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

ext4_fc_track_inode() can be called while holding i_data_sem (e.g.
fallocate). Waiting for EXT4_STATE_FC_COMMITTING in that case risks an
ABBA deadlock: i_data_sem -> wait(FC_COMMITTING) vs FC_COMMITTING ->
wait(i_data_sem) in the commit task.

Now that fast commit snapshots inode state at commit time, updates during
log writing do not need to block. Drop the wait and lockdep assertion in
ext4_fc_track_inode(), and make ext4_fc_del() wait for FC_COMMITTING so an
inode cannot be removed while the commit thread is still using it.

When an inode is modified during a fast commit, mark it with
EXT4_STATE_FC_REQUEUE so cleanup keeps it queued for the next fast commit.
This is needed because jbd2_fc_end_commit() invokes the cleanup callback
with tid == 0, so tid-based requeue logic would requeue every inode.

Testing: tracepoint ext4:ext4_fc_commit_stop with two fsyncs in the same
transaction. nblks is the number of journal blocks written for that fast
commit. Before this change, the second fsync still wrote almost the same
fast commit log (nblks 10->9), because tid == 0 in jbd2_fc_end_commit()
caused the tid-based requeue logic to keep all inodes queued. After this
change, only inodes modified during the commit are requeued, and the
second fsync wrote a nearly empty fast commit (nblks 10->1).

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/ext4.h        |   1 +
 fs/ext4/fast_commit.c | 111 ++++++++++++++++++++----------------------
 2 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 2e1681057196a..68a64fa0be926 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2004,6 +2004,7 @@ enum {
 	EXT4_STATE_FC_COMMITTING,	/* Fast commit ongoing */
 	EXT4_STATE_FC_FLUSHING_DATA,	/* Fast commit flushing data */
 	EXT4_STATE_ORPHAN_FILE,		/* Inode orphaned in orphan file */
+	EXT4_STATE_FC_REQUEUE,		/* Inode modified during fast commit */
 };
 
 #define EXT4_INODE_BIT_FNS(name, field, offset)				\
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index d5c28304e8181..809170d46167b 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -61,9 +61,8 @@
  *     setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state
  *     needed for log writing.
  * [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows
- *     starting of new handles. If new handles try to start an update on
- *     any of the inodes that are being committed, ext4_fc_track_inode()
- *     will block until those inodes have finished the fast commit.
+ *     starting of new handles. Updates to inodes being fast committed are
+ *     tracked for requeue rather than blocking.
  * [6] Commit all the directory entry updates in the fast commit space.
  * [7] Commit all the changed inodes in the fast commit space.
  * [8] Write tail tag (this tag ensures the atomicity, please read the following
@@ -217,6 +216,7 @@ void ext4_fc_init_inode(struct inode *inode)
 
 	ext4_fc_reset_inode(inode);
 	ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING);
+	ext4_clear_inode_state(inode, EXT4_STATE_FC_REQUEUE);
 	INIT_LIST_HEAD(&ei->i_fc_list);
 	INIT_LIST_HEAD(&ei->i_fc_dilist);
 	ei->i_fc_snap = NULL;
@@ -251,22 +251,30 @@ void ext4_fc_del(struct inode *inode)
 	}
 
 	/*
-	 * Since ext4_fc_del is called from ext4_evict_inode while having a
-	 * handle open, there is no need for us to wait here even if a fast
-	 * commit is going on. That is because, if this inode is being
-	 * committed, ext4_mark_inode_dirty would have waited for inode commit
-	 * operation to finish before we come here. So, by the time we come
-	 * here, inode's EXT4_STATE_FC_COMMITTING would have been cleared. So,
-	 * we shouldn't see EXT4_STATE_FC_COMMITTING to be set on this inode
-	 * here.
-	 *
-	 * We may come here without any handles open in the "no_delete" case of
-	 * ext4_evict_inode as well. However, if that happens, we first mark the
-	 * file system as fast commit ineligible anyway. So, even in that case,
-	 * it is okay to remove the inode from the fc list.
+	 * Wait for ongoing fast commit to finish. We cannot remove the inode
+	 * from fast commit lists while it is being committed.
 	 */
-	WARN_ON(ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)
-		&& !ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE));
+	while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
+#if (BITS_PER_LONG < 64)
+		DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
+				EXT4_STATE_FC_COMMITTING);
+		wq = bit_waitqueue(&ei->i_state_flags,
+				   EXT4_STATE_FC_COMMITTING);
+#else
+		DEFINE_WAIT_BIT(wait, &ei->i_flags,
+				EXT4_STATE_FC_COMMITTING);
+		wq = bit_waitqueue(&ei->i_flags,
+				   EXT4_STATE_FC_COMMITTING);
+#endif
+		prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
+		if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
+			ext4_fc_unlock(inode->i_sb, alloc_ctx);
+			schedule();
+			alloc_ctx = ext4_fc_lock(inode->i_sb);
+		}
+		finish_wait(wq, &wait.wq_entry);
+	}
+
 	while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) {
 #if (BITS_PER_LONG < 64)
 		DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
@@ -287,19 +295,22 @@ void ext4_fc_del(struct inode *inode)
 		}
 		finish_wait(wq, &wait.wq_entry);
 	}
+
 	ext4_fc_free_inode_snap(inode);
 	list_del_init(&ei->i_fc_list);
 
 	/*
-	 * Since this inode is getting removed, let's also remove all FC
-	 * dentry create references, since it is not needed to log it anyways.
+	 * Since this inode is getting removed, let's also remove all FC dentry
+	 * create references, since it is not needed to log it anyways.
 	 */
 	if (list_empty(&ei->i_fc_dilist)) {
 		ext4_fc_unlock(inode->i_sb, alloc_ctx);
 		return;
 	}
 
-	fc_dentry = list_first_entry(&ei->i_fc_dilist, struct ext4_fc_dentry_update, fcd_dilist);
+	fc_dentry = list_first_entry(&ei->i_fc_dilist,
+				     struct ext4_fc_dentry_update,
+				     fcd_dilist);
 	WARN_ON(fc_dentry->fcd_op != EXT4_FC_TAG_CREAT);
 	list_del_init(&fc_dentry->fcd_list);
 	list_del_init(&fc_dentry->fcd_dilist);
@@ -371,6 +382,8 @@ static int ext4_fc_track_template(
 
 	tid = handle->h_transaction->t_tid;
 	spin_lock(&ei->i_fc_lock);
+	if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
+		ext4_set_inode_state(inode, EXT4_STATE_FC_REQUEUE);
 	if (tid == ei->i_sync_tid) {
 		update = true;
 	} else {
@@ -557,8 +570,6 @@ static int __track_inode(handle_t *handle, struct inode *inode, void *arg,
 
 void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
 {
-	struct ext4_inode_info *ei = EXT4_I(inode);
-	wait_queue_head_t *wq;
 	int ret;
 
 	if (S_ISDIR(inode->i_mode))
@@ -577,29 +588,11 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
 		return;
 
 	/*
-	 * If we come here, we may sleep while waiting for the inode to
-	 * commit. We shouldn't be holding i_data_sem when we go to sleep since
-	 * the commit path needs to grab the lock while committing the inode.
+	 * Fast commit snapshots inode state at commit time, so there's no need
+	 * to wait for EXT4_STATE_FC_COMMITTING here. If the inode is already
+	 * on the commit queue, ext4_fc_cleanup() will requeue it for the new
+	 * transaction once the current commit finishes.
 	 */
-	lockdep_assert_not_held(&ei->i_data_sem);
-
-	while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
-#if (BITS_PER_LONG < 64)
-		DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
-				EXT4_STATE_FC_COMMITTING);
-		wq = bit_waitqueue(&ei->i_state_flags,
-				   EXT4_STATE_FC_COMMITTING);
-#else
-		DEFINE_WAIT_BIT(wait, &ei->i_flags,
-				EXT4_STATE_FC_COMMITTING);
-		wq = bit_waitqueue(&ei->i_flags,
-				   EXT4_STATE_FC_COMMITTING);
-#endif
-		prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
-		if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
-			schedule();
-		finish_wait(wq, &wait.wq_entry);
-	}
 
 	/*
 	 * From this point on, this inode will not be committed either
@@ -1525,32 +1518,32 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 
 	alloc_ctx = ext4_fc_lock(sb);
 	while (!list_empty(&sbi->s_fc_q[FC_Q_MAIN])) {
+		bool requeue;
+
 		ei = list_first_entry(&sbi->s_fc_q[FC_Q_MAIN],
 					struct ext4_inode_info,
 					i_fc_list);
 		list_del_init(&ei->i_fc_list);
 		ext4_fc_free_inode_snap(&ei->vfs_inode);
+		spin_lock(&ei->i_fc_lock);
+		if (full)
+			requeue = !tid_geq(tid, ei->i_sync_tid);
+		else
+			requeue = ext4_test_inode_state(&ei->vfs_inode,
+							EXT4_STATE_FC_REQUEUE);
+		if (!requeue)
+			ext4_fc_reset_inode(&ei->vfs_inode);
+		ext4_clear_inode_state(&ei->vfs_inode, EXT4_STATE_FC_REQUEUE);
 		ext4_clear_inode_state(&ei->vfs_inode,
 				       EXT4_STATE_FC_COMMITTING);
-		if (tid_geq(tid, ei->i_sync_tid)) {
-			ext4_fc_reset_inode(&ei->vfs_inode);
-		} else if (full) {
-			/*
-			 * We are called after a full commit, inode has been
-			 * modified while the commit was running. Re-enqueue
-			 * the inode into STAGING, which will then be splice
-			 * back into MAIN. This cannot happen during
-			 * fastcommit because the journal is locked all the
-			 * time in that case (and tid doesn't increase so
-			 * tid check above isn't reliable).
-			 */
+		spin_unlock(&ei->i_fc_lock);
+		if (requeue)
 			list_add_tail(&ei->i_fc_list,
 				      &sbi->s_fc_q[FC_Q_STAGING]);
-		}
 		/*
 		 * Make sure clearing of EXT4_STATE_FC_COMMITTING is
 		 * visible before we send the wakeup. Pairs with implicit
-		 * barrier in prepare_to_wait() in ext4_fc_track_inode().
+		 * barrier in prepare_to_wait() in ext4_fc_del().
 		 */
 		smp_mb();
 #if (BITS_PER_LONG < 64)
-- 
2.53.0


^ permalink raw reply related

* [RFC v5 4/7] ext4: fast commit: avoid self-deadlock in inode snapshotting
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

ext4_fc_snapshot_inodes() used igrab()/iput() to pin inodes while building
commit-time snapshots. With ext4_fc_del() waiting for
EXT4_STATE_FC_COMMITTING, iput() can trigger
ext4_clear_inode()->ext4_fc_del() in the commit thread and deadlock waiting
for the fast commit to finish.

Avoid taking extra references. Collect inode pointers under s_fc_lock and
rely on EXT4_STATE_FC_COMMITTING to pin inodes until ext4_fc_cleanup()
clears the bit.

Also set EXT4_STATE_FC_COMMITTING for create-only inodes referenced
from the dentry update queue, and wake up waiters when ext4_fc_cleanup()
clears the bit.

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/fast_commit.c | 47 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 809170d46167b..966211a3342a0 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1210,13 +1210,12 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
 
 	alloc_ctx = ext4_fc_lock(sb);
 	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
-		inodes[i] = igrab(&iter->vfs_inode);
-		if (inodes[i])
-			i++;
+		inodes[i++] = &iter->vfs_inode;
 	}
 
 	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
 		struct ext4_inode_info *ei;
+		struct inode *inode;
 
 		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
 			continue;
@@ -1226,12 +1225,20 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
 		/* See the comment in ext4_fc_commit_dentry_updates(). */
 		ei = list_first_entry(&fc_dentry->fcd_dilist,
 				      struct ext4_inode_info, i_fc_dilist);
+		inode = &ei->vfs_inode;
 		if (!list_empty(&ei->i_fc_list))
 			continue;
 
-		inodes[i] = igrab(&ei->vfs_inode);
-		if (inodes[i])
-			i++;
+		/*
+		 * Create-only inodes may only be referenced via fcd_dilist and
+		 * not appear on s_fc_q[MAIN]. They may hit the last iput while
+		 * we are snapshotting, but inode eviction calls ext4_fc_del(),
+		 * which waits for FC_COMMITTING to clear. Mark them FC_COMMITTING
+		 * so the inode stays pinned and the snapshot stays valid until
+		 * ext4_fc_cleanup().
+		 */
+		ext4_set_inode_state(inode, EXT4_STATE_FC_COMMITTING);
+		inodes[i++] = inode;
 	}
 	ext4_fc_unlock(sb, alloc_ctx);
 
@@ -1241,10 +1248,6 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
 			break;
 	}
 
-	for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
-		if (inodes[nr_inodes])
-			iput(inodes[nr_inodes]);
-	}
 	kvfree(inodes);
 	return ret;
 }
@@ -1312,8 +1315,9 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	jbd2_journal_lock_updates(journal);
 	/*
 	 * The journal is now locked. No more handles can start and all the
-	 * previous handles are now drained. We now mark the inodes on the
-	 * commit queue as being committed.
+	 * previous handles are now drained. Snapshotting happens in this
+	 * window so log writing can consume only stable snapshots without
+	 * doing logical-to-physical mapping.
 	 */
 	alloc_ctx = ext4_fc_lock(sb);
 	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
@@ -1565,6 +1569,25 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 					      struct ext4_inode_info,
 					      i_fc_dilist);
 			ext4_fc_free_inode_snap(&ei->vfs_inode);
+			spin_lock(&ei->i_fc_lock);
+			ext4_clear_inode_state(&ei->vfs_inode,
+					       EXT4_STATE_FC_REQUEUE);
+			ext4_clear_inode_state(&ei->vfs_inode,
+					       EXT4_STATE_FC_COMMITTING);
+			spin_unlock(&ei->i_fc_lock);
+			/*
+			 * Make sure clearing of EXT4_STATE_FC_COMMITTING is
+			 * visible before we send the wakeup. Pairs with implicit
+			 * barrier in prepare_to_wait() in ext4_fc_del().
+			 */
+			smp_mb();
+#if (BITS_PER_LONG < 64)
+			wake_up_bit(&ei->i_state_flags,
+				    EXT4_STATE_FC_COMMITTING);
+#else
+			wake_up_bit(&ei->i_flags,
+				    EXT4_STATE_FC_COMMITTING);
+#endif
 		}
 		list_del_init(&fc_dentry->fcd_dilist);
 
-- 
2.53.0


^ permalink raw reply related

* [RFC v5 5/7] ext4: fast commit: avoid i_data_sem by dropping ext4_map_blocks() in snapshots
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

Commit-time snapshots run under jbd2_journal_lock_updates(), so the work
done there must stay bounded.

The snapshot path still used ext4_map_blocks() to build data ranges. This
can take i_data_sem and pulls the mapping code into the snapshot logic.
Build inode data range snapshots from the extent status tree instead.

The extent status tree is a cache, not an authoritative source. If the
needed information is missing or unstable (e.g. delayed allocation), treat
the transaction as fast commit ineligible and fall back to full commit.

Also cap the number of inodes and ranges snapshotted per fast commit and
allocate range records from a dedicated slab cache. The inode pointer
array is allocated outside the updates-locked window.

Testing: QEMU/KVM guest, virtio-pmem + dax, ext4 -O fast_commit, mounted
dax,noatime. Ran python3 500x {4K write + fsync}, fallocate 256M, and
python3 500x {creat + fsync(dir)} without lockdep splats or errors.

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/fast_commit.c | 253 +++++++++++++++++++++++++++++-------------
 1 file changed, 177 insertions(+), 76 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 966211a3342a0..d1eefee609120 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -183,6 +183,15 @@
 
 #include <trace/events/ext4.h>
 static struct kmem_cache *ext4_fc_dentry_cachep;
+static struct kmem_cache *ext4_fc_range_cachep;
+
+/*
+ * Avoid spending unbounded time/memory snapshotting highly fragmented files
+ * under jbd2_journal_lock_updates(). If we exceed this limit, fall back to
+ * full commit.
+ */
+#define EXT4_FC_SNAPSHOT_MAX_INODES	1024
+#define EXT4_FC_SNAPSHOT_MAX_RANGES	2048
 
 static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
 {
@@ -954,7 +963,7 @@ static void ext4_fc_free_ranges(struct list_head *head)
 
 	list_for_each_entry_safe(range, range_n, head, list) {
 		list_del(&range->list);
-		kfree(range);
+		kmem_cache_free(ext4_fc_range_cachep, range);
 	}
 }
 
@@ -972,16 +981,19 @@ static void ext4_fc_free_inode_snap(struct inode *inode)
 }
 
 static int ext4_fc_snapshot_inode_data(struct inode *inode,
-				       struct list_head *ranges)
+				       struct list_head *ranges,
+				       unsigned int nr_ranges_total,
+				       unsigned int *nr_rangesp)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
+	unsigned int nr_ranges = 0;
 	ext4_lblk_t start_lblk, end_lblk, cur_lblk;
-	struct ext4_map_blocks map;
-	int ret;
 
 	spin_lock(&ei->i_fc_lock);
 	if (ei->i_fc_lblk_len == 0) {
 		spin_unlock(&ei->i_fc_lock);
+		if (nr_rangesp)
+			*nr_rangesp = 0;
 		return 0;
 	}
 	start_lblk = ei->i_fc_lblk_start;
@@ -994,61 +1006,78 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 		   start_lblk, end_lblk, inode->i_ino);
 
 	while (cur_lblk <= end_lblk) {
+		struct extent_status es;
 		struct ext4_fc_range *range;
+		ext4_lblk_t len;
 
-		map.m_lblk = cur_lblk;
-		map.m_len = end_lblk - cur_lblk + 1;
-		ret = ext4_map_blocks(NULL, inode, &map,
-				      EXT4_GET_BLOCKS_IO_SUBMIT |
-				      EXT4_EX_NOCACHE);
-		if (ret < 0)
-			return -ECANCELED;
+		if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL))
+			return -EAGAIN;
+
+		if (ext4_es_is_delayed(&es))
+			return -EAGAIN;
 
-		if (map.m_len == 0) {
+		len = es.es_len - (cur_lblk - es.es_lblk);
+		if (len > end_lblk - cur_lblk + 1)
+			len = end_lblk - cur_lblk + 1;
+		if (len == 0) {
 			cur_lblk++;
 			continue;
 		}
 
-		range = kmalloc(sizeof(*range), GFP_NOFS);
+		if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES)
+			return -E2BIG;
+
+		range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS);
 		if (!range)
 			return -ENOMEM;
+		nr_ranges++;
 
-		range->lblk = map.m_lblk;
-		range->len = map.m_len;
+		range->lblk = cur_lblk;
+		range->len = len;
 		range->pblk = 0;
 		range->unwritten = false;
 
-		if (ret == 0) {
+		if (ext4_es_is_hole(&es)) {
 			range->tag = EXT4_FC_TAG_DEL_RANGE;
-		} else {
-			unsigned int max = (map.m_flags & EXT4_MAP_UNWRITTEN) ?
-				EXT_UNWRITTEN_MAX_LEN : EXT_INIT_MAX_LEN;
-
-			/* Limit the number of blocks in one extent */
-			map.m_len = min(max, map.m_len);
+		} else if (ext4_es_is_written(&es) ||
+			   ext4_es_is_unwritten(&es)) {
+			unsigned int max;
 
 			range->tag = EXT4_FC_TAG_ADD_RANGE;
-			range->len = map.m_len;
-			range->pblk = map.m_pblk;
-			range->unwritten = !!(map.m_flags & EXT4_MAP_UNWRITTEN);
+			range->pblk = ext4_es_pblock(&es) +
+				      (cur_lblk - es.es_lblk);
+			range->unwritten = ext4_es_is_unwritten(&es);
+
+			max = range->unwritten ? EXT_UNWRITTEN_MAX_LEN :
+						 EXT_INIT_MAX_LEN;
+			if (range->len > max)
+				range->len = max;
+		} else {
+			kmem_cache_free(ext4_fc_range_cachep, range);
+			return -EAGAIN;
 		}
 
 		INIT_LIST_HEAD(&range->list);
 		list_add_tail(&range->list, ranges);
 
-		cur_lblk += map.m_len;
+		cur_lblk += range->len;
 	}
 
+	if (nr_rangesp)
+		*nr_rangesp = nr_ranges;
 	return 0;
 }
 
-static int ext4_fc_snapshot_inode(struct inode *inode)
+static int ext4_fc_snapshot_inode(struct inode *inode,
+				  unsigned int nr_ranges_total,
+				  unsigned int *nr_rangesp)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
 	struct ext4_fc_inode_snap *snap;
 	int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
 	struct ext4_iloc iloc;
 	LIST_HEAD(ranges);
+	unsigned int nr_ranges = 0;
 	int ret;
 	int alloc_ctx;
 
@@ -1072,7 +1101,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode)
 	memcpy(snap->inode_buf, (u8 *)ext4_raw_inode(&iloc), inode_len);
 	brelse(iloc.bh);
 
-	ret = ext4_fc_snapshot_inode_data(inode, &ranges);
+	ret = ext4_fc_snapshot_inode_data(inode, &ranges, nr_ranges_total,
+					  &nr_ranges);
 	if (ret) {
 		kfree(snap);
 		ext4_fc_free_ranges(&ranges);
@@ -1085,10 +1115,11 @@ static int ext4_fc_snapshot_inode(struct inode *inode)
 	list_splice_tail_init(&ranges, &snap->data_list);
 	ext4_fc_unlock(inode->i_sb, alloc_ctx);
 
+	if (nr_rangesp)
+		*nr_rangesp = nr_ranges;
 	return 0;
 }
 
-
 /* Flushes data of all the inodes in the commit queue. */
 static int ext4_fc_flush_data(journal_t *journal)
 {
@@ -1167,49 +1198,32 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
 	return 0;
 }
 
-static int ext4_fc_snapshot_inodes(journal_t *journal)
+static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb,
+					 struct inode ***inodesp,
+					 unsigned int *nr_inodesp);
+
+static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
+				   unsigned int inodes_size)
 {
 	struct super_block *sb = journal->j_private;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_inode_info *iter;
 	struct ext4_fc_dentry_update *fc_dentry;
-	struct inode **inodes;
-	unsigned int nr_inodes = 0;
 	unsigned int i = 0;
+	unsigned int idx;
+	unsigned int nr_ranges = 0;
 	int ret = 0;
 	int alloc_ctx;
 
-	alloc_ctx = ext4_fc_lock(sb);
-	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list)
-		nr_inodes++;
-
-	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
-		struct ext4_inode_info *ei;
-
-		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
-			continue;
-		if (list_empty(&fc_dentry->fcd_dilist))
-			continue;
-
-		/* See the comment in ext4_fc_commit_dentry_updates(). */
-		ei = list_first_entry(&fc_dentry->fcd_dilist,
-				      struct ext4_inode_info, i_fc_dilist);
-		if (!list_empty(&ei->i_fc_list))
-			continue;
-
-		nr_inodes++;
-	}
-	ext4_fc_unlock(sb, alloc_ctx);
-
-	if (!nr_inodes)
+	if (!inodes_size)
 		return 0;
 
-	inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS);
-	if (!inodes)
-		return -ENOMEM;
-
 	alloc_ctx = ext4_fc_lock(sb);
 	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
+		if (i >= inodes_size) {
+			ret = -E2BIG;
+			goto unlock;
+		}
 		inodes[i++] = &iter->vfs_inode;
 	}
 
@@ -1229,6 +1243,10 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
 		if (!list_empty(&ei->i_fc_list))
 			continue;
 
+		if (i >= inodes_size) {
+			ret = -E2BIG;
+			goto unlock;
+		}
 		/*
 		 * Create-only inodes may only be referenced via fcd_dilist and
 		 * not appear on s_fc_q[MAIN]. They may hit the last iput while
@@ -1240,15 +1258,22 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
 		ext4_set_inode_state(inode, EXT4_STATE_FC_COMMITTING);
 		inodes[i++] = inode;
 	}
+unlock:
 	ext4_fc_unlock(sb, alloc_ctx);
 
-	for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
-		ret = ext4_fc_snapshot_inode(inodes[nr_inodes]);
+	if (ret)
+		return ret;
+
+	for (idx = 0; idx < i; idx++) {
+		unsigned int inode_ranges = 0;
+
+		ret = ext4_fc_snapshot_inode(inodes[idx], nr_ranges,
+					     &inode_ranges);
 		if (ret)
 			break;
+		nr_ranges += inode_ranges;
 	}
 
-	kvfree(inodes);
 	return ret;
 }
 
@@ -1259,6 +1284,8 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	struct ext4_inode_info *iter;
 	struct ext4_fc_head head;
 	struct inode *inode;
+	struct inode **inodes;
+	unsigned int inodes_size;
 	struct blk_plug plug;
 	int ret = 0;
 	u32 crc = 0;
@@ -1311,6 +1338,10 @@ static int ext4_fc_perform_commit(journal_t *journal)
 		return ret;
 
 
+	ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size);
+	if (ret)
+		return ret;
+
 	/* Step 4: Mark all inodes as being committed. */
 	jbd2_journal_lock_updates(journal);
 	/*
@@ -1326,8 +1357,9 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	}
 	ext4_fc_unlock(sb, alloc_ctx);
 
-	ret = ext4_fc_snapshot_inodes(journal);
+	ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size);
 	jbd2_journal_unlock_updates(journal);
+	kvfree(inodes);
 	if (ret)
 		return ret;
 
@@ -1383,6 +1415,64 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	return ret;
 }
 
+static unsigned int ext4_fc_count_snapshot_inodes(struct super_block *sb)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_inode_info *iter;
+	struct ext4_fc_dentry_update *fc_dentry;
+	unsigned int nr_inodes = 0;
+	int alloc_ctx;
+
+	alloc_ctx = ext4_fc_lock(sb);
+	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list)
+		nr_inodes++;
+
+	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
+		struct ext4_inode_info *ei;
+
+		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
+			continue;
+		if (list_empty(&fc_dentry->fcd_dilist))
+			continue;
+
+		/* See the comment in ext4_fc_commit_dentry_updates(). */
+		ei = list_first_entry(&fc_dentry->fcd_dilist,
+				      struct ext4_inode_info, i_fc_dilist);
+		if (!list_empty(&ei->i_fc_list))
+			continue;
+
+		nr_inodes++;
+	}
+	ext4_fc_unlock(sb, alloc_ctx);
+
+	return nr_inodes;
+}
+
+static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb,
+					 struct inode ***inodesp,
+					 unsigned int *nr_inodesp)
+{
+	unsigned int nr_inodes = ext4_fc_count_snapshot_inodes(sb);
+	struct inode **inodes;
+
+	*inodesp = NULL;
+	*nr_inodesp = 0;
+
+	if (!nr_inodes)
+		return 0;
+
+	if (nr_inodes > EXT4_FC_SNAPSHOT_MAX_INODES)
+		return -E2BIG;
+
+	inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS);
+	if (!inodes)
+		return -ENOMEM;
+
+	*inodesp = inodes;
+	*nr_inodesp = nr_inodes;
+	return 0;
+}
+
 static void ext4_fc_update_stats(struct super_block *sb, int status,
 				 u64 commit_time, int nblks, tid_t commit_tid)
 {
@@ -1475,7 +1565,10 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
 	fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
 	ret = ext4_fc_perform_commit(journal);
 	if (ret < 0) {
-		status = EXT4_FC_STATUS_FAILED;
+		if (ret == -EAGAIN || ret == -E2BIG || ret == -ECANCELED)
+			status = EXT4_FC_STATUS_INELIGIBLE;
+		else
+			status = EXT4_FC_STATUS_FAILED;
 		goto fallback;
 	}
 	nblks = (sbi->s_fc_bytes + bsize - 1) / bsize - fc_bufs_before;
@@ -1559,34 +1652,35 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 
 	while (!list_empty(&sbi->s_fc_dentry_q[FC_Q_MAIN])) {
 		fc_dentry = list_first_entry(&sbi->s_fc_dentry_q[FC_Q_MAIN],
-					     struct ext4_fc_dentry_update,
-					     fcd_list);
+						 struct ext4_fc_dentry_update,
+						 fcd_list);
 		list_del_init(&fc_dentry->fcd_list);
 		if (fc_dentry->fcd_op == EXT4_FC_TAG_CREAT &&
-		    !list_empty(&fc_dentry->fcd_dilist)) {
+			!list_empty(&fc_dentry->fcd_dilist)) {
 			/* See the comment in ext4_fc_commit_dentry_updates(). */
 			ei = list_first_entry(&fc_dentry->fcd_dilist,
-					      struct ext4_inode_info,
-					      i_fc_dilist);
+						  struct ext4_inode_info,
+						  i_fc_dilist);
 			ext4_fc_free_inode_snap(&ei->vfs_inode);
 			spin_lock(&ei->i_fc_lock);
 			ext4_clear_inode_state(&ei->vfs_inode,
-					       EXT4_STATE_FC_REQUEUE);
+						   EXT4_STATE_FC_REQUEUE);
 			ext4_clear_inode_state(&ei->vfs_inode,
-					       EXT4_STATE_FC_COMMITTING);
+						   EXT4_STATE_FC_COMMITTING);
 			spin_unlock(&ei->i_fc_lock);
 			/*
 			 * Make sure clearing of EXT4_STATE_FC_COMMITTING is
-			 * visible before we send the wakeup. Pairs with implicit
-			 * barrier in prepare_to_wait() in ext4_fc_del().
+			 * visible before we send the wakeup. Pairs with
+			 * implicit barrier in prepare_to_wait() in
+			 * ext4_fc_del().
 			 */
 			smp_mb();
 #if (BITS_PER_LONG < 64)
 			wake_up_bit(&ei->i_state_flags,
-				    EXT4_STATE_FC_COMMITTING);
+					EXT4_STATE_FC_COMMITTING);
 #else
 			wake_up_bit(&ei->i_flags,
-				    EXT4_STATE_FC_COMMITTING);
+					EXT4_STATE_FC_COMMITTING);
 #endif
 		}
 		list_del_init(&fc_dentry->fcd_dilist);
@@ -2582,13 +2676,20 @@ int __init ext4_fc_init_dentry_cache(void)
 	ext4_fc_dentry_cachep = KMEM_CACHE(ext4_fc_dentry_update,
 					   SLAB_RECLAIM_ACCOUNT);
 
-	if (ext4_fc_dentry_cachep == NULL)
+	if (!ext4_fc_dentry_cachep)
 		return -ENOMEM;
 
+	ext4_fc_range_cachep = KMEM_CACHE(ext4_fc_range, SLAB_RECLAIM_ACCOUNT);
+	if (!ext4_fc_range_cachep) {
+		kmem_cache_destroy(ext4_fc_dentry_cachep);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
 void ext4_fc_destroy_dentry_cache(void)
 {
+	kmem_cache_destroy(ext4_fc_range_cachep);
 	kmem_cache_destroy(ext4_fc_dentry_cachep);
 }
-- 
2.53.0


^ permalink raw reply related

* [RFC v5 6/7] ext4: fast commit: add lock_updates tracepoint
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, linux-ext4, linux-kernel,
	linux-trace-kernel
  Cc: Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

Commit-time fast commit snapshots run under jbd2_journal_lock_updates(),
so it is useful to quantify the time spent with updates locked and to
understand why snapshotting can fail.

Add a new tracepoint, ext4_fc_lock_updates, reporting the time spent in
the updates-locked window along with the number of snapshotted inodes
and ranges. Record the first snapshot failure reason in a stable snap_err
field for tooling.

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/ext4.h              | 15 ++++++++
 fs/ext4/fast_commit.c       | 71 +++++++++++++++++++++++++++++--------
 include/trace/events/ext4.h | 61 +++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 68a64fa0be926..b9e146f3dd9e4 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1037,6 +1037,21 @@ enum {
 
 struct ext4_fc_inode_snap;
 
+/*
+ * Snapshot failure reasons for ext4_fc_lock_updates tracepoint.
+ * Keep these stable for tooling.
+ */
+enum ext4_fc_snap_err {
+	EXT4_FC_SNAP_ERR_NONE		= 0,
+	EXT4_FC_SNAP_ERR_ES_MISS	= 1,
+	EXT4_FC_SNAP_ERR_ES_DELAYED	= 2,
+	EXT4_FC_SNAP_ERR_ES_OTHER	= 3,
+	EXT4_FC_SNAP_ERR_INODES_CAP	= 4,
+	EXT4_FC_SNAP_ERR_RANGES_CAP	= 5,
+	EXT4_FC_SNAP_ERR_NOMEM		= 6,
+	EXT4_FC_SNAP_ERR_INODE_LOC	= 7,
+};
+
 /*
  * fourth extended file system inode data in memory
  */
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index d1eefee609120..4929e2990b292 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -193,6 +193,12 @@ static struct kmem_cache *ext4_fc_range_cachep;
 #define EXT4_FC_SNAPSHOT_MAX_INODES	1024
 #define EXT4_FC_SNAPSHOT_MAX_RANGES	2048
 
+static inline void ext4_fc_set_snap_err(int *snap_err, int err)
+{
+	if (snap_err && *snap_err == EXT4_FC_SNAP_ERR_NONE)
+		*snap_err = err;
+}
+
 static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
 {
 	BUFFER_TRACE(bh, "");
@@ -983,11 +989,12 @@ static void ext4_fc_free_inode_snap(struct inode *inode)
 static int ext4_fc_snapshot_inode_data(struct inode *inode,
 				       struct list_head *ranges,
 				       unsigned int nr_ranges_total,
-				       unsigned int *nr_rangesp)
+				       unsigned int *nr_rangesp,
+				       int *snap_err)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	unsigned int nr_ranges = 0;
 	ext4_lblk_t start_lblk, end_lblk, cur_lblk;
+	unsigned int nr_ranges = 0;
 
 	spin_lock(&ei->i_fc_lock);
 	if (ei->i_fc_lblk_len == 0) {
@@ -1010,11 +1017,16 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 		struct ext4_fc_range *range;
 		ext4_lblk_t len;
 
-		if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL))
+		if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) {
+			ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_MISS);
 			return -EAGAIN;
+		}
 
-		if (ext4_es_is_delayed(&es))
+		if (ext4_es_is_delayed(&es)) {
+			ext4_fc_set_snap_err(snap_err,
+					     EXT4_FC_SNAP_ERR_ES_DELAYED);
 			return -EAGAIN;
+		}
 
 		len = es.es_len - (cur_lblk - es.es_lblk);
 		if (len > end_lblk - cur_lblk + 1)
@@ -1024,12 +1036,17 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 			continue;
 		}
 
-		if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES)
+		if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) {
+			ext4_fc_set_snap_err(snap_err,
+					     EXT4_FC_SNAP_ERR_RANGES_CAP);
 			return -E2BIG;
+		}
 
 		range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS);
-		if (!range)
+		if (!range) {
+			ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
 			return -ENOMEM;
+		}
 		nr_ranges++;
 
 		range->lblk = cur_lblk;
@@ -1054,6 +1071,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 				range->len = max;
 		} else {
 			kmem_cache_free(ext4_fc_range_cachep, range);
+			ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_OTHER);
 			return -EAGAIN;
 		}
 
@@ -1070,7 +1088,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 
 static int ext4_fc_snapshot_inode(struct inode *inode,
 				  unsigned int nr_ranges_total,
-				  unsigned int *nr_rangesp)
+				  unsigned int *nr_rangesp, int *snap_err)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
 	struct ext4_fc_inode_snap *snap;
@@ -1082,8 +1100,10 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 	int alloc_ctx;
 
 	ret = ext4_get_inode_loc_noio(inode, &iloc);
-	if (ret)
+	if (ret) {
+		ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODE_LOC);
 		return ret;
+	}
 
 	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
 		inode_len = EXT4_INODE_SIZE(inode->i_sb);
@@ -1092,6 +1112,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 
 	snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS);
 	if (!snap) {
+		ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
 		brelse(iloc.bh);
 		return -ENOMEM;
 	}
@@ -1102,7 +1123,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 	brelse(iloc.bh);
 
 	ret = ext4_fc_snapshot_inode_data(inode, &ranges, nr_ranges_total,
-					  &nr_ranges);
+					  &nr_ranges, snap_err);
 	if (ret) {
 		kfree(snap);
 		ext4_fc_free_ranges(&ranges);
@@ -1203,7 +1224,10 @@ static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb,
 					 unsigned int *nr_inodesp);
 
 static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
-				   unsigned int inodes_size)
+				   unsigned int inodes_size,
+				   unsigned int *nr_inodesp,
+				   unsigned int *nr_rangesp,
+				   int *snap_err)
 {
 	struct super_block *sb = journal->j_private;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -1221,6 +1245,8 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
 	alloc_ctx = ext4_fc_lock(sb);
 	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
 		if (i >= inodes_size) {
+			ext4_fc_set_snap_err(snap_err,
+					     EXT4_FC_SNAP_ERR_INODES_CAP);
 			ret = -E2BIG;
 			goto unlock;
 		}
@@ -1244,6 +1270,8 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
 			continue;
 
 		if (i >= inodes_size) {
+			ext4_fc_set_snap_err(snap_err,
+					     EXT4_FC_SNAP_ERR_INODES_CAP);
 			ret = -E2BIG;
 			goto unlock;
 		}
@@ -1268,16 +1296,20 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
 		unsigned int inode_ranges = 0;
 
 		ret = ext4_fc_snapshot_inode(inodes[idx], nr_ranges,
-					     &inode_ranges);
+					     &inode_ranges, snap_err);
 		if (ret)
 			break;
 		nr_ranges += inode_ranges;
 	}
 
+	if (nr_inodesp)
+		*nr_inodesp = i;
+	if (nr_rangesp)
+		*nr_rangesp = nr_ranges;
 	return ret;
 }
 
-static int ext4_fc_perform_commit(journal_t *journal)
+static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
 {
 	struct super_block *sb = journal->j_private;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -1286,10 +1318,15 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	struct inode *inode;
 	struct inode **inodes;
 	unsigned int inodes_size;
+	unsigned int snap_inodes = 0;
+	unsigned int snap_ranges = 0;
+	int snap_err = EXT4_FC_SNAP_ERR_NONE;
 	struct blk_plug plug;
 	int ret = 0;
 	u32 crc = 0;
 	int alloc_ctx;
+	ktime_t lock_start;
+	u64 locked_ns;
 
 	/*
 	 * Step 1: Mark all inodes on s_fc_q[MAIN] with
@@ -1337,13 +1374,13 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	if (ret)
 		return ret;
 
-
 	ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size);
 	if (ret)
 		return ret;
 
 	/* Step 4: Mark all inodes as being committed. */
 	jbd2_journal_lock_updates(journal);
+	lock_start = ktime_get();
 	/*
 	 * The journal is now locked. No more handles can start and all the
 	 * previous handles are now drained. Snapshotting happens in this
@@ -1357,8 +1394,12 @@ static int ext4_fc_perform_commit(journal_t *journal)
 	}
 	ext4_fc_unlock(sb, alloc_ctx);
 
-	ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size);
+	ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size,
+				      &snap_inodes, &snap_ranges, &snap_err);
 	jbd2_journal_unlock_updates(journal);
+	locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start));
+	trace_ext4_fc_lock_updates(sb, commit_tid, locked_ns, snap_inodes,
+				   snap_ranges, ret, snap_err);
 	kvfree(inodes);
 	if (ret)
 		return ret;
@@ -1563,7 +1604,7 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
 		journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO;
 	set_task_ioprio(current, journal_ioprio);
 	fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
-	ret = ext4_fc_perform_commit(journal);
+	ret = ext4_fc_perform_commit(journal, commit_tid);
 	if (ret < 0) {
 		if (ret == -EAGAIN || ret == -E2BIG || ret == -ECANCELED)
 			status = EXT4_FC_STATUS_INELIGIBLE;
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index fd76d14c2776e..dc084f39b74ad 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -104,6 +104,26 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_JOURNAL_DATA);
 TRACE_DEFINE_ENUM(EXT4_FC_REASON_ENCRYPTED_FILENAME);
 TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
 
+#undef EM
+#undef EMe
+#define EM(a)	TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a);
+#define EMe(a)	TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a);
+
+#define TRACE_SNAP_ERR						\
+	EM(NONE)						\
+	EM(ES_MISS)						\
+	EM(ES_DELAYED)						\
+	EM(ES_OTHER)						\
+	EM(INODES_CAP)						\
+	EM(RANGES_CAP)						\
+	EM(NOMEM)						\
+	EMe(INODE_LOC)
+
+TRACE_SNAP_ERR
+
+#undef EM
+#undef EMe
+
 #define show_fc_reason(reason)						\
 	__print_symbolic(reason,					\
 		{ EXT4_FC_REASON_XATTR,		"XATTR"},		\
@@ -2812,6 +2832,47 @@ TRACE_EVENT(ext4_fc_commit_stop,
 		  __entry->num_fc_ineligible, __entry->nblks_agg, __entry->tid)
 );
 
+#define EM(a)	{ EXT4_FC_SNAP_ERR_##a, #a },
+#define EMe(a)	{ EXT4_FC_SNAP_ERR_##a, #a }
+
+TRACE_EVENT(ext4_fc_lock_updates,
+	    TP_PROTO(struct super_block *sb, tid_t commit_tid, u64 locked_ns,
+		     unsigned int nr_inodes, unsigned int nr_ranges, int err,
+		     int snap_err),
+
+	TP_ARGS(sb, commit_tid, locked_ns, nr_inodes, nr_ranges, err, snap_err),
+
+	TP_STRUCT__entry(/* entry */
+		__field(dev_t, dev)
+		__field(tid_t, tid)
+		__field(u64, locked_ns)
+		__field(unsigned int, nr_inodes)
+		__field(unsigned int, nr_ranges)
+		__field(int, err)
+		__field(int, snap_err)
+	),
+
+	TP_fast_assign(/* assign */
+		__entry->dev = sb->s_dev;
+		__entry->tid = commit_tid;
+		__entry->locked_ns = locked_ns;
+		__entry->nr_inodes = nr_inodes;
+		__entry->nr_ranges = nr_ranges;
+		__entry->err = err;
+		__entry->snap_err = snap_err;
+	),
+
+	TP_printk("dev %d,%d tid %u locked_ns %llu nr_inodes %u nr_ranges %u err %d snap_err %s",
+		  MAJOR(__entry->dev), MINOR(__entry->dev), __entry->tid,
+		  __entry->locked_ns, __entry->nr_inodes, __entry->nr_ranges,
+		  __entry->err, __print_symbolic(__entry->snap_err,
+						 TRACE_SNAP_ERR))
+);
+
+#undef EM
+#undef EMe
+#undef TRACE_SNAP_ERR
+
 #define FC_REASON_NAME_STAT(reason)					\
 	show_fc_reason(reason),						\
 	__entry->fc_ineligible_rc[reason]
-- 
2.53.0


^ permalink raw reply related

* [RFC v5 7/7] ext4: fast commit: export snapshot stats in fc_info
From: Li Chen @ 2026-03-17  8:46 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260317084624.457185-1-me@linux.beauty>

Snapshot-based fast commit can fall back when the commit-time snapshot
cannot be built (e.g. extent status cache misses). It is useful to
quantify the updates-locked window and to see why snapshotting failed.

Add best-effort snapshot counters to the ext4 superblock and extend
/proc/fs/ext4/<sb_id>/fc_info to report the number of snapshotted
inodes and ranges, snapshot failure reasons, and the average/max time
spent with journal updates locked.

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/ext4.h        | 31 ++++++++++++++++++++++
 fs/ext4/fast_commit.c | 61 ++++++++++++++++++++++++++++++++++++++++---
 fs/ext4/super.c       |  1 +
 3 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b9e146f3dd9e4..8b7530f2e0706 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1566,6 +1566,36 @@ struct ext4_orphan_info {
 						 * file blocks */
 };
 
+/*
+ * Ext4 fast commit snapshot statistics.
+ *
+ * These are best-effort counters intended for debugging / performance
+ * introspection; they are not exact under concurrent updates.
+ */
+struct ext4_fc_snap_stats {
+	u64 lock_updates_ns_total;
+	u64 lock_updates_ns_max;
+	u64 lock_updates_samples;
+
+	u64 snap_inodes;
+	u64 snap_ranges;
+
+	u64 snap_fail_es_miss;
+	u64 snap_fail_es_delayed;
+	u64 snap_fail_es_other;
+
+	u64 snap_fail_inodes_cap;
+	u64 snap_fail_ranges_cap;
+	u64 snap_fail_nomem;
+	u64 snap_fail_inode_loc;
+
+	/*
+	 * Missing inode snapshots during log writing should never happen.
+	 * Keep this counter to help catch unexpected regressions.
+	 */
+	u64 snap_fail_no_snap;
+};
+
 /*
  * fourth extended-fs super-block data in memory
  */
@@ -1837,6 +1867,7 @@ struct ext4_sb_info {
 	struct mutex s_fc_lock;
 	struct buffer_head *s_fc_bh;
 	struct ext4_fc_stats s_fc_stats;
+	struct ext4_fc_snap_stats s_fc_snap_stats;
 	tid_t s_fc_ineligible_tid;
 #ifdef CONFIG_EXT4_DEBUG
 	int s_fc_debug_max_replay;
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 4929e2990b292..09ae8f52abdab 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -890,13 +890,17 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
 	int inode_len;
 	int ret;
 
-	if (!snap)
+	if (!snap) {
+		EXT4_SB(inode->i_sb)->s_fc_snap_stats.snap_fail_no_snap++;
 		return -ECANCELED;
+	}
 
 	src = snap->inode_buf;
 	inode_len = snap->inode_len;
-	if (!src || inode_len == 0)
+	if (!src || inode_len == 0) {
+		EXT4_SB(inode->i_sb)->s_fc_snap_stats.snap_fail_no_snap++;
 		return -ECANCELED;
+	}
 
 	fc_inode.fc_ino = cpu_to_le32(inode->i_ino);
 	tl.fc_tag = cpu_to_le16(EXT4_FC_TAG_INODE);
@@ -931,8 +935,10 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 	struct ext4_extent *ex;
 	struct ext4_fc_range *range;
 
-	if (!snap)
+	if (!snap) {
+		EXT4_SB(inode->i_sb)->s_fc_snap_stats.snap_fail_no_snap++;
 		return -ECANCELED;
+	}
 
 	list_for_each_entry(range, &snap->data_list, list) {
 		if (range->tag == EXT4_FC_TAG_DEL_RANGE) {
@@ -993,6 +999,8 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 				       int *snap_err)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_fc_snap_stats *stats =
+		&EXT4_SB(inode->i_sb)->s_fc_snap_stats;
 	ext4_lblk_t start_lblk, end_lblk, cur_lblk;
 	unsigned int nr_ranges = 0;
 
@@ -1018,11 +1026,13 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 		ext4_lblk_t len;
 
 		if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) {
+			stats->snap_fail_es_miss++;
 			ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_MISS);
 			return -EAGAIN;
 		}
 
 		if (ext4_es_is_delayed(&es)) {
+			stats->snap_fail_es_delayed++;
 			ext4_fc_set_snap_err(snap_err,
 					     EXT4_FC_SNAP_ERR_ES_DELAYED);
 			return -EAGAIN;
@@ -1037,6 +1047,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 		}
 
 		if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) {
+			stats->snap_fail_ranges_cap++;
 			ext4_fc_set_snap_err(snap_err,
 					     EXT4_FC_SNAP_ERR_RANGES_CAP);
 			return -E2BIG;
@@ -1044,6 +1055,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 
 		range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS);
 		if (!range) {
+			stats->snap_fail_nomem++;
 			ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
 			return -ENOMEM;
 		}
@@ -1071,6 +1083,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
 				range->len = max;
 		} else {
 			kmem_cache_free(ext4_fc_range_cachep, range);
+			stats->snap_fail_es_other++;
 			ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_OTHER);
 			return -EAGAIN;
 		}
@@ -1091,6 +1104,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 				  unsigned int *nr_rangesp, int *snap_err)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_fc_snap_stats *stats =
+		&EXT4_SB(inode->i_sb)->s_fc_snap_stats;
 	struct ext4_fc_inode_snap *snap;
 	int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
 	struct ext4_iloc iloc;
@@ -1101,6 +1116,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 
 	ret = ext4_get_inode_loc_noio(inode, &iloc);
 	if (ret) {
+		stats->snap_fail_inode_loc++;
 		ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODE_LOC);
 		return ret;
 	}
@@ -1112,6 +1128,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 
 	snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS);
 	if (!snap) {
+		stats->snap_fail_nomem++;
 		ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
 		brelse(iloc.bh);
 		return -ENOMEM;
@@ -1136,6 +1153,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
 	list_splice_tail_init(&ranges, &snap->data_list);
 	ext4_fc_unlock(inode->i_sb, alloc_ctx);
 
+	stats->snap_inodes++;
+	stats->snap_ranges += nr_ranges;
 	if (nr_rangesp)
 		*nr_rangesp = nr_ranges;
 	return 0;
@@ -1245,6 +1264,7 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
 	alloc_ctx = ext4_fc_lock(sb);
 	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
 		if (i >= inodes_size) {
+			sbi->s_fc_snap_stats.snap_fail_inodes_cap++;
 			ext4_fc_set_snap_err(snap_err,
 					     EXT4_FC_SNAP_ERR_INODES_CAP);
 			ret = -E2BIG;
@@ -1270,6 +1290,7 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
 			continue;
 
 		if (i >= inodes_size) {
+			sbi->s_fc_snap_stats.snap_fail_inodes_cap++;
 			ext4_fc_set_snap_err(snap_err,
 					     EXT4_FC_SNAP_ERR_INODES_CAP);
 			ret = -E2BIG;
@@ -1313,6 +1334,7 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
 {
 	struct super_block *sb = journal->j_private;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_fc_snap_stats *snap_stats = &sbi->s_fc_snap_stats;
 	struct ext4_inode_info *iter;
 	struct ext4_fc_head head;
 	struct inode *inode;
@@ -1375,8 +1397,13 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
 		return ret;
 
 	ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size);
-	if (ret)
+	if (ret) {
+		if (ret == -E2BIG)
+			snap_stats->snap_fail_inodes_cap++;
+		else if (ret == -ENOMEM)
+			snap_stats->snap_fail_nomem++;
 		return ret;
+	}
 
 	/* Step 4: Mark all inodes as being committed. */
 	jbd2_journal_lock_updates(journal);
@@ -1398,6 +1425,10 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
 				      &snap_inodes, &snap_ranges, &snap_err);
 	jbd2_journal_unlock_updates(journal);
 	locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start));
+	snap_stats->lock_updates_ns_total += locked_ns;
+	snap_stats->lock_updates_samples++;
+	if (locked_ns > snap_stats->lock_updates_ns_max)
+		snap_stats->lock_updates_ns_max = locked_ns;
 	trace_ext4_fc_lock_updates(sb, commit_tid, locked_ns, snap_inodes,
 				   snap_ranges, ret, snap_err);
 	kvfree(inodes);
@@ -2694,11 +2725,17 @@ int ext4_fc_info_show(struct seq_file *seq, void *v)
 {
 	struct ext4_sb_info *sbi = EXT4_SB((struct super_block *)seq->private);
 	struct ext4_fc_stats *stats = &sbi->s_fc_stats;
+	struct ext4_fc_snap_stats *snap_stats = &sbi->s_fc_snap_stats;
+	u64 lock_avg_ns = 0;
 	int i;
 
 	if (v != SEQ_START_TOKEN)
 		return 0;
 
+	if (snap_stats->lock_updates_samples)
+		lock_avg_ns = div_u64(snap_stats->lock_updates_ns_total,
+				      snap_stats->lock_updates_samples);
+
 	seq_printf(seq,
 		"fc stats:\n%ld commits\n%ld ineligible\n%ld numblks\n%lluus avg_commit_time\n",
 		   stats->fc_num_commits, stats->fc_ineligible_commits,
@@ -2709,6 +2746,22 @@ int ext4_fc_info_show(struct seq_file *seq, void *v)
 		seq_printf(seq, "\"%s\":\t%d\n", fc_ineligible_reasons[i],
 			stats->fc_ineligible_reason_count[i]);
 
+	seq_printf(seq,
+		   "Snapshot stats:\n%llu inodes\n%llu ranges\n%lluus lock_updates_avg\n%lluus lock_updates_max\n",
+		   snap_stats->snap_inodes, snap_stats->snap_ranges,
+		   div_u64(lock_avg_ns, 1000),
+		   div_u64(snap_stats->lock_updates_ns_max, 1000));
+	seq_printf(seq,
+		   "Snapshot failures:\n%llu es_miss\n%llu es_delayed\n%llu es_other\n%llu inodes_cap\n%llu ranges_cap\n%llu nomem\n%llu inode_loc\n%llu no_snap\n",
+		   snap_stats->snap_fail_es_miss,
+		   snap_stats->snap_fail_es_delayed,
+		   snap_stats->snap_fail_es_other,
+		   snap_stats->snap_fail_inodes_cap,
+		   snap_stats->snap_fail_ranges_cap,
+		   snap_stats->snap_fail_nomem,
+		   snap_stats->snap_fail_inode_loc,
+		   snap_stats->snap_fail_no_snap);
+
 	return 0;
 }
 
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4f5f0c21d436f..3afcaf9d80078 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4500,6 +4500,7 @@ static void ext4_fast_commit_init(struct super_block *sb)
 	sbi->s_fc_ineligible_tid = 0;
 	mutex_init(&sbi->s_fc_lock);
 	memset(&sbi->s_fc_stats, 0, sizeof(sbi->s_fc_stats));
+	memset(&sbi->s_fc_snap_stats, 0, sizeof(sbi->s_fc_snap_stats));
 	sbi->s_fc_replay_state.fc_regions = NULL;
 	sbi->s_fc_replay_state.fc_regions_size = 0;
 	sbi->s_fc_replay_state.fc_regions_used = 0;
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH 33/53] ext4: use on-stack dentries in ext4_fc_replay_link_internal()
From: Jan Kara @ 2026-03-17  9:37 UTC (permalink / raw)
  To: NeilBrown
  Cc: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
	Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
	Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
	Baolin Wang, David Howells, Marc Dionne, Steve French,
	Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
	Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
	Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
	Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel,
	linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
	linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
	linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
	linux-efi
In-Reply-To: <20260312214330.3885211-34-neilb@ownmail.net>

On Fri 13-03-26 08:12:20, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> ext4_fc_replay_link_internal() uses two dentries to simply code-reuse
> when replaying a "link" operation.  It does not need to interact with
> the dcache and removes the dentries shortly after adding them.
> 
> They are passed to __ext4_link() which only performs read accesses on
> these dentries and only uses the name and parent of dentry_inode (plus
> checking a flag is unset) and only uses the inode of the parent.
> 
> So instead of allocating dentries and adding them to the dcache, allocat
> two dentries on the stack, set up the required fields, and pass these to
> __ext4_link().
> 
> This substantially simplifies the code and removes on of the few uses of
> d_alloc() - preparing for its removal.
> 
> Signed-off-by: NeilBrown <neil@brown.name>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/fast_commit.c | 40 ++++++++--------------------------------
>  1 file changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 2a5daf1d9667..e3593bb90a62 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1446,8 +1446,6 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
>  				struct inode *inode)
>  {
>  	struct inode *dir = NULL;
> -	struct dentry *dentry_dir = NULL, *dentry_inode = NULL;
> -	struct qstr qstr_dname = QSTR_INIT(darg->dname, darg->dname_len);
>  	int ret = 0;
>  
>  	dir = ext4_iget(sb, darg->parent_ino, EXT4_IGET_NORMAL);
> @@ -1457,28 +1455,14 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
>  		goto out;
>  	}
>  
> -	dentry_dir = d_obtain_alias(dir);
> -	if (IS_ERR(dentry_dir)) {
> -		ext4_debug("Failed to obtain dentry");
> -		dentry_dir = NULL;
> -		goto out;
> -	}
> +	{
> +		struct dentry dentry_dir = { .d_inode = dir };
> +		const struct dentry dentry_inode = {
> +			.d_parent = &dentry_dir,
> +			.d_name = QSTR_LEN(darg->dname, darg->dname_len),
> +		};
>  
> -	dentry_inode = d_alloc(dentry_dir, &qstr_dname);
> -	if (!dentry_inode) {
> -		ext4_debug("Inode dentry not created.");
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> -
> -	ihold(inode);
> -	inc_nlink(inode);
> -	ret = __ext4_link(dir, inode, dentry_inode);
> -	if (ret) {
> -		drop_nlink(inode);
> -		iput(inode);
> -	} else {
> -		d_instantiate(dentry_inode, inode);
> +		ret = __ext4_link(dir, inode, &dentry_inode);
>  	}
>  	/*
>  	 * It's possible that link already existed since data blocks
> @@ -1493,16 +1477,8 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
>  
>  	ret = 0;
>  out:
> -	if (dentry_dir) {
> -		d_drop(dentry_dir);
> -		dput(dentry_dir);
> -	} else if (dir) {
> +	if (dir)
>  		iput(dir);
> -	}
> -	if (dentry_inode) {
> -		d_drop(dentry_inode);
> -		dput(dentry_inode);
> -	}
>  
>  	return ret;
>  }
> -- 
> 2.50.0.107.gf914562f5916.dirty
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH 32/53] ext4: move dcache modifying code out of __ext4_link()
From: Jan Kara @ 2026-03-17 10:00 UTC (permalink / raw)
  To: NeilBrown
  Cc: Linus Torvalds, Alexander Viro, Christian Brauner, Jan Kara,
	Jeff Layton, Trond Myklebust, Anna Schumaker, Carlos Maiolino,
	Miklos Szeredi, Amir Goldstein, Jan Harkes, Hugh Dickins,
	Baolin Wang, David Howells, Marc Dionne, Steve French,
	Namjae Jeon, Sungjong Seo, Yuezhang Mo, Andreas Hindborg,
	Breno Leitao, Theodore Ts'o, Andreas Dilger, Steven Rostedt,
	Masami Hiramatsu, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
	Tyler Hicks, Andreas Gruenbacher, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Jeremy Kerr, Ard Biesheuvel,
	linux-fsdevel, linux-nfs, linux-xfs, linux-unionfs, coda,
	linux-mm, linux-afs, linux-cifs, linux-ext4, linux-kernel,
	linux-trace-kernel, ceph-devel, ecryptfs, gfs2, linux-um,
	linux-efi
In-Reply-To: <20260312214330.3885211-33-neilb@ownmail.net>

On Fri 13-03-26 08:12:19, NeilBrown wrote:
...
> diff --git a/fs/dcache.c b/fs/dcache.c
> index a1219b446b74..c48337d95f9a 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -358,7 +358,7 @@ static inline int dname_external(const struct dentry *dentry)
>  	return dentry->d_name.name != dentry->d_shortname.string;
>  }
>  
> -void take_dentry_name_snapshot(struct name_snapshot *name, struct dentry *dentry)
> +void take_dentry_name_snapshot(struct name_snapshot *name, const struct dentry *dentry)
>  {
>  	unsigned seq;
>  	const unsigned char *s;

The constification of take_dentry_name_snapshot() should probably be a
separate patch? Also I'd note that this constification (and the
constification of __ext4_fc_track_link()) isn't really needed here because
ext4_fc_track_link() will immediately bail through ext4_fc_disabled() when
fast commit replay is happening so __ext4_fc_track_link() never gets called
in that case - more about that below.

> @@ -1471,7 +1471,15 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
>  		goto out;
>  	}
>  
> +	ihold(inode);
> +	inc_nlink(inode);
>  	ret = __ext4_link(dir, inode, dentry_inode);
> +	if (ret) {
> +		drop_nlink(inode);
> +		iput(inode);
> +	} else {
> +		d_instantiate(dentry_inode, inode);
> +	}
>  	/*
>  	 * It's possible that link already existed since data blocks
>  	 * for the dir in question got persisted before we crashed OR
...
> @@ -3460,8 +3460,6 @@ int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry)
>  		ext4_handle_sync(handle);
>  
>  	inode_set_ctime_current(inode);
> -	ext4_inc_count(inode);
> -	ihold(inode);
>  
>  	err = ext4_add_entry(handle, dentry, inode);
>  	if (!err) {
> @@ -3471,11 +3469,7 @@ int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry)
>  		 */
>  		if (inode->i_nlink == 1)
>  			ext4_orphan_del(handle, inode);
> -		d_instantiate(dentry, inode);
> -		ext4_fc_track_link(handle, dentry);
> -	} else {
> -		drop_nlink(inode);
> -		iput(inode);
> +		__ext4_fc_track_link(handle, inode, dentry);

This looks wrong. If fastcommit replay is running, we must skip calling
__ext4_fc_track_link(). Similarly if the filesystem is currently
inelligible for fastcommit (due to some complex unsupported operations
running in parallel). Why did you change ext4_fc_track_link() to
__ext4_fc_track_link()?

> @@ -3504,7 +3498,16 @@ static int ext4_link(struct dentry *old_dentry,
>  	err = dquot_initialize(dir);
>  	if (err)
>  		return err;
> -	return __ext4_link(dir, inode, dentry);
> +	ihold(inode);
> +	ext4_inc_count(inode);

I'd put inc_nlink() here instead. We are guaranteed to have a regular file
anyway and it matches what we do in ext4_fc_replay_link_internal().
Alternatively we could consistently use ext4_inc_count() &
ext4_dec_count() in these functions.

> +	err = __ext4_link(dir, inode, dentry);
> +	if (err) {
> +		drop_nlink(inode);
> +		iput(inode);
> +	} else {
> +		d_instantiate(dentry, inode);
> +	}
> +	return err;
>  }

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* [PATCH] ext4: xattr: fix size_t underflow in ext4_xattr_set_entry
From: ZhengYuan Huang @ 2026-03-17 10:28 UTC (permalink / raw)
  To: tytso, adilger.kernel
  Cc: linux-ext4, linux-kernel, baijiaju1990, r33s3n6, zzzccc427,
	ZhengYuan Huang, stable

[BUG]
KASAN reports an out-of-bounds read with an astronomically large access
size when ext4_xattr_set_entry() deletes a corrupted ibody xattr entry:

  BUG: KASAN: out-of-bounds in ext4_xattr_set_entry+0x11a3/0x1f40 fs/ext4/xattr.c:1756
  Read of size 18446744073709551600 at addr ffff8880179737cc
  Call Trace:
   ...
   ext4_xattr_set_entry+0x11a3/0x1f40 fs/ext4/xattr.c:1756
   ext4_xattr_ibody_set+0x396/0x5a0 fs/ext4/xattr.c:2268
   ext4_destroy_inline_data_nolock+0x25e/0x560 fs/ext4/inline.c:463
   ext4_convert_inline_data_nolock+0x186/0xa80 fs/ext4/inline.c:1105
   ext4_try_add_inline_entry+0x58e/0x960 fs/ext4/inline.c:1224
   ext4_add_entry+0x6d2/0xce0 fs/ext4/namei.c:2389
   ext4_add_nondir+0x9c/0x280 fs/ext4/namei.c:2784
   ext4_create+0x380/0x500 fs/ext4/namei.c:2830
   ...

The access size 18446744073709551600 equals (size_t)(-16), the result of
a pointer arithmetic underflow.

The bug is reproducible on next-20260313 with our dynamic metadata
fuzzing tool that corrupts ext4 metadata at runtime.

[CAUSE]
When deleting an ibody xattr entry (i->value == NULL), the code computes
the memmove length as:

  (void *)last - (void *)here + sizeof(__u32)

where `last` is first the result of walking the xattr entry list to its
terminating IS_LAST_ENTRY, and then stepped back by EXT4_XATTR_LEN(name_len):

  last = ENTRY((void *)last - size);

Consider a corrupted ibody xattr list where a spurious IS_LAST_ENTRY (four
zero bytes) has been planted before the real target entry (e.g.,
system.data used for inline data).  When ext4_xattr_ibody_find() calls
xattr_find_entry() to locate the entry, the walk stops at the spurious
terminator.  xattr_find_entry() returns -ENODATA and sets s->here to that
terminator position; s->not_found is set to -ENODATA accordingly.

Back in ext4_xattr_set_entry(), the entry-walking for loop starts from
s->first and also stops at the same spurious IS_LAST_ENTRY, so
last == here after the loop.  Because the "remove old value" block is
guarded by (!s->not_found), it is skipped.  The delete-name path
(if (!i->value)) is not guarded by s->not_found at all, so it runs
unconditionally.  It then executes:

  last = ENTRY((void *)last - size);  /* last = here - 20 */

which places last before here.  The subsequent memmove() computes its
length as:

  (here - 20) - here + sizeof(__u32) = -16

which wraps to (size_t)(-16) = 18446744073709551600 as an unsigned type,
causing memmove() to attempt an enormous read.

The load-time check_xattrs() call also stops at the first IS_LAST_ENTRY
it encounters.  A spurious terminator inserted before the real entries
silently truncates the range that check_xattrs() examines, so entries
beyond it are never validated at load time.

[FIX]
After adjusting `last`, check that it has not moved before `here`. If it
has, the xattr entry list is corrupted; reject it with -EFSCORRUPTED.

After this fix, the code will detect the corrupted entry and reject it with
-EFSCORRUPTED, preventing the out-of-bounds access.

Cc: stable@vger.kernel.org
Signed-off-by: ZhengYuan Huang <gality369@gmail.com>
---
It might also be worth considering whether check_xattrs() needs a similar fix.
I'm not deeply familiar with the ext4 codebase, so I'd appreciate any guidance
from the maintainers and would be happy to update and resend the patch if needed.
---
 fs/ext4/xattr.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ce7253b3f549..b8f1102972f0 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1753,6 +1753,11 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 		size_t size = EXT4_XATTR_LEN(name_len);
 
 		last = ENTRY((void *)last - size);
+		if ((void *)last < (void *)here) {
+			EXT4_ERROR_INODE(inode, "corrupted xattr entries: last before here");
+			ret = -EFSCORRUPTED;
+			goto out;
+		}
 		memmove(here, (void *)here + size,
 			(void *)last - (void *)here + sizeof(__u32));
 		memset(last, 0, size);
-- 
2.43.0

^ permalink raw reply related

* Re: [PATCH v1] ext4: fix use-after-free in update_super_work when racing with umount
From: Jan Kara @ 2026-03-17 11:38 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: linux-ext4, Theodore Ts'o, Andreas Dilger, Jan Kara,
	Ritesh Harjani, Ye Bin, linux-kernel
In-Reply-To: <20260313065206.152645-1-jiayuan.chen@linux.dev>

On Fri 13-03-26 14:52:04, Jiayuan Chen wrote:
> Commit b98535d09179 ("ext4: fix bug_on in start_this_handle during umount filesystem")
> moved ext4_unregister_sysfs() before flushing s_sb_upd_work to prevent new
> error work from being queued via /proc/fs/ext4/xx/mb_groups reads during
> unmount. However, this introduced a use-after-free because
> update_super_work calls ext4_notify_error_sysfs() -> sysfs_notify() which
> accesses the kobject's kernfs_node after it has been freed:
> 
>   update_super_work                ext4_put_super
>   -----------------                --------------
>                                    ext4_unregister_sysfs(sb)
>                                      kobject_del(&sbi->s_kobj)
>                                        __kobject_del()
>                                          sysfs_remove_dir()
>                                            kobj->sd = NULL
>                                          sysfs_put(sd)
>                                            kernfs_put()  // RCU free
>   ext4_notify_error_sysfs(sbi)
>     sysfs_notify(&sbi->s_kobj)
>       kn = kobj->sd              // stale pointer
>       kernfs_get(kn)             // UAF on freed kernfs_node
>                                    ext4_journal_destroy()
>                                      flush_work(&sbi->s_sb_upd_work)
> 
> The original blamed commit needed procfs removal before the work
> flush to prevent /proc/fs/ext4/xx/mb_groups reads from queuing new error
> work. But it bundled procfs removal and kobject_del together in
> ext4_unregister_sysfs(), causing the sysfs kobject to be torn down too
> early.
> 
> The correct teardown ordering has three constraints:
> 
>   1. procfs removal must happen before flushing s_sb_upd_work, to prevent
>      /proc reads from queuing new error work that would BUG_ON.
>   2. sysfs kobject removal must happen after flushing s_sb_upd_work, since
>      the work calls sysfs_notify() which accesses the kernfs_node.
>   3. sysfs kobject removal must happen before jbd2_journal_destroy(), since
>      userspace could read the journal_task sysfs attribute and dereference
>      j_task after the journal thread has been killed.
> 
> Fix this by:
>   - Adding ext4_sb_release_proc() to remove procfs entries early.
>   - Splitting ext4_journal_destroy() into ext4_journal_stop_work() and
>     ext4_journal_finish(), so that ext4_unregister_sysfs() can be placed
>     between them to satisfy all three ordering constraints.
> 
> Fixes: b98535d09179 ("ext4: fix bug_on in start_this_handle during umount filesystem")
> Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Thanks for the analysis and the patch! I fully agree with your analysis but
I think your solution shows that the teardown sequence is just too subtle
(too many different dependencies). Ideally, we'd instead modify
ext4_notify_error_sysfs() to detect sysfs is already torn down by
ext4_unregister_sysfs() and do nothing. We can check
sbi->s_kobj.state_in_sysfs for that. The only trouble with this is that
sysfs_notify() could still race with kobject_del() so we also need some
locking in ext4_unregister_sysfs() locking out parallel
ext4_notify_error_sysfs() and we probably need to introduce a separate
mutex for that. What do you think?

								Honza
> ---
>  fs/ext4/ext4.h      |  1 +
>  fs/ext4/ext4_jbd2.h | 45 ++++++++++++++++++++++++++++-----------------
>  fs/ext4/super.c     | 34 +++++++++++++++++++++++-----------
>  fs/ext4/sysfs.c     | 10 ++++++++++
>  4 files changed, 62 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index b76966dc06c0..a693365d224c 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3757,6 +3757,7 @@ extern const struct inode_operations ext4_fast_symlink_inode_operations;
>  /* sysfs.c */
>  extern void ext4_notify_error_sysfs(struct ext4_sb_info *sbi);
>  extern int ext4_register_sysfs(struct super_block *sb);
> +extern void ext4_sb_release_proc(struct super_block *sb);
>  extern void ext4_unregister_sysfs(struct super_block *sb);
>  extern int __init ext4_init_sysfs(void);
>  extern void ext4_exit_sysfs(void);
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 63d17c5201b5..2b7d68b11578 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -430,32 +430,43 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
>  }
>  
>  /*
> - * Pass journal explicitly as it may not be cached in the sbi->s_journal in some
> - * cases
> + * Stop new s_sb_upd_work from being queued and flush any pending work.
> + *
> + * At this point only two things can be operating on the journal:
> + * JBD2 thread performing transaction commit and s_sb_upd_work
> + * issuing sb update through the journal. Once we set
> + * EXT4_MF_JOURNAL_DESTROY, new ext4_handle_error() calls will not
> + * queue s_sb_upd_work and ext4_force_commit() makes sure any
> + * ext4_handle_error() calls from the running transaction commit are
> + * finished. Hence no new s_sb_upd_work can be queued after we
> + * flush it here.
>   */
> -static inline int ext4_journal_destroy(struct ext4_sb_info *sbi, journal_t *journal)
> +static inline void ext4_journal_stop_work(struct ext4_sb_info *sbi)
>  {
> -	int err = 0;
> -
> -	/*
> -	 * At this point only two things can be operating on the journal.
> -	 * JBD2 thread performing transaction commit and s_sb_upd_work
> -	 * issuing sb update through the journal. Once we set
> -	 * EXT4_JOURNAL_DESTROY, new ext4_handle_error() calls will not
> -	 * queue s_sb_upd_work and ext4_force_commit() makes sure any
> -	 * ext4_handle_error() calls from the running transaction commit are
> -	 * finished. Hence no new s_sb_upd_work can be queued after we
> -	 * flush it here.
> -	 */
>  	ext4_set_mount_flag(sbi->s_sb, EXT4_MF_JOURNAL_DESTROY);
> -
>  	ext4_force_commit(sbi->s_sb);
>  	flush_work(&sbi->s_sb_upd_work);
> +}
> +
> +/*
> + * Destroy the journal. Must be called after ext4_journal_stop_work().
> + * Pass journal explicitly as it may not be cached in the sbi->s_journal
> + * in some cases.
> + */
> +static inline int ext4_journal_finish(struct ext4_sb_info *sbi,
> +				      journal_t *journal)
> +{
> +	int err;
>  
>  	err = jbd2_journal_destroy(journal);
>  	sbi->s_journal = NULL;
> -
>  	return err;
>  }
>  
> +static inline int ext4_journal_destroy(struct ext4_sb_info *sbi, journal_t *journal)
> +{
> +	ext4_journal_stop_work(sbi);
> +	return ext4_journal_finish(sbi, journal);
> +}
> +
>  #endif	/* _EXT4_JBD2_H */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 752f414aa06b..9bba783f44e1 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1280,16 +1280,12 @@ static void ext4_put_super(struct super_block *sb)
>  	int err;
>  
>  	/*
> -	 * Unregister sysfs before destroying jbd2 journal.
> -	 * Since we could still access attr_journal_task attribute via sysfs
> -	 * path which could have sbi->s_journal->j_task as NULL
> -	 * Unregister sysfs before flush sbi->s_sb_upd_work.
> -	 * Since user may read /proc/fs/ext4/xx/mb_groups during umount, If
> -	 * read metadata verify failed then will queue error work.
> -	 * update_super_work will call start_this_handle may trigger
> -	 * BUG_ON.
> +	 * Remove procfs entries before flush s_sb_upd_work. Since user may
> +	 * read /proc/fs/ext4/xx/mb_groups during umount, if read metadata
> +	 * verify failed then will queue error work. update_super_work will
> +	 * call start_this_handle which may trigger BUG_ON.
>  	 */
> -	ext4_unregister_sysfs(sb);
> +	ext4_sb_release_proc(sb);
>  
>  	if (___ratelimit(&ext4_mount_msg_ratelimit, "EXT4-fs unmount"))
>  		ext4_msg(sb, KERN_INFO, "unmounting filesystem %pU.",
> @@ -1301,14 +1297,30 @@ static void ext4_put_super(struct super_block *sb)
>  	destroy_workqueue(sbi->rsv_conversion_wq);
>  	ext4_release_orphan_info(sb);
>  
> +	/*
> +	 * Flush s_sb_upd_work before unregistering sysfs, since
> +	 * update_super_work calls ext4_notify_error_sysfs() which accesses
> +	 * the kobject's kernfs_node via sysfs_notify(). Unregistering sysfs
> +	 * before the flush could lead to a use-after-free on the
> +	 * kernfs_node.
> +	 *
> +	 * Also unregister sysfs before destroying jbd2 journal, since
> +	 * userspace could read the journal_task sysfs attribute while
> +	 * jbd2_journal_destroy() is killing the journal thread, leading to
> +	 * a NULL pointer dereference of j_task in journal_task_show().
> +	 */
>  	if (sbi->s_journal) {
>  		aborted = is_journal_aborted(sbi->s_journal);
> -		err = ext4_journal_destroy(sbi, sbi->s_journal);
> +		ext4_journal_stop_work(sbi);
> +		ext4_unregister_sysfs(sb);
> +		err = ext4_journal_finish(sbi, sbi->s_journal);
>  		if ((err < 0) && !aborted) {
>  			ext4_abort(sb, -err, "Couldn't clean up the journal");
>  		}
> -	} else
> +	} else {
>  		flush_work(&sbi->s_sb_upd_work);
> +		ext4_unregister_sysfs(sb);
> +	}
>  
>  	ext4_es_unregister_shrinker(sbi);
>  	timer_shutdown_sync(&sbi->s_err_report);
> diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
> index d2ecc1026c0c..f6947416c1e7 100644
> --- a/fs/ext4/sysfs.c
> +++ b/fs/ext4/sysfs.c
> @@ -638,6 +638,16 @@ int ext4_register_sysfs(struct super_block *sb)
>  	return 0;
>  }
>  
> +void ext4_sb_release_proc(struct super_block *sb)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +
> +	if (sbi->s_proc) {
> +		remove_proc_subtree(sb->s_id, ext4_proc_root);
> +		sbi->s_proc = NULL;
> +	}
> +}
> +
>  void ext4_unregister_sysfs(struct super_block *sb)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH e2fsprogs] lib/quota: fix checksum mismatch on uninitialized PRJQUOTA inode
From: Jan Kara @ 2026-03-17 11:50 UTC (permalink / raw)
  To: Baokun Li; +Cc: tytso, adilger.kernel, jack, linux-ext4
In-Reply-To: <8382fe78-5b3f-4c1a-96eb-62494835f693@linux.alibaba.com>

On Thu 12-03-26 14:26:08, Baokun Li wrote:
> A gentle ping...
> 
> On 12/10/25 4:15 PM, libaokun@huaweicloud.com wrote:
> > From: Baokun Li <libaokun1@huawei.com>
> >
> > In quota_inode_init_new(), we attempt to read and truncate an existing
> > quota inode before proceeding with its initialization.
> >
> > This read operation verifies the inode's checksum. This works fine for
> > USRQUOTA and GRPQUOTA inodes because write_reserved_inodes() is always
> > called during ext4 image creation to set appropriate checksums for these
> > reserved inodes.
> >
> > However, the PRJQUOTA inode is not reserved, and its corresponding inode
> > table block may not have been zeroed, potentially containing stale data.
> > Consequently, reading this inode can fail due to a checksum mismatch.
> >
> > This can be reproduced by running the following sequence:
> >
> >   dd if=/dev/random of=$DISK bs=1M count=128
> >   mkfs.ext4 -F -q -b 1024 $DISK 5G
> >   tune2fs -O quota,project $DISK
> >
> > Which results in the following error output:
> >
> >  tune2fs 1.47.3 (8-Jul-2025)
> >  [ERROR] quotaio.c:279:quota_inode_init_new: ex2fs_read_inode failed
> >  [ERROR] quotaio.c:341:quota_file_create: init_new_quota_inode failed
> >  tune2fs: Inode checksum does not match inode while writing quota file (2)
> >
> > While running `kvm-xfstests -c ext4/1k -C 1 generic/383`, the test itself
> > does not fail, but checksum verification failures are reported even
> > without fault injection, which led to discovering this issue.
> >
> > To fix this, we stop attempting to read the quota inode that is about
> > to be initialized inside quota_inode_init_new(). Instead, the logic
> > to attempt truncation of an existing quota inode is moved to be handled
> > inside quota_file_create().
> >
> > Fixes: 080e09b4 ("Add project quota support")
> > Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> > ---
> >  lib/support/quotaio.c | 30 +++++++++++++-----------------
> >  1 file changed, 13 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/support/quotaio.c b/lib/support/quotaio.c
> > index f5f2c7f7..827df85b 100644
> > --- a/lib/support/quotaio.c
> > +++ b/lib/support/quotaio.c
> > @@ -274,18 +274,6 @@ static errcode_t quota_inode_init_new(ext2_filsys fs, ext2_ino_t ino)
> >  	errcode_t err = 0;
> >  	time_t now;
> >  
> > -	err = ext2fs_read_inode(fs, ino, &inode);
> > -	if (err) {
> > -		log_err("ex2fs_read_inode failed");
> > -		return err;
> > -	}
> > -
> > -	if (EXT2_I_SIZE(&inode)) {
> > -		err = quota_inode_truncate(fs, ino);
> > -		if (err)
> > -			return err;
> > -	}
> > -
> >  	memset(&inode, 0, sizeof(struct ext2_inode));
> >  	ext2fs_iblk_set(fs, &inode, 0);
> >  	now = fs->now ? fs->now : time(0);
> > @@ -319,6 +307,10 @@ errcode_t quota_file_create(struct quota_handle *h, ext2_filsys fs,
> >  	if (fmt == -1)
> >  		fmt = QFMT_VFS_V1;
> >  
> > +	err = ext2fs_read_bitmaps(fs);
> > +	if (err)
> > +		goto out_err;
> > +
> >  	h->qh_qf.fs = fs;
> >  	qf_inum = quota_type2inum(qtype, fs->super);
> >  	if (qf_inum == 0 && qtype == PRJQUOTA) {
> > @@ -330,15 +322,19 @@ errcode_t quota_file_create(struct quota_handle *h, ext2_filsys fs,
> >  		ext2fs_mark_ib_dirty(fs);
> >  	} else if (qf_inum == 0) {
> >  		return EXT2_ET_BAD_INODE_NUM;
> > +	} else {
> > +		err = quota_inode_truncate(fs, qf_inum);
> > +		if (err) {
> > +			log_err("quota_inode_truncate failed, ino=%u, type=%d",
> > +				qf_inum, qtype);
> > +			return err;
> > +		}
> >  	}
> >  
> > -	err = ext2fs_read_bitmaps(fs);
> > -	if (err)
> > -		goto out_err;
> > -
> >  	err = quota_inode_init_new(fs, qf_inum);
> >  	if (err) {
> > -		log_err("init_new_quota_inode failed");
> > +		log_err("init_new_quota_inode failed, ino=%u, type=%d",
> > +			qf_inum, qtype);
> >  		goto out_err;
> >  	}
> >  	h->qh_qf.ino = qf_inum;
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH v2 1/1] jbd2: gracefully abort on checkpointing state corruptions
From: Jan Kara @ 2026-03-17 12:28 UTC (permalink / raw)
  To: Milos Nikic
  Cc: jack, tytso, linux-ext4, linux-kernel, Andreas Dilger, Zhang Yi,
	Baokun Li
In-Reply-To: <20260311041548.159424-1-nikic.milos@gmail.com>

On Tue 10-03-26 21:15:48, Milos Nikic wrote:
> This patch targets two internal state machine invariants in checkpoint.c
> residing inside functions that natively return integer error codes.
> 
> - In jbd2_cleanup_journal_tail(): A blocknr of 0 indicates a severely
> corrupted journal superblock. Replaced the J_ASSERT with a WARN_ON_ONCE
> and a graceful journal abort, returning -EFSCORRUPTED.
> 
> - In jbd2_log_do_checkpoint(): Replaced the J_ASSERT_BH checking for
> an unexpected buffer_jwrite state. If the warning triggers, we
> explicitly drop the just-taken get_bh() reference and call __flush_batch()
> to safely clean up any previously queued buffers in the j_chkpt_bhs array,
> preventing a memory leak before returning -EFSCORRUPTED.
> 
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Baokun Li <libaokun@linux.alibaba.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  Changes in v2:
> 
>     Replaced the -EUCLEAN error code with -EFSCORRUPTED to better align with ext4/jbd2 semantics for on-disk metadata inconsistencies (per Baokun's review).
> 
>     Reordered the error path in jbd2_log_do_checkpoint() so that jbd2_journal_abort() is called after __flush_batch(). This ensures cleanly batched buffers are logically flushed before the journal kill switch is flipped.
> 
>     Collected Reviewed-by tags from Andreas Dilger, Zhang Yi, and Baokun Li.
> 
> Changes in v1:
> 
>     Initial implementation converting J_ASSERTs in jbd2_cleanup_journal_tail() and jbd2_log_do_checkpoint() to WARN_ON_ONCE and graceful journal aborts.
> 
>  fs/jbd2/checkpoint.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index de89c5bef607..1508e2f54462 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -267,7 +267,15 @@ int jbd2_log_do_checkpoint(journal_t *journal)
>  			 */
>  			BUFFER_TRACE(bh, "queue");
>  			get_bh(bh);
> -			J_ASSERT_BH(bh, !buffer_jwrite(bh));
> +			if (WARN_ON_ONCE(buffer_jwrite(bh))) {
> +				put_bh(bh); /* drop the ref we just took */
> +				spin_unlock(&journal->j_list_lock);
> +				/* Clean up any previously batched buffers */
> +				if (batch_count)
> +					__flush_batch(journal, &batch_count);
> +				jbd2_journal_abort(journal, -EFSCORRUPTED);
> +				return -EFSCORRUPTED;
> +			}
>  			journal->j_chkpt_bhs[batch_count++] = bh;
>  			transaction->t_chp_stats.cs_written++;
>  			transaction->t_checkpoint_list = jh->b_cpnext;
> @@ -325,7 +333,10 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
>  
>  	if (!jbd2_journal_get_log_tail(journal, &first_tid, &blocknr))
>  		return 1;
> -	J_ASSERT(blocknr != 0);
> +	if (WARN_ON_ONCE(blocknr == 0)) {
> +		jbd2_journal_abort(journal, -EFSCORRUPTED);
> +		return -EFSCORRUPTED;
> +	}
>  
>  	/*
>  	 * We need to make sure that any blocks that were recently written out
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox