[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm

[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm

[Tiger Yang]

We used to return positive EAGAIN to indicate a retry action
is needed in dlm_begin_reco_handler(). Now we use negative -EAGAIN
to erase the confusion caused by this error code.

Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
---
 fs/ocfs2/dlm/dlmrecovery.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index d9fa3d2..8818c8c 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -2589,6 +2589,15 @@ retry:
 			     "begin reco msg (%d)\n", dlm->name, nodenum, ret);
 			ret = 0;
 		}
+		if (ret == -EAGAIN) {
+			mlog(0, "%s: trying to start recovery of node "
+			     "%u, but node %u is waiting for last recovery "
+			     "to complete, backoff for a bit\n", dlm->name,
+			     dead_node, nodenum);
+			/*TODO Look into replacing msleep with cond_resched()*/
+			msleep(100);
+			goto retry;
+		}
 		if (ret < 0) {
 			struct dlm_lock_resource *res;
 			/* this is now a serious problem, possibly ENOMEM 
@@ -2608,14 +2617,6 @@ retry:
 			 * another ENOMEM */
 			msleep(100);
 			goto retry;
-		} else if (ret == EAGAIN) {
-			mlog(0, "%s: trying to start recovery of node "
-			     "%u, but node %u is waiting for last recovery "
-			     "to complete, backoff for a bit\n", dlm->name,
-			     dead_node, nodenum);
-			/* TODO Look into replacing msleep with cond_resched() */
-			msleep(100);
-			goto retry;
 		}
 	}
 
@@ -2639,7 +2640,7 @@ int dlm_begin_reco_handler(struct o2net_msg *msg, u32 len, void *data,
 		     dlm->name, br->node_idx, br->dead_node,
 		     dlm->reco.dead_node, dlm->reco.new_master);
 		spin_unlock(&dlm->spinlock);
-		return EAGAIN;
+		return -EAGAIN;
 	}
 	spin_unlock(&dlm->spinlock);
 
-- 
1.6.2.5

[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm

[Sunil Mushran]

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

comments inlined

Tiger Yang wrote:
> We used to return positive EAGAIN to indicate a retry action
> is needed in dlm_begin_reco_handler(). Now we use negative -EAGAIN
> to erase the confusion caused by this error code.
>
> Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
> ---
>  fs/ocfs2/dlm/dlmrecovery.c |   19 ++++++++++---------
>  1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index d9fa3d2..8818c8c 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -2589,6 +2589,15 @@ retry:
>  			     "begin reco msg (%d)\n", dlm->name, nodenum, ret);
>  			ret = 0;
>  		}
> +		if (ret == -EAGAIN) {
> +			mlog(0, "%s: trying to start recovery of node "
> +			     "%u, but node %u is waiting for last recovery "
> +			     "to complete, backoff for a bit\n", dlm->name,
> +			     dead_node, nodenum);
> +			/*TODO Look into replacing msleep with cond_resched()*/

This comment can be removed. We cannot replace the msleep with 
cond_resched().
If anything we may want to increase the msleep to 1 sec. But leave that 
as is for now.

> +			msleep(100);
> +			goto retry;
> +		}
>  		if (ret < 0) {
>  			struct dlm_lock_resource *res;
>  			/* this is now a serious problem, possibly ENOMEM 
> @@ -2608,14 +2617,6 @@ retry:
>  			 * another ENOMEM */
>  			msleep(100);
>  			goto retry;
> -		} else if (ret == EAGAIN) {
> -			mlog(0, "%s: trying to start recovery of node "
> -			     "%u, but node %u is waiting for last recovery "
> -			     "to complete, backoff for a bit\n", dlm->name,
> -			     dead_node, nodenum);
> -			/* TODO Look into replacing msleep with cond_resched() */
> -			msleep(100);
> -			goto retry;
>  		}
>  	}
>  
> @@ -2639,7 +2640,7 @@ int dlm_begin_reco_handler(struct o2net_msg *msg, u32 len, void *data,
>  		     dlm->name, br->node_idx, br->dead_node,
>  		     dlm->reco.dead_node, dlm->reco.new_master);
>  		spin_unlock(&dlm->spinlock);
> -		return EAGAIN;
> +		return -EAGAIN;
>  	}
>  	spin_unlock(&dlm->spinlock);
>  

This looks sane. Though I would like someone else to review it too as we
have no real way of testing this.

[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm

[Tiger Yang]

We used to return positive EAGAIN to indicate a retry action
is needed in dlm_begin_reco_handler(). Now we return negative
-EAGAIN to erase the confusion caused by this error code.

Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
---
 fs/ocfs2/dlm/dlmrecovery.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index d9fa3d2..2f9e4e1 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -2589,6 +2589,14 @@ retry:
 			     "begin reco msg (%d)\n", dlm->name, nodenum, ret);
 			ret = 0;
 		}
+		if (ret == -EAGAIN) {
+			mlog(0, "%s: trying to start recovery of node "
+			     "%u, but node %u is waiting for last recovery "
+			     "to complete, backoff for a bit\n", dlm->name,
+			     dead_node, nodenum);
+			msleep(100);
+			goto retry;
+		}
 		if (ret < 0) {
 			struct dlm_lock_resource *res;
 			/* this is now a serious problem, possibly ENOMEM 
@@ -2608,14 +2616,6 @@ retry:
 			 * another ENOMEM */
 			msleep(100);
 			goto retry;
-		} else if (ret == EAGAIN) {
-			mlog(0, "%s: trying to start recovery of node "
-			     "%u, but node %u is waiting for last recovery "
-			     "to complete, backoff for a bit\n", dlm->name,
-			     dead_node, nodenum);
-			/* TODO Look into replacing msleep with cond_resched() */
-			msleep(100);
-			goto retry;
 		}
 	}
 
@@ -2639,7 +2639,7 @@ int dlm_begin_reco_handler(struct o2net_msg *msg, u32 len, void *data,
 		     dlm->name, br->node_idx, br->dead_node,
 		     dlm->reco.dead_node, dlm->reco.new_master);
 		spin_unlock(&dlm->spinlock);
-		return EAGAIN;
+		return -EAGAIN;
 	}
 	spin_unlock(&dlm->spinlock);
 
-- 
1.6.2.5

[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm

[Sunil Mushran]

ack

Tiger Yang wrote:
> We used to return positive EAGAIN to indicate a retry action
> is needed in dlm_begin_reco_handler(). Now we return negative
> -EAGAIN to erase the confusion caused by this error code.
>
> Signed-off-by: Tiger Yang <tiger.yang@oracle.com>
> ---
>  fs/ocfs2/dlm/dlmrecovery.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index d9fa3d2..2f9e4e1 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -2589,6 +2589,14 @@ retry:
>  			     "begin reco msg (%d)\n", dlm->name, nodenum, ret);
>  			ret = 0;
>  		}
> +		if (ret == -EAGAIN) {
> +			mlog(0, "%s: trying to start recovery of node "
> +			     "%u, but node %u is waiting for last recovery "
> +			     "to complete, backoff for a bit\n", dlm->name,
> +			     dead_node, nodenum);
> +			msleep(100);
> +			goto retry;
> +		}
>  		if (ret < 0) {
>  			struct dlm_lock_resource *res;
>  			/* this is now a serious problem, possibly ENOMEM 
> @@ -2608,14 +2616,6 @@ retry:
>  			 * another ENOMEM */
>  			msleep(100);
>  			goto retry;
> -		} else if (ret == EAGAIN) {
> -			mlog(0, "%s: trying to start recovery of node "
> -			     "%u, but node %u is waiting for last recovery "
> -			     "to complete, backoff for a bit\n", dlm->name,
> -			     dead_node, nodenum);
> -			/* TODO Look into replacing msleep with cond_resched() */
> -			msleep(100);
> -			goto retry;
>  		}
>  	}
>  
> @@ -2639,7 +2639,7 @@ int dlm_begin_reco_handler(struct o2net_msg *msg, u32 len, void *data,
>  		     dlm->name, br->node_idx, br->dead_node,
>  		     dlm->reco.dead_node, dlm->reco.new_master);
>  		spin_unlock(&dlm->spinlock);
> -		return EAGAIN;
> +		return -EAGAIN;
>  	}
>  	spin_unlock(&dlm->spinlock);
>  
>   

[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm

[Joel Becker]

On Thu, Nov 19, 2009 at 10:17:46AM +0800, Tiger Yang wrote:
> We used to return positive EAGAIN to indicate a retry action
> is needed in dlm_begin_reco_handler(). Now we return negative
> -EAGAIN to erase the confusion caused by this error code.
> 
> Signed-off-by: Tiger Yang <tiger.yang@oracle.com>

	This is now in the 'merge-window' branch of ocfs2.git.

Joel

-- 

"I always thought the hardest questions were those I could not answer.
 Now I know they are the ones I can never ask."
			- Charlie Watkins

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