From: Jeff Layton <jlayton@kernel.org>
To: xiubli@redhat.com, ceph-devel@vger.kernel.org
Cc: idryomov@gmail.com, vshankar@redhat.com, mchangir@redhat.com,
Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH v2] ceph: fix deadlock or deadcode of misusing dget()
Date: Fri, 17 Nov 2023 05:39:34 -0500 [thread overview]
Message-ID: <42137a50ccb465afe852756185f0c34ccc836a0a.camel@kernel.org> (raw)
In-Reply-To: <20231117053627.716877-1-xiubli@redhat.com>
On Fri, 2023-11-17 at 13:36 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> The lock order is incorrect between denty and its parent, we should
> always make sure that the parent get the lock first.
>
> But since this deadcode is never used and the parent dir will always
> be set from the callers, let's just remove it.
>
> Reported-by: Al Viro <viro@zeniv.linux.org.uk>
> URL: https://www.spinics.net/lists/ceph-devel/msg58622.html
> Cc: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>
>
> V2:
> - Just remove the deadcode. Actually this just removed the deadcode,
> not fixing any deadlock issue and I also just removed ccing the
> stable list.
>
>
> fs/ceph/caps.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 284424659329..a9e19f1f26e0 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -4916,13 +4916,15 @@ int ceph_encode_dentry_release(void **p, struct dentry *dentry,
> struct inode *dir,
> int mds, int drop, int unless)
> {
> - struct dentry *parent = NULL;
> struct ceph_mds_request_release *rel = *p;
> struct ceph_dentry_info *di = ceph_dentry(dentry);
> struct ceph_client *cl;
> int force = 0;
> int ret;
>
> + /* This shouldn't happen */
> + BUG_ON(!dir);
> +
> /*
> * force an record for the directory caps if we have a dentry lease.
> * this is racy (can't take i_ceph_lock and d_lock together), but it
> @@ -4932,14 +4934,9 @@ int ceph_encode_dentry_release(void **p, struct dentry *dentry,
> spin_lock(&dentry->d_lock);
> if (di->lease_session && di->lease_session->s_mds == mds)
> force = 1;
> - if (!dir) {
> - parent = dget(dentry->d_parent);
> - dir = d_inode(parent);
> - }
> spin_unlock(&dentry->d_lock);
>
> ret = ceph_encode_inode_release(p, dir, mds, drop, unless, force);
> - dput(parent);
>
> cl = ceph_inode_to_client(dir);
> spin_lock(&dentry->d_lock);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
prev parent reply other threads:[~2023-11-17 10:39 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-17 5:36 [PATCH v2] ceph: fix deadlock or deadcode of misusing dget() xiubli
2023-11-17 10:39 ` Jeff Layton [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=42137a50ccb465afe852756185f0c34ccc836a0a.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=ceph-devel@vger.kernel.org \
--cc=idryomov@gmail.com \
--cc=mchangir@redhat.com \
--cc=viro@zeniv.linux.org.uk \
--cc=vshankar@redhat.com \
--cc=xiubli@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).