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@xxxxxxxxxx>
> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
> ---
> 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);
> }
> }
>
