Re: [PATCH 0/2] Fix two regressions from start_creating()/start_removing() conversion

Re: [PATCH 0/2] Fix two regressions from start_creating()/start_removing() conversion

[Amir Goldstein]

On Tue, Dec 23, 2025 at 9:42 PM Tyler Hicks <code@tyhicks.com> wrote:
>
> When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats
> from every test and that the umount utility was segfaulting when tearing
> down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
> use new start_creating/start_removing APIs").
>
> This patch series addresses that regression and also a mknod problem
> spotted during code review.
>

Ouch!

Christian,

In retrospect, it's a shame that patches get merged with zero test coverage
and no ACK from the maintainer.

OTOH, relying on ACKs from all fs maintainers will seriously impair
the ability to make vfs wide changes like this one.

Feels like we need to find a better balance.

At least for ecryptfs, if we know that Tyler is at least testing rc1
regularly (?) that's a comfort.

Thanks,
Amir.

Re: [PATCH 2/2] ecryptfs: Release lower parent dentry after creating dir

[Amir Goldstein]

On Tue, Dec 23, 2025 at 9:42 PM Tyler Hicks <code@tyhicks.com> wrote:
>
> Fix a mkdir-induced usage count imbalance that tripped a umount_check()
> BUG while unmounting the lower filesystem. Commit f046fbb4d81d
> ("ecryptfs: use new start_creating/start_removing APIs") added a new
> dget() of the lower parent dir, in ecryptfs_mkdir(), but did not dput()
> the dentry before returning from that function.
>
> The BUG output as seen while running the eCryptfs test suite:
>
> $ ./run_tests.sh -b 131072 -c safe,destructive -f ext4 -K -t lp-926292.sh
> ...
> Running eCryptfs filesystem tests on ext4
> lp-926292
> ------------[ cut here ]------------
> BUG: Dentry ffff8e6692d11988{i=c,n=ECRYPTFS_FNEK_ENCRYPTED.FXZuRGZL7QAFtER.JeA46DtdKqkkQx9H2Vpmv234J5CU8YSsrUwZJK4AbXbrN5WkZ348wnqstovKKxA-}  still in use (1) [unmount of ext4 loop0]
> WARNING: CPU: 7 PID: 950 at fs/dcache.c:1590 umount_check+0x5e/0x80
> Modules linked in: md5 libmd5 ecryptfs encrypted_keys ext4 crc16 mbcache jbd2
> CPU: 7 UID: 0 PID: 950 Comm: umount Not tainted 6.18.0-rc1-00013-gf046fbb4d81d #17 PREEMPT(full)
> Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> RIP: 0010:umount_check+0x5e/0x80
> Code: 88 38 06 00 00 48 8b 40 28 4c 8b 08 48 8b 46 68 48 85 c0 74 04 48 8b 50 38 51 48 c7 c7 60 32 9c b5 48 89 f1 e8 43 5e ca ff 90 <0f> 0b 90 90 58 31 c0 e9 46 9d 6c 00 41 83 f8 01 75 b8 eb a3 66 66
> RSP: 0018:ffffa19940c4bdd0 EFLAGS: 00010282
> RAX: 0000000000000000 RBX: ffff8e6692fad4c0 RCX: 0000000000000000
> RDX: 0000000000000004 RSI: ffffa19940c4bc70 RDI: 00000000ffffffff
> RBP: ffffffffb4eb5930 R08: 00000000ffffdfff R09: 0000000000000001
> R10: 00000000ffffdfff R11: ffffffffb5c8a9e0 R12: ffff8e6692fad4c0
> R13: ffff8e6692fad4c0 R14: ffff8e6692d11a40 R15: ffff8e6692d11988
> FS:  00007f6b4b491800(0000) GS:ffff8e670506e000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f6b4b5f8d40 CR3: 0000000114eb7001 CR4: 0000000000772ef0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  d_walk+0xfd/0x370
>  shrink_dcache_for_umount+0x4d/0x140
>  generic_shutdown_super+0x20/0x160
>  kill_block_super+0x1a/0x40
>  ext4_kill_sb+0x22/0x40 [ext4]
>  deactivate_locked_super+0x33/0xa0
>  cleanup_mnt+0xba/0x150
>  task_work_run+0x5c/0xa0
>  exit_to_user_mode_loop+0xac/0xb0
>  do_syscall_64+0x2ab/0xfa0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f6b4b6c2a2b
> Code: c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 f3 0f 1e fa 31 f6 e9 05 00 00 00 0f 1f 44 00 00 f3 0f 1e fa b8 a6 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 b9 83 0d 00 f7 d8
> RSP: 002b:00007ffcd5b8b498 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
> RAX: 0000000000000000 RBX: 000055b84af0b9e0 RCX: 00007f6b4b6c2a2b
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000055b84af0bdf0
> RBP: 00007ffcd5b8b570 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000103 R11: 0000000000000246 R12: 000055b84af0bae0
> R13: 0000000000000000 R14: 000055b84af0bdf0 R15: 0000000000000000
>  </TASK>
> ---[ end trace 0000000000000000 ]---
> EXT4-fs (loop0): unmounting filesystem 00d9ea41-f61e-43d0-a449-6be03e7e8428.
> EXT4-fs (loop0): sb orphan head is 12
> sb_info orphan list:
>   inode loop0:12 at ffff8e66950e1df0: mode 40700, nlink 0, next 0
> Assertion failure in ext4_put_super() at fs/ext4/super.c:1345: 'list_empty(&sbi->s_orphan)'
>
> Fixes: f046fbb4d81d ("ecryptfs: use new start_creating/start_removing APIs")
> Signed-off-by: Tyler Hicks <code@tyhicks.com>
> ---
>  fs/ecryptfs/inode.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index e73d9de676a6..8ab014db3e03 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -533,6 +533,7 @@ static struct dentry *ecryptfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>         fsstack_copy_inode_size(dir, lower_dir);
>         set_nlink(dir, lower_dir->i_nlink);
>  out:
> +       dput(lower_dir_dentry);
>         end_creating(lower_dentry);
>         if (d_really_is_negative(dentry))
>                 d_drop(dentry);
> --
> 2.43.0
>

Damn! I doubt that these bugs would have gone unnoticed by
LLM reviews... :-/

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

Re: [PATCH 0/2] Fix two regressions from start_creating()/start_removing() conversion

[Tyler Hicks]

On 2025-12-24 07:31:59, Amir Goldstein wrote:
> On Tue, Dec 23, 2025 at 9:42 PM Tyler Hicks <code@tyhicks.com> wrote:
> >
> > When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats
> > from every test and that the umount utility was segfaulting when tearing
> > down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
> > use new start_creating/start_removing APIs").
> >
> > This patch series addresses that regression and also a mknod problem
> > spotted during code review.
> >
> 
> Ouch!
> 
> Christian,
> 
> In retrospect, it's a shame that patches get merged with zero test coverage
> and no ACK from the maintainer.

I wasn't able to be a very active maintainer over the last year. I think
Christian did the right thing here.

> OTOH, relying on ACKs from all fs maintainers will seriously impair
> the ability to make vfs wide changes like this one.

Exactly. The fringe filesystems shouldn't slow down the entire VFS.

> Feels like we need to find a better balance.
> 
> At least for ecryptfs, if we know that Tyler is at least testing rc1
> regularly (?) that's a comfort.

I will be more active going forward and now have an easy setup for
testing rc1's regularly.

Tyler

> 
> Thanks,
> Amir.