Project

General

Profile

Actions

Bug #38824

closed

kclient: ceph: remove duplicated filelock ref increase

Added by Zhi Zhang about 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
% Done:

0%

Source:
Community (dev)
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Crash signature (v1):
Crash signature (v2):

Description

In kernel client, inode i_filelock_ref is increased in ceph_lock or ceph_flock, but it is increased again in ceph_lock_message. This results in this ref won't become zero. If CEPH_I_ERROR_FILELOCK flag is set in remove_session_caps once, this flag can't be cleared even if client is back to normal. So further file lock will return EIO.

Recently this issue caused massive IO error and IO hang in some of our clusters when user massively used file lock and experienced MDS fault and recovery.

Although the kernel version we used is very old, I checked the latest kernel version and it should have the same issue.


Files

Actions #1

Updated by Zhi Zhang about 5 years ago

  • File 0001-ceph-remove-duplicated-filelock-ref-increase.patch added
From 839bd289dafa941f2eec4f51b116b643a3b83fab Mon Sep 17 00:00:00 2001
From: Zhi Zhang <willzzhang@tencent.com>
Date: Wed, 20 Mar 2019 14:18:41 +0800
Subject: [PATCH] ceph: remove duplicated filelock ref increase

Inode i_filelock_ref is increased in ceph_lock or ceph_flock, but it is
increased again in ceph_lock_message. This results in this ref won't
become zero. If CEPH_I_ERROR_FILELOCK flag is set in
remove_session_caps once, this flag can't be cleared even if client is
back to normal. So further file lock will return EIO.

Signed-off-by: Zhi Zhang <zhang.david2011@gmail.com>
---
 fs/ceph/locks.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
index 9dae2ec..2785390 100644
--- a/fs/ceph/locks.c
+++ b/fs/ceph/locks.c
@@ -65,17 +65,6 @@ static int ceph_lock_message(u8 lock_type, u16 operation, struct inode *inode,
        u64 length = 0;
        u64 owner;

-       if (operation == CEPH_MDS_OP_SETFILELOCK) {
-               /*
-                * increasing i_filelock_ref closes race window between
-                * handling request reply and adding file_lock struct to
-                * inode. Otherwise, auth caps may get trimmed in the
-                * window. Caller function will decrease the counter.
-                */
-               fl->fl_ops = &ceph_fl_lock_ops;
-               atomic_inc(&ceph_inode(inode)->i_filelock_ref);
-       }
-
        if (operation != CEPH_MDS_OP_SETFILELOCK || cmd == CEPH_LOCK_UNLOCK)
                wait = 0;

-- 
1.8.3.1

Hi Yan,

Pls take a look at this fix. Thanks.

Actions #2

Updated by Zheng Yan about 5 years ago

should remove code in ceph_lock().

Actions #3

Updated by Zhi Zhang about 5 years ago

  • File deleted (0001-ceph-remove-duplicated-filelock-ref-increase.patch)
Actions #4

Updated by Zhi Zhang about 5 years ago

Yan, thanks for your review and comments. It is not right to remove the ref increase code in ceph_lock_message. Pls review the latest changes.

From 76ddd7ca3e0a20888d85ebaad129e5a95c39fd99 Mon Sep 17 00:00:00 2001
From: Zhi Zhang <willzzhang@tencent.com>
Date: Fri, 22 Mar 2019 14:16:33 +0800
Subject: [PATCH] ceph: remove duplicated filelock ref increase

Inode i_filelock_ref is increased in ceph_lock or ceph_flock, but it is
increased again in ceph_lock_message. This results in this ref won't
become zero. If CEPH_I_ERROR_FILELOCK flag is set in
remove_session_caps once, this flag can't be cleared even if client is
back to normal. So further file lock will return EIO.

Signed-off-by: Zhi Zhang <zhang.david2011@gmail.com>
---
 fs/ceph/locks.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
index 9dae2ec..ac9b53b 100644
--- a/fs/ceph/locks.c
+++ b/fs/ceph/locks.c
@@ -237,15 +237,6 @@ int ceph_lock(struct file *file, int cmd, struct file_lock *fl)
        spin_lock(&ci->i_ceph_lock);
        if (ci->i_ceph_flags & CEPH_I_ERROR_FILELOCK) {
                err = -EIO;
-       } else if (op == CEPH_MDS_OP_SETFILELOCK) {
-               /*
-                * increasing i_filelock_ref closes race window between
-                * handling request reply and adding file_lock struct to
-                * inode. Otherwise, i_auth_cap may get trimmed in the
-                * window. Caller function will decrease the counter.
-                */
-               fl->fl_ops = &ceph_fl_lock_ops;
-               atomic_inc(&ci->i_filelock_ref);
        }
        spin_unlock(&ci->i_ceph_lock);
        if (err < 0) {
@@ -299,10 +290,6 @@ int ceph_flock(struct file *file, int cmd, struct file_lock *fl)
        spin_lock(&ci->i_ceph_lock);
        if (ci->i_ceph_flags & CEPH_I_ERROR_FILELOCK) {
                err = -EIO;
-       } else {
-               /* see comment in ceph_lock */
-               fl->fl_ops = &ceph_fl_lock_ops;
-               atomic_inc(&ci->i_filelock_ref);
        }
        spin_unlock(&ci->i_ceph_lock);
        if (err < 0) {
-- 
1.8.3.1
Actions #6

Updated by Zheng Yan about 5 years ago

  • Status changed from New to 7

applied

Actions #7

Updated by Patrick Donnelly about 5 years ago

  • Project changed from CephFS to Linux kernel client
  • Category deleted (Correctness/Safety)
  • Assignee changed from Zheng Yan to Zhi Zhang
Actions #8

Updated by Zheng Yan almost 5 years ago

  • Status changed from 7 to Resolved
Actions

Also available in: Atom PDF