On 2020/4/30 10:50, Yan, Zheng wrote:
On Wed, Apr 29, 2020 at 8:49 AM Wu Bo <wubo40@xxxxxxxxxx> wrote:
On 2020/4/28 22:48, Jeff Layton wrote:
On Tue, 2020-04-28 at 21:13 +0800, Wu Bo wrote:
if the ceph_mdsc_open_export_target_session() return fails,
should add a lock to avoid twice unlocking.
Because the lock will be released at the retry or out_unlock tag.
The problem looks real, but...
--
v1 -> v2:
add spin_lock(&ci->i_ceph_lock) before goto out_unlock tag.
Signed-off-by: Wu Bo <wubo40@xxxxxxxxxx>
---
fs/ceph/caps.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 185db76..414c0e2 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -3731,22 +3731,25 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex,
/* open target session */
tsession = ceph_mdsc_open_export_target_session(mdsc, target);
- if (!IS_ERR(tsession)) {
- if (mds > target) {
- mutex_lock(&session->s_mutex);
- mutex_lock_nested(&tsession->s_mutex,
- SINGLE_DEPTH_NESTING);
- } else {
- mutex_lock(&tsession->s_mutex);
- mutex_lock_nested(&session->s_mutex,
- SINGLE_DEPTH_NESTING);
- }
- new_cap = ceph_get_cap(mdsc, NULL);
- } else {
+ if (IS_ERR(tsession)) {
WARN_ON(1);
tsession = NULL;
target = -1;
+ mutex_lock(&session->s_mutex);
+ spin_lock(&ci->i_ceph_lock);
+ goto out_unlock;
Why did you make this case goto out_unlock instead of retrying as it did
before?
If the problem occurs, target = -1, and goto retry lable, you need to
call __get_cap_for_mds() or even call __ceph_remove_cap(), and then jump
to out_unlock lable. All I think is unnecessary, goto out_unlock instead
of retrying directly.
__ceph_remove_cap() must be called even if opening target session
failed. I think adding a mutex_lock(&session->s_mutex) to the
IS_ERR(tsession) block should be enough.
Yes，I will send the V3 patch later.
Thanks.
Wu Bo
+ }
+
+ if (mds > target) {
+ mutex_lock(&session->s_mutex);
+ mutex_lock_nested(&tsession->s_mutex,
+ SINGLE_DEPTH_NESTING);
+ } else {
+ mutex_lock(&tsession->s_mutex);
+ mutex_lock_nested(&session->s_mutex,
+ SINGLE_DEPTH_NESTING);
}
+ new_cap = ceph_get_cap(mdsc, NULL);
goto retry;
out_unlock:
.
