Re: [PATCH] ceph: drop stale dentry when ceph_unlink() gets -ENOENT

[Xiubo Li]

On Tue, Nov 10, 2026 zhangdaolong <dlzhangak@xxxxxxxxxxxxxx> wrote:
> When a synchronous unlink, rmdir, or rmsnap request in ceph_unlink()
> fails with -ENOENT ...

Hi Daolong,

Thanks for your patch.

The change looks fine to me, and it matches what ceph_async_unlink_cb()
already does on failure. But I think the commit message should be
narrowed down.

I tested it with two kernel clients against a vstart cluster, with A and
B removing the same 2000 files (and 2000 dirs) at the same time. The MDS
returned -ENOENT to about 2000 of the requests, but even without this
patch A was left with no stale dentry: stat returns ENOENT, readdir is
empty and every name can be created again. B's unlink revokes A's
dentry lease and the parent's Fs caps before it completes, so A looks
the name up again anyway.

The case that really leaves a stale dentry is .snap: there is no lease
there, so after a failed RMSNAP "stat .snap/s2" still finds the
snapshot and "mkdir .snap/s2" fails with -EEXIST. Your patch fixes that.
With the d_revalidate change from the other patch the RMSNAP is not
even sent any more, so what is left is the window between d_revalidate
and the RMSNAP. Please describe this case in the commit message, and
call the regular-file part hardening rather than a bug fix.

Feel free to add:

Tested-by: Xiubo Li <xiubo.li@xxxxxxxxx>
Reviewed-by: Xiubo Li <xiubo.li@xxxxxxxxx>

Thanks,
- Xiubo

zhangdaolong <dlzhangak@xxxxxxxxxxxxxx> 于2026年11月10日周二 02:31写道：
>
> When a synchronous unlink, rmdir, or rmsnap request in ceph_unlink()
> fails with -ENOENT (for example, because the target was already removed
> on the MDS by another client or a snapshot was already purged before
> the request reached the MDS), the MDS replies without a dentry trace.
>
> Currently, ceph_fill_trace() only invalidates the parent directory when
> result == 0, and neither ceph_unlink() nor the VFS layer unhashes the
> dentry on error. As a result, the stale positive dentry remains hashed
> in the dcache and the parent directory may retain CEPH_I_COMPLETE.
>
> Fix this by clearing the parent directory's completeness and calling
> d_drop(dentry) when ceph_mdsc_do_request() returns -ENOENT in
> ceph_unlink().
>
> Suggested-by: Xiubo Li <xiubli@xxxxxxxxxx>
> Signed-off-by: zhangdaolong <dlzhangak@xxxxxxxxxxxxxx>
> ---
>  fs/ceph/dir.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 6eb5b1cb039b..f4c0521f3bcc 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1473,8 +1473,12 @@ static int ceph_unlink(struct inode *dir, struct dentry *dentry)
>         } else {
>                 set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
>                 err = ceph_mdsc_do_request(mdsc, dir, req);
> -               if (!err && !req->r_reply_info.head->is_dentry)
> +               if (!err && !req->r_reply_info.head->is_dentry) {
>                         d_delete(dentry);
> +               } else if (err == -ENOENT) {
> +                       ceph_dir_clear_complete(dir);
> +                       d_drop(dentry);
> +               }
>         }
>
>         ceph_mdsc_put_request(req);
> --
> 2.48.1
>