From 40e13bc5b44d0b0cdaf7833c848d4a52352e0a13 Mon Sep 17 00:00:00 2001 From: Kaushal M Date: Thu, 8 Aug 2013 15:50:31 +0530 Subject: rpc,glusterd: Use rpc_clnt notifyfn to cleanup mydata rpc: - On a RPC_TRANSPORT_CLEANUP event, rpc_clnt_notify calls the registered notifyfn with a RPC_CLNT_DESTROY event. The notifyfn should properly cleanup the saved mydata on this event. - Break the reconnect chain when an rpc client is disabled. This will prevent new disconnect events which can lead to crashes. glusterd: - Added support for RPC_CLNT_DESTROY in glusterd_brick_rpc_notify - Use a common glusterd_rpc_clnt_unref() function throught glusterd in place of rpc_clnt_unref(). This function correctly gives up the big-lock before performing the unref. Change-Id: I93230441c5089039643fc9f5632477ef1b695348 BUG: 962619 Signed-off-by: Kaushal M Reviewed-on: http://review.gluster.org/5512 Tested-by: Gluster Build System Reviewed-by: Krishnan Parthasarathi Reviewed-by: Vijay Bellur --- rpc/rpc-lib/src/rpc-clnt.c | 9 ++++++++- rpc/rpc-lib/src/rpc-clnt.h | 3 ++- xlators/mgmt/glusterd/src/glusterd-handler.c | 6 ++++-- xlators/mgmt/glusterd/src/glusterd-rebalance.c | 2 +- xlators/mgmt/glusterd/src/glusterd-utils.c | 24 +++++++++++++++++++----- xlators/mgmt/glusterd/src/glusterd-utils.h | 3 +++ xlators/nfs/server/src/nlm4.c | 2 ++ 7 files changed, 39 insertions(+), 10 deletions(-) diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c index ac98a5c9110..1e9f307be40 100644 --- a/rpc/rpc-lib/src/rpc-clnt.c +++ b/rpc/rpc-lib/src/rpc-clnt.c @@ -416,7 +416,7 @@ rpc_clnt_reconnect (void *trans_ptr) conn->reconnect); conn->reconnect = 0; - if (conn->connected == 0) { + if ((conn->connected == 0) && !clnt->disabled) { ts.tv_sec = 3; ts.tv_nsec = 0; @@ -834,6 +834,7 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata, rpc_request_info_t *req_info = NULL; rpc_transport_pollin_t *pollin = NULL; struct timespec ts = {0, }; + void *clnt_mydata = NULL; conn = mydata; if (conn == NULL) { @@ -870,6 +871,12 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata, } case RPC_TRANSPORT_CLEANUP: + if (clnt->notifyfn) { + clnt_mydata = clnt->mydata; + clnt->mydata = NULL; + ret = clnt->notifyfn (clnt, clnt_mydata, + RPC_CLNT_DESTROY, NULL); + } rpc_clnt_destroy (clnt); ret = 0; break; diff --git a/rpc/rpc-lib/src/rpc-clnt.h b/rpc/rpc-lib/src/rpc-clnt.h index 584963ad036..2596c3508c2 100644 --- a/rpc/rpc-lib/src/rpc-clnt.h +++ b/rpc/rpc-lib/src/rpc-clnt.h @@ -19,7 +19,8 @@ typedef enum { RPC_CLNT_CONNECT, RPC_CLNT_DISCONNECT, - RPC_CLNT_MSG + RPC_CLNT_MSG, + RPC_CLNT_DESTROY } rpc_clnt_event_t; diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index 3aafa122b6a..0407741bb7b 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -3713,10 +3713,12 @@ __glusterd_brick_rpc_notify (struct rpc_clnt *rpc, void *mydata, "%s:%s", brickinfo->hostname, brickinfo->path); glusterd_set_brick_status (brickinfo, GF_BRICK_STOPPED); - if (rpc_clnt_is_disabled (rpc)) - GF_FREE (brickid); break; + case RPC_CLNT_DESTROY: + GF_FREE (mydata); + mydata = NULL; + break; default: gf_log (this->name, GF_LOG_TRACE, "got some other RPC event %d", event); diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c index ea8558894e7..1ac9d64ce20 100644 --- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c +++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c @@ -152,7 +152,7 @@ __glusterd_defrag_notify (struct rpc_clnt *rpc, void *mydata, glusterd_store_perform_node_state_store (volinfo); if (defrag->rpc) { - rpc_clnt_unref (defrag->rpc); + glusterd_rpc_clnt_unref (priv, defrag->rpc); defrag->rpc = NULL; } if (defrag->cbk_fn) diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index f398e49dc62..c11965d40de 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -1054,7 +1054,7 @@ glusterd_friend_cleanup (glusterd_peerinfo_t *peerinfo) peerctx = peerinfo->rpc->mydata; peerinfo->rpc->mydata = NULL; - peerinfo->rpc = rpc_clnt_unref (peerinfo->rpc); + peerinfo->rpc = glusterd_rpc_clnt_unref (priv, peerinfo->rpc); peerinfo->rpc = NULL; if (peerctx) { GF_FREE (peerctx->errstr); @@ -1466,9 +1466,7 @@ glusterd_brick_disconnect (glusterd_brickinfo_t *brickinfo) brickinfo->rpc = NULL; if (rpc) { - synclock_unlock (&priv->big_lock); - rpc_clnt_unref (rpc); - synclock_lock (&priv->big_lock); + glusterd_rpc_clnt_unref (priv, rpc); } return 0; @@ -3894,12 +3892,13 @@ int32_t glusterd_nodesvc_disconnect (char *server) { struct rpc_clnt *rpc = NULL; + glusterd_conf_t *priv = THIS->private; rpc = glusterd_nodesvc_get_rpc (server); (void)glusterd_nodesvc_set_rpc (server, NULL); if (rpc) - rpc_clnt_unref (rpc); + glusterd_rpc_clnt_unref (priv, rpc); return 0; } @@ -9357,3 +9356,18 @@ out: return ret; } + +rpc_clnt_t * +glusterd_rpc_clnt_unref (glusterd_conf_t *conf, rpc_clnt_t *rpc) +{ + rpc_clnt_t *ret = NULL; + + GF_ASSERT (conf); + GF_ASSERT (rpc); + synclock_unlock (&conf->big_lock); + ret = rpc_clnt_unref (rpc); + synclock_lock (&conf->big_lock); + + return ret; +} + diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index 20cd00cbe88..9ef09d7b089 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -635,4 +635,7 @@ glusterd_status_has_tasks (int cmd); int gd_stop_rebalance_process (glusterd_volinfo_t *volinfo); + +rpc_clnt_t * +glusterd_rpc_clnt_unref (glusterd_conf_t *conf, rpc_clnt_t *rpc); #endif diff --git a/xlators/nfs/server/src/nlm4.c b/xlators/nfs/server/src/nlm4.c index 5c5d87412d0..c186537ea0c 100644 --- a/xlators/nfs/server/src/nlm4.c +++ b/xlators/nfs/server/src/nlm4.c @@ -918,6 +918,8 @@ nlm_rpcclnt_notify (struct rpc_clnt *rpc_clnt, void *mydata, case RPC_CLNT_DISCONNECT: nlm_unset_rpc_clnt (rpc_clnt); break; + default: + break; } err: -- cgit