glusterd: Fixed spurious wakeups in glusterd syncops

glusterd syncops perform a barrier_wake whenever rpc_clnt_submit returned -1.
This is based on the wrong assumption that the cbkfn wasn't called.
This would result in one more wakeup than there ought to be.

BUG: 948686
Change-Id: I839fd218a81255fe50c2047d67461d45360e894d
Signed-off-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-on: http://review.gluster.org/4834
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 64 insertions, 22 deletions

diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c
index e6c681df8..a27d04791 100644
--- a/rpc/rpc-lib/src/rpc-clnt.c
+++ b/rpc/rpc-lib/src/rpc-clnt.c
@@ -1384,13 +1384,14 @@ out:
 
 
 int
-rpc_clnt_submit (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
-                 int procnum, fop_cbk_fn_t cbkfn,
-                 struct iovec *proghdr, int proghdrcount,
-                 struct iovec *progpayload, int progpayloadcount,
-                 struct iobref *iobref, void *frame, struct iovec *rsphdr,
-                 int rsphdr_count, struct iovec *rsp_payload,
-                 int rsp_payload_count, struct iobref *rsp_iobref)
+rpc_clnt_submit2 (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
+                  int procnum, fop_cbk_fn_t cbkfn,
+                  struct iovec *proghdr, int proghdrcount,
+                  struct iovec *progpayload, int progpayloadcount,
+                  struct iobref *iobref, void *frame, struct iovec *rsphdr,
+                  int rsphdr_count, struct iovec *rsp_payload,
+                  int rsp_payload_count, struct iobref *rsp_iobref,
+                  gf_boolean_t *lost)
 {
         rpc_clnt_connection_t *conn        = NULL;
         struct iobuf          *request_iob = NULL;
@@ -1401,6 +1402,7 @@ rpc_clnt_submit (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
         int                    proglen     = 0;
         char                   new_iobref  = 0;
         uint64_t               callid      = 0;
+        gf_boolean_t           cbk_lost    = _gf_true;
 
         if (!rpc || !prog || !frame) {
                 goto out;
@@ -1489,6 +1491,7 @@ rpc_clnt_submit (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
 
                 if ((ret >= 0) && frame) {
                         /* Save the frame in queue */
+                        cbk_lost = _gf_false;
                         __save_frame (rpc, frame, rpcreq);
 
                         gf_log ("rpc-clnt", GF_LOG_TRACE, "submitted request "
@@ -1519,12 +1522,31 @@ out:
                 if (rpcreq) {
                         rpcreq->rpc_status = -1;
                         cbkfn (rpcreq, NULL, 0, frame);
+                        cbk_lost = _gf_false;
                         mem_put (rpcreq);
                 }
         }
+        if (lost)
+                *lost = cbk_lost;
         return ret;
 }
 
+int
+rpc_clnt_submit (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
+                 int procnum, fop_cbk_fn_t cbkfn,
+                 struct iovec *proghdr, int proghdrcount,
+                 struct iovec *progpayload, int progpayloadcount,
+                 struct iobref *iobref, void *frame, struct iovec *rsphdr,
+                 int rsphdr_count, struct iovec *rsp_payload,
+                 int rsp_payload_count, struct iobref *rsp_iobref)
+{
+        return rpc_clnt_submit2 (rpc, prog, procnum, cbkfn, proghdr,
+                                 proghdrcount, progpayload, progpayloadcount,
+                                 iobref, frame, rsphdr, rsphdr_count,
+                                 rsp_payload, rsp_payload_count, rsp_iobref,
+                                 NULL);
+}
+
 
 struct rpc_clnt *
 rpc_clnt_ref (struct rpc_clnt *rpc)
diff --git a/rpc/rpc-lib/src/rpc-clnt.h b/rpc/rpc-lib/src/rpc-clnt.h
index 0da165559..34612fcc4 100644
--- a/rpc/rpc-lib/src/rpc-clnt.h
+++ b/rpc/rpc-lib/src/rpc-clnt.h
@@ -216,6 +216,15 @@ int rpc_clnt_submit (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
                      int rsphdr_count, struct iovec *rsp_payload,
                      int rsp_payload_count, struct iobref *rsp_iobref);
 
+int rpc_clnt_submit2 (struct rpc_clnt *rpc, rpc_clnt_prog_t *prog,
+                     int procnum, fop_cbk_fn_t cbkfn,
+                     struct iovec *proghdr, int proghdrcount,
+                     struct iovec *progpayload, int progpayloadcount,
+                     struct iobref *iobref, void *frame, struct iovec *rsphdr,
+                     int rsphdr_count, struct iovec *rsp_payload,
+                     int rsp_payload_count, struct iobref *rsp_iobref,
+                     gf_boolean_t *cbk_lost);
+
 struct rpc_clnt *
 rpc_clnt_ref (struct rpc_clnt *rpc);
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.c b/xlators/mgmt/glusterd/src/glusterd-syncop.c
index 3249dbc00..4430b41a8 100644
--- a/xlators/mgmt/glusterd/src/glusterd-syncop.c
+++ b/xlators/mgmt/glusterd/src/glusterd-syncop.c
@@ -72,7 +72,8 @@ gd_brick_op_req_free (gd1_mgmt_brick_op_req *req)
 int
 gd_syncop_submit_request (struct rpc_clnt *rpc, void *req,
                           void *cookie, rpc_clnt_prog_t *prog,
-                          int procnum, fop_cbk_fn_t cbkfn, xdrproc_t xdrproc)
+                          int procnum, fop_cbk_fn_t cbkfn, xdrproc_t xdrproc,
+                          gf_boolean_t *cbk_lost)
 {
         int            ret      = -1;
         struct iobuf  *iobuf    = NULL;
@@ -115,9 +116,9 @@ gd_syncop_submit_request (struct rpc_clnt *rpc, void *req,
         frame->local = cookie;
 
         /* Send the msg */
-        ret = rpc_clnt_submit (rpc, prog, procnum, cbkfn,
-                               &iov, count, NULL, 0, iobref,
-                               frame, NULL, 0, NULL, 0, NULL);
+        ret = rpc_clnt_submit2 (rpc, prog, procnum, cbkfn, &iov, count, NULL,
+                                0, iobref, frame, NULL, 0, NULL, 0, NULL,
+                                cbk_lost);
 
         /* TODO: do we need to start ping also? */
 
@@ -235,13 +236,15 @@ gd_syncop_mgmt_lock (struct rpc_clnt *rpc, struct syncargs *args,
 {
         int                       ret = -1;
         gd1_mgmt_cluster_lock_req req  = {{0},};
+        gf_boolean_t              cbk_lost = _gf_true;
 
         uuid_copy (req.uuid, my_uuid);
         ret = gd_syncop_submit_request (rpc, &req, args, &gd_mgmt_prog,
                                         GLUSTERD_MGMT_CLUSTER_LOCK,
                                         gd_syncop_mgmt_lock_cbk,
-                                        (xdrproc_t) xdr_gd1_mgmt_cluster_lock_req);
-        if (ret)
+                                        (xdrproc_t) xdr_gd1_mgmt_cluster_lock_req,
+                                        &cbk_lost);
+        if (cbk_lost)
                 synctask_barrier_wake(args);
         return ret;
 }
@@ -285,13 +288,15 @@ gd_syncop_mgmt_unlock (struct rpc_clnt *rpc, struct syncargs *args,
 {
         int                         ret     = -1;
         gd1_mgmt_cluster_unlock_req req     = {{0},};
+        gf_boolean_t                cbk_lost = _gf_true;
 
         uuid_copy (req.uuid, my_uuid);
         ret = gd_syncop_submit_request (rpc, &req, args, &gd_mgmt_prog,
                                         GLUSTERD_MGMT_CLUSTER_UNLOCK,
                                         gd_syncop_mgmt_unlock_cbk,
-                                        (xdrproc_t) xdr_gd1_mgmt_cluster_lock_req);
-        if (ret)
+                                        (xdrproc_t) xdr_gd1_mgmt_cluster_lock_req,
+                                        &cbk_lost);
+        if (cbk_lost)
                 synctask_barrier_wake(args);
         return ret;
 }
@@ -370,6 +375,7 @@ gd_syncop_mgmt_stage_op (struct rpc_clnt *rpc, struct syncargs *args,
 {
         gd1_mgmt_stage_op_req *req  = NULL;
         int                   ret  = -1;
+        gf_boolean_t          cbk_lost = _gf_true;
 
         req = GF_CALLOC (1, sizeof (*req), gf_gld_mt_mop_stage_req_t);
         if (!req)
@@ -386,10 +392,11 @@ gd_syncop_mgmt_stage_op (struct rpc_clnt *rpc, struct syncargs *args,
         ret = gd_syncop_submit_request (rpc, req, args, &gd_mgmt_prog,
                                         GLUSTERD_MGMT_STAGE_OP,
                                         gd_syncop_stage_op_cbk,
-                                        (xdrproc_t) xdr_gd1_mgmt_stage_op_req);
+                                        (xdrproc_t) xdr_gd1_mgmt_stage_op_req,
+                                        &cbk_lost);
 out:
         gd_stage_op_req_free (req);
-        if (ret)
+        if (cbk_lost)
                 synctask_barrier_wake(args);
 
         return ret;
@@ -587,6 +594,7 @@ gd_syncop_mgmt_commit_op (struct rpc_clnt *rpc, struct syncargs *args,
 {
         gd1_mgmt_commit_op_req *req  = NULL;
         int                    ret  = -1;
+        gf_boolean_t           cbk_lost = _gf_true;
 
         req = GF_CALLOC (1, sizeof (*req), gf_gld_mt_mop_commit_req_t);
         if (!req)
@@ -603,10 +611,11 @@ gd_syncop_mgmt_commit_op (struct rpc_clnt *rpc, struct syncargs *args,
         ret = gd_syncop_submit_request (rpc, req, args, &gd_mgmt_prog,
                                         GLUSTERD_MGMT_COMMIT_OP ,
                                         gd_syncop_commit_op_cbk,
-                                        (xdrproc_t) xdr_gd1_mgmt_commit_op_req);
+                                        (xdrproc_t) xdr_gd1_mgmt_commit_op_req,
+                                        &cbk_lost);
 out:
         gd_commit_op_req_free (req);
-        if (ret)
+        if (cbk_lost)
                 synctask_barrier_wake(args);
 
         return ret;
diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.h b/xlators/mgmt/glusterd/src/glusterd-syncop.h
index 658ed4e2a..212e495d5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-syncop.h
+++ b/xlators/mgmt/glusterd/src/glusterd-syncop.h
@@ -18,13 +18,15 @@
 #define GD_SYNCOP(rpc, stb, cbk, req, prog, procnum, xdrproc) do {      \
                 int ret = 0;                                            \
                 struct  synctask        *task = NULL;                   \
+                gf_boolean_t            cbk_lost = _gf_true;            \
                 task = synctask_get ();                                 \
                 stb->task = task;                                       \
                                                                         \
                 ret = gd_syncop_submit_request (rpc, req, stb,          \
                                                 prog, procnum, cbk,     \
-                                                (xdrproc_t)xdrproc);    \
-                if (!ret)                                               \
+                                                (xdrproc_t)xdrproc,     \
+                                                &cbk_lost);             \
+                if (!cbk_lost)                                          \
                         synctask_yield (stb->task);                     \
         } while (0)
 
@@ -32,7 +34,7 @@
 int gd_syncop_submit_request (struct rpc_clnt *rpc, void *req,
                                void *cookie, rpc_clnt_prog_t *prog,
                                int procnum, fop_cbk_fn_t cbkfn,
-                               xdrproc_t xdrproc);
+                               xdrproc_t xdrproc, gf_boolean_t *cbk_lost);
 
 
 int gd_syncop_mgmt_lock (struct rpc_clnt *rpc, struct syncargs *arg,