Tiger Yang
2009-Nov-18 10:35 UTC
[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm
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 at 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
Sunil Mushran
2009-Nov-18 22:26 UTC
[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm
Signed-off-by: Sunil Mushran <sunil.mushran at 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 at 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.
Tiger Yang
2009-Nov-19 02:17 UTC
[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm
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 at 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
Sunil Mushran
2009-Nov-21 01:25 UTC
[Ocfs2-devel] [PATCH 1/1] ocfs2: return -EAGAIN instead of EAGAIN in dlm
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 at 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); > >