[PATCH v2 1/2] ceph/mdsc: Move CEPH_CAP_PIN reference when r_parent is updated

[PATCH v2 1/2] ceph/mdsc: Move CEPH_CAP_PIN reference when r_parent is updated

[Alex Markuze]

When the parent directory lock is not held, req->r_parent can become stale and is updated to point to the correct inode.
However, the associated CEPH_CAP_PIN reference was not being adjusted.
The CEPH_CAP_PIN is a reference on an inode that is tracked for accounting purposes.
Moving this pin is important to keep the accounting balanced. When the pin was not moved from the old parent to the new one, it created two problems:
The reference on the old, stale parent was never released, causing a reference leak.
A reference for the new parent was never acquired, creating the risk of a reference underflow later in ceph_mdsc_release_request().
This patch corrects the logic by releasing the pin from the old parent and acquiring it for the new parent when r_parent is switched.
This ensures reference accounting stays balanced.

Signed-off-by: Alex Markuze <amarkuze@redhat.com>
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
---
 fs/ceph/mds_client.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index ce0c129f4651..4e5926f36e8d 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3053,12 +3053,19 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
 	 */
 	if (!parent_locked && req->r_parent && path_info1.vino.ino &&
 	    ceph_ino(req->r_parent) != path_info1.vino.ino) {
+		struct inode *old_parent = req->r_parent;
 		struct inode *correct_dir = ceph_get_inode(mdsc->fsc->sb, path_info1.vino, NULL);
 		if (!IS_ERR(correct_dir)) {
 			WARN_ONCE(1, "ceph: r_parent mismatch (had %llx wanted %llx) - updating\n",
-				  ceph_ino(req->r_parent), path_info1.vino.ino);
-			iput(req->r_parent);
+			          ceph_ino(old_parent), path_info1.vino.ino);
+			/*
+			 * Transfer CEPH_CAP_PIN from the old parent to the new one.
+			 * The pin was taken earlier in ceph_mdsc_submit_request().
+			 */
+			ceph_put_cap_refs(ceph_inode(old_parent), CEPH_CAP_PIN);
+			iput(old_parent);
 			req->r_parent = correct_dir;
+			ceph_get_cap_refs(ceph_inode(req->r_parent), CEPH_CAP_PIN);
 		}
 	}
 
-- 
2.34.1

Re: [PATCH v2 1/2] ceph/mdsc: Move CEPH_CAP_PIN reference when r_parent is updated

[Viacheslav Dubeyko]

On Thu, 2025-09-04 at 10:11 +0000, Alex Markuze wrote:
> When the parent directory lock is not held, req->r_parent can become stale and is updated to point to the correct inode.
> However, the associated CEPH_CAP_PIN reference was not being adjusted.
> The CEPH_CAP_PIN is a reference on an inode that is tracked for accounting purposes.
> Moving this pin is important to keep the accounting balanced. When the pin was not moved from the old parent to the new one, it created two problems:
> The reference on the old, stale parent was never released, causing a reference leak.
> A reference for the new parent was never acquired, creating the risk of a reference underflow later in ceph_mdsc_release_request().
> This patch corrects the logic by releasing the pin from the old parent and acquiring it for the new parent when r_parent is switched.
> This ensures reference accounting stays balanced.

This explanation looks much cleaner. :)

Thanks,
Slava.

> 
> Signed-off-by: Alex Markuze <amarkuze@redhat.com>
> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> ---
>  fs/ceph/mds_client.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index ce0c129f4651..4e5926f36e8d 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3053,12 +3053,19 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
>  	 */
>  	if (!parent_locked && req->r_parent && path_info1.vino.ino &&
>  	    ceph_ino(req->r_parent) != path_info1.vino.ino) {
> +		struct inode *old_parent = req->r_parent;
>  		struct inode *correct_dir = ceph_get_inode(mdsc->fsc->sb, path_info1.vino, NULL);
>  		if (!IS_ERR(correct_dir)) {
>  			WARN_ONCE(1, "ceph: r_parent mismatch (had %llx wanted %llx) - updating\n",
> -				  ceph_ino(req->r_parent), path_info1.vino.ino);
> -			iput(req->r_parent);
> +			          ceph_ino(old_parent), path_info1.vino.ino);
> +			/*
> +			 * Transfer CEPH_CAP_PIN from the old parent to the new one.
> +			 * The pin was taken earlier in ceph_mdsc_submit_request().
> +			 */
> +			ceph_put_cap_refs(ceph_inode(old_parent), CEPH_CAP_PIN);
> +			iput(old_parent);
>  			req->r_parent = correct_dir;
> +			ceph_get_cap_refs(ceph_inode(req->r_parent), CEPH_CAP_PIN);
>  		}
>  	}
>  

Re: [PATCH v2 1/2] ceph/mdsc: Move CEPH_CAP_PIN reference when r_parent is updated

[Ilya Dryomov]

On Thu, Sep 4, 2025 at 12:11 PM Alex Markuze <amarkuze@redhat.com> wrote:
>
> When the parent directory lock is not held, req->r_parent can become stale and is updated to point to the correct inode.
> However, the associated CEPH_CAP_PIN reference was not being adjusted.
> The CEPH_CAP_PIN is a reference on an inode that is tracked for accounting purposes.
> Moving this pin is important to keep the accounting balanced. When the pin was not moved from the old parent to the new one, it created two problems:
> The reference on the old, stale parent was never released, causing a reference leak.
> A reference for the new parent was never acquired, creating the risk of a reference underflow later in ceph_mdsc_release_request().
> This patch corrects the logic by releasing the pin from the old parent and acquiring it for the new parent when r_parent is switched.
> This ensures reference accounting stays balanced.
>
> Signed-off-by: Alex Markuze <amarkuze@redhat.com>
> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> ---
>  fs/ceph/mds_client.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index ce0c129f4651..4e5926f36e8d 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3053,12 +3053,19 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
>          */
>         if (!parent_locked && req->r_parent && path_info1.vino.ino &&
>             ceph_ino(req->r_parent) != path_info1.vino.ino) {
> +               struct inode *old_parent = req->r_parent;
>                 struct inode *correct_dir = ceph_get_inode(mdsc->fsc->sb, path_info1.vino, NULL);
>                 if (!IS_ERR(correct_dir)) {
>                         WARN_ONCE(1, "ceph: r_parent mismatch (had %llx wanted %llx) - updating\n",
> -                                 ceph_ino(req->r_parent), path_info1.vino.ino);
> -                       iput(req->r_parent);
> +                                 ceph_ino(old_parent), path_info1.vino.ino);
> +                       /*
> +                        * Transfer CEPH_CAP_PIN from the old parent to the new one.
> +                        * The pin was taken earlier in ceph_mdsc_submit_request().
> +                        */
> +                       ceph_put_cap_refs(ceph_inode(old_parent), CEPH_CAP_PIN);
> +                       iput(old_parent);
>                         req->r_parent = correct_dir;
> +                       ceph_get_cap_refs(ceph_inode(req->r_parent), CEPH_CAP_PIN);
>                 }
>         }
>
> --
> 2.34.1
>

Folded into "ceph: fix client race condition validating r_parent before
applying state" and queued up for 6.17-rc6.

Thanks,

                Ilya