Skip to contentSkip to conversation

ocfs2/dlm: call dlm_lockres_put without resource spinlock

PATCH New alex chen 3 participants 3 messages Sep 26, 2014 – Oct 8, 2014 OCFS2

lore
Share & tools
lore

alex chen

author
#0 lore raw

PATCH 1/1 ocfs2/dlm: call dlm_lockres_put without resource spinlock

dlm_lockres_put should be called without &res->spinlock, otherwise a deadlock case may happen.

spin_lock(&res->spinlock)
...
dlm_lockres_put
->dlm_lockres_release

    ->dlm_print_one_lock_resource
      ->spin_lock(&res->spinlock)
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
---
 fs/ocfs2/dlm/dlmrecovery.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
fs/ocfs2/dlm/dlmrecovery.c +5 -2
index 45067fa..3365839 100644
@@ -1710,9 +1710,12 @@ int dlm_master_requery_handler(struct o2net_msg *msg, u32 len, void *data,
 				BUG();
 			} else
 				__dlm_lockres_grab_inflight_worker(dlm, res);
-		} else /* put.. incase we are not the master */
+			spin_unlock(&res->spinlock);
+		} else {
+			/* put.. incase we are not the master */
+			spin_unlock(&res->spinlock);
 			dlm_lockres_put(res);
-		spin_unlock(&res->spinlock);
+		}
 	}
 	spin_unlock(&dlm->spinlock);
Signature · 1 line
1.8.4.3
#1 lore raw

PATCH 1/1 ocfs2/dlm: call dlm_lockres_put without resource spinlock

Quoted 21 lines · On Fri, 26 Sep 2014 16:41:39 +0800 alex chen <[email protected]> wrote:

dlm_lockres_put should be called without &res->spinlock, otherwise a deadlock case may happen.

spin_lock(&res->spinlock)
...
dlm_lockres_put
->dlm_lockres_release

    ->dlm_print_one_lock_resource
      ->spin_lock(&res->spinlock)
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
---
 fs/ocfs2/dlm/dlmrecovery.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index 45067fa..3365839 100644

fs/ocfs2/dlm/dlmrecovery.c int dlm_master_requery_handler(struct o2net_msg *msg, u32 len, void *data, · line 1710

Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c +0 -0
@@ -1710,9 +1710,12 @@ int dlm_master_requery_handler(struct o2net_msg *msg, u32 len, void *data,
 				BUG();

This code does a GFP_ATOMIC allocation attempt and if that fails, it goes BUG().

Guys, GFP_ATOMIC is unreliable. This isn't production quality code :(

#2 lore raw

PATCH 1/1 ocfs2/dlm: call dlm_lockres_put without resource spinlock

Quoted 28 lines · Andrew Morton
Quoted 21 lines · On Fri, 26 Sep 2014 16:41:39 +0800 alex chen <[email protected]> wrote:

dlm_lockres_put should be called without &res->spinlock, otherwise a deadlock case may happen.

spin_lock(&res->spinlock)
...
dlm_lockres_put
->dlm_lockres_release

    ->dlm_print_one_lock_resource
      ->spin_lock(&res->spinlock)
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
---
 fs/ocfs2/dlm/dlmrecovery.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index 45067fa..3365839 100644

fs/ocfs2/dlm/dlmrecovery.c int dlm_master_requery_handler(struct o2net_msg *msg, u32 len, void *data, · line 1710

Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c +0 -0
@@ -1710,9 +1710,12 @@ int dlm_master_requery_handler(struct o2net_msg *msg, u32 len, void *data,
 				BUG();

This code does a GFP_ATOMIC allocation attempt and if that fails, it goes BUG().

Quoted 2 lines

Guys, GFP_ATOMIC is unreliable. This isn't production quality code :(

Last time we talked about this and Wengang sugguested it return an error to the sender and let the sender retry.
I'll take this idea and send a patch.

Quoted 8 lines

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel