ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

PATCH New alex chen 4 participants 10 messages Mar 30, 2015 – Apr 22, 2015 OCFS2

Share & tools

alex chen

author

2015-03-30 03:22 UTC #0 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

If ocfs2 lockres has not been initialized before calling ocfs2_dlm_lock, the lock won't be dropped and then will lead umount hung. The case is described below:

ocfs2_mknod
ocfs2_mknod_locked
__ocfs2_mknod_locked
ocfs2_journal_access_di
Failed because of -ENOMEM or other reasons, the inode lockres has not been initialized yet.

iput(inode)
ocfs2_evict_inode
ocfs2_delete_inode
ocfs2_inode_lock
ocfs2_inode_lock_full_nested
__ocfs2_cluster_lock
Succeeds and allocates a new dlm lockres.
ocfs2_clear_inode
ocfs2_open_unlock
ocfs2_drop_inode_locks
ocfs2_drop_lock
Since lockres has not been initialized, the lock can't be dropped and the lockres can't be migrated, thus umount will hang forever.

Signed-off-by Alex Chen <[email protected]>

Reviewed-by Joseph Qi <[email protected]>

Reviewed-by joyce.xue <[email protected]>

---
 fs/ocfs2/dlmglue.c | 5 +++++
 1 file changed, 5 insertions(+)

fs/ocfs2/dlmglue.c +5 -0

index 11849a4..8b23aa2 100644
@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}
+
 	ocfs2_init_mask_waiter(&mw);
 
 	if (lockres->l_ops->flags & LOCK_TYPE_USES_LVB)

Signature · 1 line

1.8.4.3

Andrew Morton

2015-03-31 22:13 UTC #1 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

fs/ocfs2/dlmglue.c static int __ocfs2_cluster_lock(struct ocfs2_super *osb, · line 1397

Quoted 10 lines · On Mon, 30 Mar 2015 11:22:13 +0800 alex chen <[email protected]> wrote:

fs/ocfs2/dlmglue.c +4 -0

@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}

hm. How about we do this?

From: Andrew Morton <[email protected]>
Subject: ocfs2: make mlog_errno return the errno

ocfs2 does

	mlog_errno(v);
	return v;

in many places. Change mlog_errno() so we can do

	return mlog_errno(v);

For some weird reason this patch reduces the size of ocfs2 by 6k:

akpm3:/usr/src/25> size fs/ocfs2/ocfs2.ko
text data bss dec hex filename

1146613   82767  832192 2061572  1f7504 fs/ocfs2/ocfs2.ko-before
1140857   82767  832192 2055816  1f5e88 fs/ocfs2/ocfs2.ko-after

Cc Mark Fasheh <[email protected]>

Cc Joel Becker <[email protected]>

Cc alex chen <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

---

 fs/ocfs2/cluster/masklog.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

fs/ocfs2/cluster/masklog.h renamed from fs/ocfs2/cluster/masklog.h~a +3 -2

diff -puN fs/ocfs2/cluster/masklog.h~a fs/ocfs2/cluster/masklog.h
@@ -196,13 +196,14 @@ extern struct mlog_bits mlog_and_bits, m
 	}								\
 } while (0)
 
-#define mlog_errno(st) do {						\
+#define mlog_errno(st) ({						\
 	int _st = (st);							\
 	if (_st != -ERESTARTSYS && _st != -EINTR &&			\
 	    _st != AOP_TRUNCATED_PAGE && _st != -ENOSPC &&		\
 	    _st != -EDQUOT)						\
 		mlog(ML_ERROR, "status = %lld\n", (long long)_st);	\
-} while (0)
+	st;								\
+})
 
 #define mlog_bug_on_msg(cond, fmt, args...) do {			\
 	if (cond) {							\

Joseph Qi

2015-04-01 00:43 UTC #2 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Hi Andrew,

Quoted 19 lines · Andrew Morton

fs/ocfs2/dlmglue.c static int __ocfs2_cluster_lock(struct ocfs2_super *osb, · line 1397
Quoted 10 lines · On Mon, 30 Mar 2015 11:22:13 +0800 alex chen <[email protected]> wrote:
fs/ocfs2/dlmglue.c +4 -0
@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}
hm. How about we do this?
From: Andrew Morton <[email protected]>
Subject: ocfs2: make mlog_errno return the errno

Quoted 9 lines

ocfs2 does
	mlog_errno(v);
	return v;
in many places. Change mlog_errno() so we can do
	return mlog_errno(v);

I don't think this is fit for all.
In many places it should do cleanup rather than just return the error code.

Quoted 41 lines

For some weird reason this patch reduces the size of ocfs2 by 6k:
akpm3:/usr/src/25> size fs/ocfs2/ocfs2.ko
text data bss dec hex filename
1146613   82767  832192 2061572  1f7504 fs/ocfs2/ocfs2.ko-before
1140857   82767  832192 2055816  1f5e88 fs/ocfs2/ocfs2.ko-after
Cc Mark Fasheh <[email protected]>
Cc Joel Becker <[email protected]>
Cc alex chen <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
---
 fs/ocfs2/cluster/masklog.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
diff -puN fs/ocfs2/cluster/masklog.h~a fs/ocfs2/cluster/masklog.h
fs/ocfs2/cluster/masklog.h renamed from fs/ocfs2/cluster/masklog.h~a +3 -2
@@ -196,13 +196,14 @@ extern struct mlog_bits mlog_and_bits, m
 	}								\
 } while (0)
 
-#define mlog_errno(st) do {						\
+#define mlog_errno(st) ({						\
 	int _st = (st);							\
 	if (_st != -ERESTARTSYS && _st != -EINTR &&			\
 	    _st != AOP_TRUNCATED_PAGE && _st != -ENOSPC &&		\
 	    _st != -EDQUOT)						\
 		mlog(ML_ERROR, "status = %lld\n", (long long)_st);	\
-} while (0)
+	st;								\
+})
 
 #define mlog_bug_on_msg(cond, fmt, args...) do {			\
 	if (cond) {							\
_
.

Andrew Morton

2015-04-01 02:19 UTC #3 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Quoted 8 lines · On Wed, 1 Apr 2015 08:43:45 +0800 Joseph Qi <[email protected]> wrote:

Quoted 8 lines
From: Andrew Morton <[email protected]>
Subject: ocfs2: make mlog_errno return the errno
ocfs2 does
	mlog_errno(v);
	return v;

Quoted 7 lines

Quoted 4 lines
in many places. Change mlog_errno() so we can do
	return mlog_errno(v);
I don't think this is fit for all.
In many places it should do cleanup rather than just return the error code.

There are about 50 sites which can use this.

alex chen

author

2015-04-02 03:14 UTC #4 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Hi Andrew,

Quoted 12 lines · Andrew Morton

Quoted 10 lines · On Wed, 1 Apr 2015 08:43:45 +0800 Joseph Qi <[email protected]> wrote:
Quoted 10 lines
From: Andrew Morton <[email protected]>
Subject: ocfs2: make mlog_errno return the errno
ocfs2 does
	mlog_errno(v);
	return v;
in many places. Change mlog_errno() so we can do

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
	return mlog_errno(v);
I don't think this is fit for all.
In many places it should do cleanup rather than just return the error code.
There are about 50 sites which can use this.

Can we define a new macro 'mlog_errno_return' as described below ? In addition, ocfs2 does

	if (v)
		mlog_errno(v);
	return v;

in some places. In order to deal with this situation we can judge if 'st' is not equal to zero before printing log.

Thanks
Alex

---
 fs/ocfs2/cluster/masklog.h | 9 +++++++++
 1 file changed, 9 insertions(+)

fs/ocfs2/cluster/masklog.h +9 -0

index 2260fb9..269fef9 100644
@@ -204,6 +204,15 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
 		mlog(ML_ERROR, "status = %lld\n", (long long)_st);	\
 } while (0)
 
+#define mlog_errno_return(st) ({					\
+	int _st = (st);							\
+	if (_st != 0 && _st != -ERESTARTSYS && _st != -EINTR &&		\
+	    _st != AOP_TRUNCATED_PAGE && _st != -ENOSPC &&		\
+	    _st != -EDQUOT)						\
+		mlog(ML_ERROR, "status = %lld\n", (long long)_st);	\
+	st;								\
+})
+
 #define mlog_bug_on_msg(cond, fmt, args...) do {			\
 	if (cond) {							\
 		mlog(ML_ERROR, "bug expression: " #cond "\n");		\

Signature · 1 line

1.8.4.3

Andrew Morton

2015-04-02 03:45 UTC #5 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Quoted 8 lines · On Thu, 2 Apr 2015 11:14:38 +0800 alex chen <[email protected]> wrote:

Quoted 7 lines
Quoted 4 lines
Quoted 1 line
I don't think this is fit for all.
In many places it should do cleanup rather than just return the error code.
There are about 50 sites which can use this.

Quoted 7 lines

Can we define a new macro 'mlog_errno_return' as described below ? In addition, ocfs2 does
	if (v)
		mlog_errno(v);
	return v;
in some places. In order to deal with this situation we can judge if 'st' is not equal to zero before printing log.

Macros which hide control flow are evil, although in this case the "return" in the name gives people info about what's happening.

But why bother? This:

fs/ocfs2/cluster/masklog.h renamed from fs/ocfs2/cluster/masklog.h~ocfs2-make-mlog_errno-return-the-errno +3 -2

@@ -196,13 +196,14 @@ extern struct mlog_bits mlog_and_bits, m
 	}								\
 } while (0)
 
-#define mlog_errno(st) do {						\
+#define mlog_errno(st) ({						\
 	int _st = (st);							\
 	if (_st != -ERESTARTSYS && _st != -EINTR &&			\
 	    _st != AOP_TRUNCATED_PAGE && _st != -ENOSPC &&		\
 	    _st != -EDQUOT)						\
 		mlog(ML_ERROR, "status = %lld\n", (long long)_st);	\
-} while (0)
+	st;								\
+})
 
 #define mlog_bug_on_msg(cond, fmt, args...) do {			\
 	if (cond) {							\

is clean, idiomatic and works great. And with no other change it shrinks the fs object code by 6k.

Gad, mlog_errno() is a *huge* source of bloat. Look:

fs/ocfs2/cluster/masklog.h renamed from fs/ocfs2/cluster/masklog.h~a +2 ~~-20~~

@@ -183,27 +183,9 @@ extern struct mlog_bits mlog_and_bits, m
 	       task_pid_nr(current), __mlog_cpu_guess,			\
 	       __PRETTY_FUNCTION__, __LINE__ , ##args)
 
-#define mlog(mask, fmt, args...) do {					\
-	u64 __m = MLOG_MASK_PREFIX | (mask);				\
-	if ((__m & ML_ALLOWED_BITS) &&					\
-	    __mlog_test_u64(__m, mlog_and_bits) &&			\
-	    !__mlog_test_u64(__m, mlog_not_bits)) {			\
-		if (__m & ML_ERROR)					\
-			__mlog_printk(KERN_ERR, "ERROR: "fmt , ##args);	\
-		else if (__m & ML_NOTICE)				\
-			__mlog_printk(KERN_NOTICE, fmt , ##args);	\
-		else __mlog_printk(KERN_INFO, fmt , ##args);		\
-	}								\
-} while (0)
+#define mlog(mask, fmt, args...) do { } while (0)
 
-#define mlog_errno(st) ({						\
-	int _st = (st);							\
-	if (_st != -ERESTARTSYS && _st != -EINTR &&			\
-	    _st != AOP_TRUNCATED_PAGE && _st != -ENOSPC &&		\
-	    _st != -EDQUOT)						\
-		mlog(ML_ERROR, "status = %lld\n", (long long)_st);	\
-	st;								\
-})
+#define mlog_errno(st) do { } while (0)
 
 #define mlog_bug_on_msg(cond, fmt, args...) do {			\
 	if (cond) {							\

z:/usr/src/25> size fs/ocfs2/ocfs2.ko
text data bss dec hex filename

1140849   82767  832192 2055808  1f5e80 fs/ocfs2/ocfs2.ko-before
 675402   82767  226104  984273   f04d1 fs/ocfs2/ocfs2.ko

It almost doubles the size of the object code!

Someone, please put this thing on a diet. It doesn't all need to be inlined. We could just do

extern void __mlog_errno(int st);
#define mlog_errno(st) __mlog_errno(st, __LINE__)

and save hundreds of kilobytes of text. It'll be faster too, due to the reduced instruction cache footprint.

Junxiao Bi

2015-04-16 07:28 UTC #6 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

In reply to alex chen

Hi Alex,

Quoted 10 lines · alex chen

If ocfs2 lockres has not been initialized before calling ocfs2_dlm_lock, the lock won't be dropped and then will lead umount hung. The case is described below:
ocfs2_mknod
ocfs2_mknod_locked
__ocfs2_mknod_locked
ocfs2_journal_access_di
Failed because of -ENOMEM or other reasons, the inode lockres has not been initialized yet.

If failed here, is OCFS2_I(inode)->ip_inode_lockres initialized? If not how can you break __ocfs2_cluster_lock with the following condition?

if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED))

Thanks,
Junxiao.

Quoted 41 lines

iput(inode)
ocfs2_evict_inode
ocfs2_delete_inode
ocfs2_inode_lock
ocfs2_inode_lock_full_nested
__ocfs2_cluster_lock
Succeeds and allocates a new dlm lockres.
ocfs2_clear_inode
ocfs2_open_unlock
ocfs2_drop_inode_locks
ocfs2_drop_lock
Since lockres has not been initialized, the lock can't be dropped and the lockres can't be migrated, thus umount will hang forever.
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
Reviewed-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlmglue.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/dlmglue.c +5 -0
index 11849a4..8b23aa2 100644
@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}
+
 	ocfs2_init_mask_waiter(&mw);
 
 	if (lockres->l_ops->flags & LOCK_TYPE_USES_LVB)

alex chen

author

2015-04-21 02:54 UTC #7 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Hi Junxiao,

Quoted 7 lines · Junxiao Bi

Hi Alex,
Quoted 4 lines · alex chen
If ocfs2 lockres has not been initialized before calling ocfs2_dlm_lock, the lock won't be dropped and then will lead umount hung. The case is described below:

Quoted 8 lines

Quoted 6 lines
ocfs2_mknod
ocfs2_mknod_locked
__ocfs2_mknod_locked
ocfs2_journal_access_di
Failed because of -ENOMEM or other reasons, the inode lockres has not been initialized yet.
If failed here, is OCFS2_I(inode)->ip_inode_lockres initialized? If not

The OCFS2_I(inode)->ip_inode_lockres is initialized as follows:
__ocfs2_mknod_locked
ocfs2_populate_inode
ocfs2_inode_lock_res_init
ocfs2_lock_res_init_common
So if ocfs2_journal_access_di is failed, the ip_inode_lockres will not be initialized.
In this situation, we should not allocate a new dlm lockres through calling ocfs2_dlm_lock() in __ocfs2_cluster_lock(), otherwise it will lead umount hung. So we need bread __ocfs2_cluster_lock() if the inode lockres is not be initialized, that is the condition
(!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) is TRUE.

Quoted 52 lines

how can you break __ocfs2_cluster_lock with the following condition?
if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED))
Thanks,
Junxiao.
Quoted 31 lines
iput(inode)
ocfs2_evict_inode
ocfs2_delete_inode
ocfs2_inode_lock
ocfs2_inode_lock_full_nested
__ocfs2_cluster_lock
Succeeds and allocates a new dlm lockres.
ocfs2_clear_inode
ocfs2_open_unlock
ocfs2_drop_inode_locks
ocfs2_drop_lock
Since lockres has not been initialized, the lock can't be dropped and the lockres can't be migrated, thus umount will hang forever.
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
Reviewed-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlmglue.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/dlmglue.c +0 -0
index 11849a4..8b23aa2 100644
@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
fs/ocfs2/dlmglue.c static int __ocfs2_cluster_lock(struct ocfs2_super *osb, · line 1401
Quoted 10 lines
fs/ocfs2/dlmglue.c +5 -0
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}
+
 	ocfs2_init_mask_waiter(&mw);
 
 	if (lockres->l_ops->flags & LOCK_TYPE_USES_LVB)
.

Junxiao Bi

2015-04-22 01:46 UTC #8 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Quoted 19 lines · alex chen

Hi Junxiao,
Quoted 7 lines · Junxiao Bi
Hi Alex,
Quoted 4 lines · alex chen
If ocfs2 lockres has not been initialized before calling ocfs2_dlm_lock, the lock won't be dropped and then will lead umount hung. The case is described below:
Quoted 8 lines
Quoted 6 lines
ocfs2_mknod
ocfs2_mknod_locked
__ocfs2_mknod_locked
ocfs2_journal_access_di
Failed because of -ENOMEM or other reasons, the inode lockres has not been initialized yet.
If failed here, is OCFS2_I(inode)->ip_inode_lockres initialized? If not

Quoted 12 lines

The OCFS2_I(inode)->ip_inode_lockres is initialized as follows:
__ocfs2_mknod_locked
ocfs2_populate_inode
ocfs2_inode_lock_res_init
ocfs2_lock_res_init_common
So if ocfs2_journal_access_di is failed, the ip_inode_lockres will not be initialized.
In this situation, we should not allocate a new dlm lockres through calling ocfs2_dlm_lock() in __ocfs2_cluster_lock(), otherwise it will lead umount hung. So we need bread __ocfs2_cluster_lock() if the inode lockres is not be initialized, that is the condition
(!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) is TRUE.

Looks good. Just didn't remind that inode_info is init once when it is allocated from "ocfs2_inode_cache". lockres->l_flags is init there.

Thanks,
Junxiao.

Quoted 54 lines

Quoted 53 lines
how can you break __ocfs2_cluster_lock with the following condition?
if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED))
Thanks,
Junxiao.
Quoted 31 lines
iput(inode)
ocfs2_evict_inode
ocfs2_delete_inode
ocfs2_inode_lock
ocfs2_inode_lock_full_nested
__ocfs2_cluster_lock
Succeeds and allocates a new dlm lockres.
ocfs2_clear_inode
ocfs2_open_unlock
ocfs2_drop_inode_locks
ocfs2_drop_lock
Since lockres has not been initialized, the lock can't be dropped and the lockres can't be migrated, thus umount will hang forever.
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
Reviewed-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlmglue.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/dlmglue.c +0 -0
index 11849a4..8b23aa2 100644
@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
fs/ocfs2/dlmglue.c static int __ocfs2_cluster_lock(struct ocfs2_super *osb, · line 1401
Quoted 10 lines
fs/ocfs2/dlmglue.c +5 -0
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}
+
 	ocfs2_init_mask_waiter(&mw);
 
 	if (lockres->l_ops->flags & LOCK_TYPE_USES_LVB)
.

alex chen

author

2015-04-22 08:14 UTC #9 lore raw

PATCH 1/1 ocfs2: check if the ocfs2 lock resource be initialized before calling ocfs2_dlm_lock

Quoted 33 lines · Junxiao Bi

Quoted 19 lines · alex chen
Hi Junxiao,
Quoted 7 lines · Junxiao Bi
Hi Alex,
Quoted 4 lines · alex chen
If ocfs2 lockres has not been initialized before calling ocfs2_dlm_lock, the lock won't be dropped and then will lead umount hung. The case is described below:
Quoted 8 lines
Quoted 6 lines
ocfs2_mknod
ocfs2_mknod_locked
__ocfs2_mknod_locked
ocfs2_journal_access_di
Failed because of -ENOMEM or other reasons, the inode lockres has not been initialized yet.
If failed here, is OCFS2_I(inode)->ip_inode_lockres initialized? If not
Quoted 12 lines
The OCFS2_I(inode)->ip_inode_lockres is initialized as follows:
__ocfs2_mknod_locked
ocfs2_populate_inode
ocfs2_inode_lock_res_init
ocfs2_lock_res_init_common
So if ocfs2_journal_access_di is failed, the ip_inode_lockres will not be initialized.
In this situation, we should not allocate a new dlm lockres through calling ocfs2_dlm_lock() in __ocfs2_cluster_lock(), otherwise it will lead umount hung. So we need bread __ocfs2_cluster_lock() if the inode lockres is not be initialized, that is the condition
(!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) is TRUE.

Quoted 6 lines

Looks good. Just didn't remind that inode_info is init once when it is allocated from "ocfs2_inode_cache". lockres->l_flags is init there.
Thanks,
Junxiao.

Thank you for your remind. If the subject is "check if lockres->l_flags be initialized to OCFS2_LOCK_INITIALIZED before calling ocfs2_dlm_lock" will be better.

Quoted 58 lines

Quoted 47 lines
Quoted 47 lines
how can you break __ocfs2_cluster_lock with the following condition?
if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED))
Thanks,
Junxiao.
Quoted 39 lines
iput(inode)
ocfs2_evict_inode
ocfs2_delete_inode
ocfs2_inode_lock
ocfs2_inode_lock_full_nested
__ocfs2_cluster_lock
Succeeds and allocates a new dlm lockres.
ocfs2_clear_inode
ocfs2_open_unlock
ocfs2_drop_inode_locks
ocfs2_drop_lock
Since lockres has not been initialized, the lock can't be dropped and the lockres can't be migrated, thus umount will hang forever.
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
Reviewed-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlmglue.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/ocfs2/dlmglue.c +5 -0
index 11849a4..8b23aa2 100644
@@ -1391,6 +1391,11 @@ static int __ocfs2_cluster_lock(struct ocfs2_super *osb,
 	int noqueue_attempted = 0;
 	int dlm_locked = 0;
 
+	if (!(lockres->l_flags & OCFS2_LOCK_INITIALIZED)) {
+		mlog_errno(-EINVAL);
+		return -EINVAL;
+	}
+
 	ocfs2_init_mask_waiter(&mw);
 
Quoted 7 lines
Quoted 6 lines
fs/ocfs2/dlmglue.c static int __ocfs2_cluster_lock(struct ocfs2_super *osb, · line 1401
Quoted 2 lines
fs/ocfs2/dlmglue.c +0 -0
 	if (lockres->l_ops->flags & LOCK_TYPE_USES_LVB)
.
.