summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKrutika Dhananjay <kdhananj@redhat.com>2014-07-08 08:18:27 +0530
committerVijay Bellur <vbellur@redhat.com>2014-07-09 06:57:20 -0700
commitb9f1d1a120b4469d51d2a96eecc7ce83516593ba (patch)
treee298705ca9ff57181728c0eca323ee67534218e7
parent82e0fb290db880323613a3791acd33f96d421363 (diff)
rpc: Do not reset @ping_started to 0 in ping callback
This is to avoid indefinite recursion of the following kind, that could lead to a stack overflow: rpc_clnt_start_ping() -> rpc_clnt_ping() -> rpc_clnt_submit() -> rpc_clnt_start_ping() -> rpc_clnt_ping() -> rpc_clnt_submit() ... and so on, since it is possible that before rpc_clnt_start_ping() is called a second time by the thread executing this codepath, the response to previous ping request could ALWAYS come by and cause epoll thread to reset conn->ping_started to 0. This patch also fixes the issue of excessive ping traffic, which was due to the client sending one ping rpc for every fop in the worst case. Also removed dead code in glusterd. Change-Id: I7c5e6ae3b1c9d23407c0a12a319bdcb43ba7a359 BUG: 1116243 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com> Reviewed-on: http://review.gluster.org/8257 Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--rpc/rpc-lib/src/rpc-clnt-ping.c35
-rw-r--r--rpc/rpc-lib/src/rpc-clnt-ping.h2
-rw-r--r--rpc/rpc-lib/src/rpc-clnt.c2
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c23
4 files changed, 31 insertions, 31 deletions
diff --git a/rpc/rpc-lib/src/rpc-clnt-ping.c b/rpc/rpc-lib/src/rpc-clnt-ping.c
index 0ef54f8c6f8..948a244838e 100644
--- a/rpc/rpc-lib/src/rpc-clnt-ping.c
+++ b/rpc/rpc-lib/src/rpc-clnt-ping.c
@@ -35,6 +35,9 @@ struct rpc_clnt_program clnt_ping_prog = {
.procnames = clnt_ping_procs,
};
+static void
+rpc_clnt_start_ping (void *rpc_ptr);
+
void
rpc_clnt_ping_timer_expired (void *rpc_ptr)
{
@@ -88,6 +91,7 @@ rpc_clnt_ping_timer_expired (void *rpc_ptr)
if (conn->ping_timer == NULL) {
gf_log (trans->name, GF_LOG_WARNING,
"unable to setup ping timer");
+ conn->ping_started = 0;
rpc_clnt_unref (rpc);
}
} else {
@@ -154,10 +158,6 @@ rpc_clnt_ping_cbk (struct rpc_req *req, struct iovec *iov, int count,
goto unlock;
}
- /*This allows other RPCs to be able to start the ping timer
- * if they come by before the following start ping routine
- * is executed by the timer thread.*/
- conn->ping_started = 0;
gf_timer_call_cancel (this->ctx,
conn->ping_timer);
@@ -171,6 +171,7 @@ rpc_clnt_ping_cbk (struct rpc_req *req, struct iovec *iov, int count,
if (conn->ping_timer == NULL) {
gf_log (this->name, GF_LOG_WARNING,
"failed to set the ping timer");
+ conn->ping_started = 0;
rpc_clnt_unref (rpc);
}
@@ -214,7 +215,7 @@ fail:
}
-void
+static void
rpc_clnt_start_ping (void *rpc_ptr)
{
struct rpc_clnt *rpc = NULL;
@@ -233,14 +234,10 @@ rpc_clnt_start_ping (void *rpc_ptr)
pthread_mutex_lock (&conn->lock);
{
- if (conn->ping_started) {
- pthread_mutex_unlock (&conn->lock);
- return;
- }
-
if (conn->ping_timer) {
gf_timer_call_cancel (rpc->ctx, conn->ping_timer);
conn->ping_timer = NULL;
+ conn->ping_started = 0;
rpc_clnt_unref (rpc);
}
@@ -284,3 +281,21 @@ rpc_clnt_start_ping (void *rpc_ptr)
rpc_clnt_ping(rpc);
}
+
+void
+rpc_clnt_check_and_start_ping (struct rpc_clnt *rpc)
+{
+ char start_ping = 0;
+
+ pthread_mutex_lock (&rpc->conn.lock);
+ {
+ if (!rpc->conn.ping_started)
+ start_ping = 1;
+ }
+ pthread_mutex_unlock (&rpc->conn.lock);
+
+ if (start_ping)
+ rpc_clnt_start_ping ((void *)rpc);
+
+ return;
+}
diff --git a/rpc/rpc-lib/src/rpc-clnt-ping.h b/rpc/rpc-lib/src/rpc-clnt-ping.h
index 4edc416cee9..d7cd1d965e5 100644
--- a/rpc/rpc-lib/src/rpc-clnt-ping.h
+++ b/rpc/rpc-lib/src/rpc-clnt-ping.h
@@ -16,4 +16,4 @@
#define RPC_DEFAULT_PING_TIMEOUT 30
void
-rpc_clnt_start_ping (void *rpc_ptr);
+rpc_clnt_check_and_start_ping (struct rpc_clnt *rpc_ptr);
diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c
index 8a460cfa617..b831c537723 100644
--- a/rpc/rpc-lib/src/rpc-clnt.c
+++ b/rpc/rpc-lib/src/rpc-clnt.c
@@ -1579,7 +1579,7 @@ rpc_clnt_submit (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
goto out;
}
- rpc_clnt_start_ping (rpc);
+ rpc_clnt_check_and_start_ping (rpc);
ret = 0;
out:
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index c358e9f6d05..dc923b1eeb4 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -235,12 +235,12 @@ glusterd_submit_request_unlocked (struct rpc_clnt *rpc, void *req,
xlator_t *this, fop_cbk_fn_t cbkfn,
xdrproc_t xdrproc)
{
+ char new_iobref = 0;
int ret = -1;
- struct iobuf *iobuf = NULL;
- int count = 0;
- char new_iobref = 0, start_ping = 0;
- struct iovec iov = {0, };
+ int count = 0;
ssize_t req_size = 0;
+ struct iobuf *iobuf = NULL;
+ struct iovec iov = {0, };
GF_ASSERT (rpc);
GF_ASSERT (this);
@@ -279,21 +279,6 @@ glusterd_submit_request_unlocked (struct rpc_clnt *rpc, void *req,
ret = rpc_clnt_submit (rpc, prog, procnum, cbkfn,
&iov, count,
NULL, 0, iobref, frame, NULL, 0, NULL, 0, NULL);
-
- if (ret == 0) {
- pthread_mutex_lock (&rpc->conn.lock);
- {
- if (!rpc->conn.ping_started) {
- start_ping = 1;
- }
- }
- pthread_mutex_unlock (&rpc->conn.lock);
- }
-
- if (start_ping)
- //client_start_ping ((void *) this);
-
- ret = 0;
out:
if (new_iobref) {
iobref_unref (iobref);