ocfs2/dlm: fix misuse of list_move_tail() in dlm_run_purge_list()

PATCH Superseded Xue jiufei 2 participants 4 messages Jun 13, 2014 – Jun 16, 2014 OCFS2

Share & tools

Xue jiufei

author

2014-06-13 07:26 UTC #0 lore raw

PATCH 1/1 ocfs2/dlm: fix misuse of list_move_tail() in dlm_run_purge_list()

When a lockres in purge list but is still in use, it should be moved to the tail of purge list. dlm_thread will continue to check next lockres in purge list. However, code list_move_tail(&dlm->purge_list, &lockres->purge) will do *no* movements, so dlm_thread will purge the same lockres in this loop again and again. If it is in use for a long time, other lockres will not be processed.

Signed-off-by Yiwen Jiang <[email protected]>

Signed-off-by joyce.xue <[email protected]>

---
 fs/ocfs2/dlm/dlmthread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/ocfs2/dlm/dlmthread.c +1 -1

index 9db869d..ce017de 100644
@@ -264,7 +264,7 @@ static void dlm_run_purge_list(struct dlm_ctxt *dlm,
 			     "used %d, state %d\n", dlm->name,
 			     lockres->lockname.len, lockres->lockname.name,
 			     !unused, lockres->state);
-			list_move_tail(&dlm->purge_list, &lockres->purge);
+			list_move_tail(&lockres->purge, dlm->purge_list);
 			spin_unlock(&lockres->spinlock);
 			continue;
 		}

Signature · 1 line

1.8.3.4

Mark Fasheh

2014-06-13 21:16 UTC #1 lore raw

PATCH 1/1 ocfs2/dlm: fix misuse of list_move_tail() in dlm_run_purge_list()

Quoted 18 lines · Xue jiufei

When a lockres in purge list but is still in use, it should be moved to the tail of purge list. dlm_thread will continue to check next lockres in purge list. However, code list_move_tail(&dlm->purge_list, &lockres->purge) will do *no* movements, so dlm_thread will purge the same lockres in this loop again and again. If it is in use for a long time, other lockres will not be processed.
Signed-off-by Yiwen Jiang <[email protected]>
Signed-off-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlm/dlmthread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/dlm/dlmthread.c +0 -0
index 9db869d..ce017de 100644

fs/ocfs2/dlm/dlmthread.c static void dlm_run_purge_list(struct dlm_ctxt *dlm, · line 267

Quoted 5 lines

fs/ocfs2/dlm/dlmthread.c +1 -1

@@ -264,7 +264,7 @@ static void dlm_run_purge_list(struct dlm_ctxt *dlm,
 			     "used %d, state %d\n", dlm->name,
 			     lockres->lockname.len, lockres->lockname.name,
 			     !unused, lockres->state);
-			list_move_tail(&dlm->purge_list, &lockres->purge);
+			list_move_tail(&lockres->purge, dlm->purge_list);

Erf, good catch!

But this wants to be:

list_move_tail(&lockres->purge, &dlm->purge_list);
^^^^
Notice the extra '&'

correct?

	--Mark

--
Mark Fasheh

Xue jiufei

author

2014-06-16 01:20 UTC #2 lore raw

PATCH 1/1 ocfs2/dlm: fix misuse of list_move_tail() in dlm_run_purge_list()

Quoted 27 lines · Mark Fasheh

Quoted 18 lines · Xue jiufei
When a lockres in purge list but is still in use, it should be moved to the tail of purge list. dlm_thread will continue to check next lockres in purge list. However, code list_move_tail(&dlm->purge_list, &lockres->purge) will do *no* movements, so dlm_thread will purge the same lockres in this loop again and again. If it is in use for a long time, other lockres will not be processed.
Signed-off-by Yiwen Jiang <[email protected]>
Signed-off-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlm/dlmthread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/dlm/dlmthread.c +0 -0
index 9db869d..ce017de 100644
fs/ocfs2/dlm/dlmthread.c static void dlm_run_purge_list(struct dlm_ctxt *dlm, · line 267
Quoted 5 lines
fs/ocfs2/dlm/dlmthread.c +1 -1
@@ -264,7 +264,7 @@ static void dlm_run_purge_list(struct dlm_ctxt *dlm,
 			     "used %d, state %d\n", dlm->name,
 			     lockres->lockname.len, lockres->lockname.name,
 			     !unused, lockres->state);
-			list_move_tail(&dlm->purge_list, &lockres->purge);
+			list_move_tail(&lockres->purge, dlm->purge_list);
Erf, good catch!

Quoted 9 lines

But this wants to be:
list_move_tail(&lockres->purge, &dlm->purge_list);
^^^^
Notice the extra '&'
correct?
	--Mark

Yes, my mistake. I have already sent the version two. Would you please review it? thank you.

Quoted 3 lines

--
Mark Fasheh

Mark Fasheh

2014-06-16 22:06 UTC #3 lore raw

PATCH 1/1 ocfs2/dlm: fix misuse of list_move_tail() in dlm_run_purge_list()

Quoted 34 lines · Xue jiufei

Quoted 33 lines · Mark Fasheh
Quoted 18 lines · Xue jiufei
When a lockres in purge list but is still in use, it should be moved to the tail of purge list. dlm_thread will continue to check next lockres in purge list. However, code list_move_tail(&dlm->purge_list, &lockres->purge) will do *no* movements, so dlm_thread will purge the same lockres in this loop again and again. If it is in use for a long time, other lockres will not be processed.
Signed-off-by Yiwen Jiang <[email protected]>
Signed-off-by joyce.xue <[email protected]>
---
 fs/ocfs2/dlm/dlmthread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/dlm/dlmthread.c +0 -0
index 9db869d..ce017de 100644
fs/ocfs2/dlm/dlmthread.c static void dlm_run_purge_list(struct dlm_ctxt *dlm, · line 267
Quoted 5 lines
fs/ocfs2/dlm/dlmthread.c +1 -1
@@ -264,7 +264,7 @@ static void dlm_run_purge_list(struct dlm_ctxt *dlm,
 			     "used %d, state %d\n", dlm->name,
 			     lockres->lockname.len, lockres->lockname.name,
 			     !unused, lockres->state);
-			list_move_tail(&dlm->purge_list, &lockres->purge);
+			list_move_tail(&lockres->purge, dlm->purge_list);
Erf, good catch!
But this wants to be:
list_move_tail(&lockres->purge, &dlm->purge_list);
^^^^
Notice the extra '&'

Quoted 5 lines

Quoted 3 lines
correct?
	--Mark
Yes, my mistake. I have already sent the version two. Would you please review it? thank you.

No problem, you should've gotten my signoff on the fixed patch already.

	--Mark

--
Mark Fasheh