From efa154bb0a4cac34d5a9610ec25d38eebe495f22 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Wed, 10 Apr 2013 17:12:01 +0530 Subject: 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. Change-Id: I591e67c267f0e26d1145bf8fb5feeb2c13a751a1 BUG: 948686 Signed-off-by: Krishnan Parthasarathi Reviewed-on: http://review.gluster.org/4802 Reviewed-by: Jeff Darcy Tested-by: Gluster Build System --- rpc/rpc-lib/src/rpc-clnt.c | 36 +++++++++++++++++++++++------ rpc/rpc-lib/src/rpc-clnt.h | 9 ++++++++ xlators/mgmt/glusterd/src/glusterd-syncop.c | 33 ++++++++++++++++---------- xlators/mgmt/glusterd/src/glusterd-syncop.h | 8 ++++--- 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 e6c681df..a27d0479 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 0da16555..34612fcc 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 17e7f1b8..5bdf5917 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? */ @@ -236,13 +237,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; } @@ -286,13 +289,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; } @@ -371,6 +376,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) @@ -387,10 +393,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; @@ -588,6 +595,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) @@ -604,10 +612,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 658ed4e2..212e495d 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, -- cgit