Ocfs2 devel - Jul 2010 - [PATCH] ocfs2/dlm: remove BUG_ON in a rare case

For migration, we are waiting for DLM_LOCK_RES_MIGRATING flag to be set
before sending DLM_MIG_LOCKRES_MSG message to the target. We are using
dlm_migration_can_proceed() for that purpose. while, dlm_migration_can_proceed()
also returns "go ahead" if target node is down. In this rare case, the
DLM_LOCK_RES_MIGRATING maybe is not set yet. Remove the BUG_ON() in this case.

Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>
---
 fs/ocfs2/dlm/dlmmaster.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 4a7506a..94b97fc 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -2808,14 +2808,8 @@ again:
 		mlog(0, "trying again...\n");
 		goto again;
 	}
-	/* now that we are sure the MIGRATING state is there, drop
-	 * the unneded state which blocked threads trying to DIRTY */
-	spin_lock(&res->spinlock);
-	BUG_ON(!(res->state & DLM_LOCK_RES_BLOCK_DIRTY));
-	BUG_ON(!(res->state & DLM_LOCK_RES_MIGRATING));
-	res->state &= ~DLM_LOCK_RES_BLOCK_DIRTY;
-	spin_unlock(&res->spinlock);
 
+	ret = 0;
 	/* did the target go down or die? */
 	spin_lock(&dlm->spinlock);
 	if (!test_bit(target, dlm->domain_map)) {
@@ -2826,9 +2820,21 @@ again:
 	spin_unlock(&dlm->spinlock);
 
 	/*
+	 * if target is down, we need to clear DLM_LOCK_RES_BLOCK_DIRTY for
+	 * another try; otherwise, we are sure the MIGRATING state is there,
+	 * drop the unneded state which blocked threads trying to DIRTY
+	 */
+	spin_lock(&res->spinlock);
+	BUG_ON(!(res->state & DLM_LOCK_RES_BLOCK_DIRTY));
+	res->state &= ~DLM_LOCK_RES_BLOCK_DIRTY;
+	if (!ret)
+		BUG_ON(!(res->state & DLM_LOCK_RES_MIGRATING));
+	spin_unlock(&res->spinlock);
+
+	/*
 	 * at this point:
 	 *
-	 *   o the DLM_LOCK_RES_MIGRATING flag is set
+	 *   o the DLM_LOCK_RES_MIGRATING flag is set if target not down
 	 *   o there are no pending asts on this lockres
 	 *   o all processes trying to reserve an ast on this
 	 *     lockres must wait for the MIGRATING flag to clear
-- 
1.6.6.1

Signed-off-by: Sunil Mushran<sunil.mushran at oracle.com>


On 07/14/2010 07:38 AM, Wengang Wang wrote:
> For migration, we are waiting for DLM_LOCK_RES_MIGRATING flag to be set
> before sending DLM_MIG_LOCKRES_MSG message to the target. We are using
> dlm_migration_can_proceed() for that purpose. while,
dlm_migration_can_proceed()
> also returns "go ahead" if target node is down. In this rare
case, the
> DLM_LOCK_RES_MIGRATING maybe is not set yet. Remove the BUG_ON() in this
case.
>
> Signed-off-by: Wengang Wang<wen.gang.wang at oracle.com>
> ---
>   fs/ocfs2/dlm/dlmmaster.c |   22 ++++++++++++++--------
>   1 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
> index 4a7506a..94b97fc 100644
> --- a/fs/ocfs2/dlm/dlmmaster.c
> +++ b/fs/ocfs2/dlm/dlmmaster.c
> @@ -2808,14 +2808,8 @@ again:
>   		mlog(0, "trying again...\n");
>   		goto again;
>   	}
> -	/* now that we are sure the MIGRATING state is there, drop
> -	 * the unneded state which blocked threads trying to DIRTY */
> -	spin_lock(&res->spinlock);
> -	BUG_ON(!(res->state&  DLM_LOCK_RES_BLOCK_DIRTY));
> -	BUG_ON(!(res->state&  DLM_LOCK_RES_MIGRATING));
> -	res->state&= ~DLM_LOCK_RES_BLOCK_DIRTY;
> -	spin_unlock(&res->spinlock);
>
> +	ret = 0;
>   	/* did the target go down or die? */
>   	spin_lock(&dlm->spinlock);
>   	if (!test_bit(target, dlm->domain_map)) {
> @@ -2826,9 +2820,21 @@ again:
>   	spin_unlock(&dlm->spinlock);
>
>   	/*
> +	 * if target is down, we need to clear DLM_LOCK_RES_BLOCK_DIRTY for
> +	 * another try; otherwise, we are sure the MIGRATING state is there,
> +	 * drop the unneded state which blocked threads trying to DIRTY
> +	 */
> +	spin_lock(&res->spinlock);
> +	BUG_ON(!(res->state&  DLM_LOCK_RES_BLOCK_DIRTY));
> +	res->state&= ~DLM_LOCK_RES_BLOCK_DIRTY;
> +	if (!ret)
> +		BUG_ON(!(res->state&  DLM_LOCK_RES_MIGRATING));
> +	spin_unlock(&res->spinlock);
> +
> +	/*
>   	 * at this point:
>   	 *
> -	 *   o the DLM_LOCK_RES_MIGRATING flag is set
> +	 *   o the DLM_LOCK_RES_MIGRATING flag is set if target not down
>   	 *   o there are no pending asts on this lockres
>   	 *   o all processes trying to reserve an ast on this
>   	 *     lockres must wait for the MIGRATING flag to clear
>

On Wed, Jul 14, 2010 at 10:38:21PM +0800, Wengang Wang
wrote:
> For migration, we are waiting for DLM_LOCK_RES_MIGRATING flag to be set
> before sending DLM_MIG_LOCKRES_MSG message to the target. We are using
> dlm_migration_can_proceed() for that purpose. while,
dlm_migration_can_proceed()
> also returns "go ahead" if target node is down. In this rare
case, the
> DLM_LOCK_RES_MIGRATING maybe is not set yet. Remove the BUG_ON() in this
case.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>
	This patch is now in the 'fixes' branch of ocfs2.git.

Joel

-- 

"The whole problem with the world is that fools and fanatics are always
 so certain of themselves, and wiser people so full of doubts."
	- Bertrand Russell

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127