| Message ID | 20231117053627.716877-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ceph: fix deadlock or deadcode of misusing dget() | expand |
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>
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);