On Thu, May 21, 2020 at 3:39 PM <xiubli@xxxxxxxxxx> wrote: > > From: Xiubo Li <xiubli@xxxxxxxxxx> > > In the ceph_check_caps() it may call the session lock/unlock stuff. > > There have some deadlock cases, like: > handle_forward() > ... > mutex_lock(&mdsc->mutex) > ... > ceph_mdsc_put_request() > --> ceph_mdsc_release_request() > --> ceph_put_cap_request() > --> ceph_put_cap_refs() > --> ceph_check_caps() > ... > mutex_unlock(&mdsc->mutex) For this case, it's better to call ceph_mdsc_put_request() after unlock mdsc->mutex > > And also there maybe has some double session lock cases, like: > > send_mds_reconnect() > ... > mutex_lock(&session->s_mutex); > ... > --> replay_unsafe_requests() > --> ceph_mdsc_release_dir_caps() > --> ceph_put_cap_refs() > --> ceph_check_caps() > ... > mutex_unlock(&session->s_mutex); There is no point to check_caps() and send cap message while reconnecting caps. So I think it's better to just skip calling ceph_check_caps() for this case. Regards Yan, Zheng > > URL: https://tracker.ceph.com/issues/45635 > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > --- > fs/ceph/caps.c | 2 +- > fs/ceph/inode.c | 10 ++++++++++ > fs/ceph/super.h | 12 ++++++++++++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index 27c2e60..08194c4 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -3073,7 +3073,7 @@ void ceph_put_cap_refs(struct ceph_inode_info *ci, int had) > last ? " last" : "", put ? " put" : ""); > > if (last) > - ceph_check_caps(ci, 0, NULL); > + ceph_async_check_caps(ci); > else if (flushsnaps) > ceph_flush_snaps(ci, NULL); > if (wake) > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > index 357c937..84a61d4 100644 > --- a/fs/ceph/inode.c > +++ b/fs/ceph/inode.c > @@ -35,6 +35,7 @@ > static const struct inode_operations ceph_symlink_iops; > > static void ceph_inode_work(struct work_struct *work); > +static void ceph_check_caps_work(struct work_struct *work); > > /* > * find or create an inode, given the ceph ino number > @@ -518,6 +519,7 @@ struct inode *ceph_alloc_inode(struct super_block *sb) > INIT_LIST_HEAD(&ci->i_snap_flush_item); > > INIT_WORK(&ci->i_work, ceph_inode_work); > + INIT_WORK(&ci->check_caps_work, ceph_check_caps_work); > ci->i_work_mask = 0; > memset(&ci->i_btime, '\0', sizeof(ci->i_btime)); > > @@ -2012,6 +2014,14 @@ static void ceph_inode_work(struct work_struct *work) > iput(inode); > } > > +static void ceph_check_caps_work(struct work_struct *work) > +{ > + struct ceph_inode_info *ci = container_of(work, struct ceph_inode_info, > + check_caps_work); > + > + ceph_check_caps(ci, 0, NULL); > +} > + > /* > * symlinks > */ > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index 226f19c..96d0e41 100644 > --- a/fs/ceph/super.h > +++ b/fs/ceph/super.h > @@ -421,6 +421,8 @@ struct ceph_inode_info { > struct timespec64 i_btime; > struct timespec64 i_snap_btime; > > + struct work_struct check_caps_work; > + > struct work_struct i_work; > unsigned long i_work_mask; > > @@ -1102,6 +1104,16 @@ extern void ceph_flush_snaps(struct ceph_inode_info *ci, > extern bool __ceph_should_report_size(struct ceph_inode_info *ci); > extern void ceph_check_caps(struct ceph_inode_info *ci, int flags, > struct ceph_mds_session *session); > +static void inline > +ceph_async_check_caps(struct ceph_inode_info *ci) > +{ > + struct inode *inode = &ci->vfs_inode; > + > + /* It's okay if queue_work fails */ > + queue_work(ceph_inode_to_client(inode)->inode_wq, > + &ceph_inode(inode)->check_caps_work); > +} > + > extern void ceph_check_delayed_caps(struct ceph_mds_client *mdsc); > extern void ceph_flush_dirty_caps(struct ceph_mds_client *mdsc); > extern int ceph_drop_caps_for_unlink(struct inode *inode); > -- > 1.8.3.1 >
