From 92729add67e2e7b8c7589c2dfab0bde071a7faf2 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Mon, 15 Apr 2013 15:56:57 +0530 Subject: glusterd: big lock - a coarse-grained locking to prevent races There are primarily three lists that are part of glusterd process, that are concurrently accessed. Namely, priv->volumes, priv->peers and volinfo->bricks_list. Big-lock approach ----------------- WHAT IS IT? Big lock is a coarse-grained lock which protects all three lists, mentioned above, from racy access. HOW DOES IT WORK? At any given point in time, glusterd's thread(s) are in execution _iff_ there is a preceding, inbound network event. Of course, the sigwaiter thread and timer thread are exceptions. A network event is an external trigger to glusterd, via the epoll thread, in the form of POLLIN and POLLERR. As long as we take the big-lock at all such entry points and yield it when we are done, we are guaranteed that all the network events, accessing the global lists, are serialised. This amounts to holding the big lock at - all the handlers of all the actors in glusterd. (POLLIN) - all the cbks in glusterd. (POLLIN) - rpc_notify (DISCONNECT event), if we access/modify one of the three lists. (POLLERR) In the case of synctask'ized volume operations, we must remember that, if we held the big lock for the entire duration of the handler, we may block other non-synctask rpc actors from executing. For eg, volume-start would block in PMAP SIGNIN, if done incorrectly. To prevent this, we need to yield the big lock, when we yield the synctask, and reacquire on waking up of the synctask. BUG: 948686 Change-Id: I429832f1fed67bcac0813403d58346558a403ce9 Signed-off-by: Krishnan Parthasarathi Reviewed-on: http://review.gluster.org/4835 Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/mgmt/glusterd/src/glusterd-pmap.c | 43 +++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 5 deletions(-) (limited to 'xlators/mgmt/glusterd/src/glusterd-pmap.c') diff --git a/xlators/mgmt/glusterd/src/glusterd-pmap.c b/xlators/mgmt/glusterd/src/glusterd-pmap.c index cb29462c9b7..aab6744a418 100644 --- a/xlators/mgmt/glusterd/src/glusterd-pmap.c +++ b/xlators/mgmt/glusterd/src/glusterd-pmap.c @@ -280,7 +280,7 @@ out: } int -gluster_pmap_portbybrick (rpcsvc_request_t *req) +__gluster_pmap_portbybrick (rpcsvc_request_t *req) { pmap_port_by_brick_req args = {0,}; pmap_port_by_brick_rsp rsp = {0,}; @@ -314,7 +314,14 @@ fail: int -gluster_pmap_brickbyport (rpcsvc_request_t *req) +gluster_pmap_portbybrick (rpcsvc_request_t *req) +{ + return glusterd_big_locked_handler (req, __gluster_pmap_portbybrick); +} + + +int +__gluster_pmap_brickbyport (rpcsvc_request_t *req) { pmap_brick_by_port_req args = {0,}; pmap_brick_by_port_rsp rsp = {0,}; @@ -340,6 +347,14 @@ fail: return 0; } + +int +gluster_pmap_brickbyport (rpcsvc_request_t *req) +{ + return glusterd_big_locked_handler (req, __gluster_pmap_brickbyport); +} + + static int glusterd_brick_update_signin (glusterd_brickinfo_t *brickinfo, gf_boolean_t value) @@ -350,7 +365,7 @@ glusterd_brick_update_signin (glusterd_brickinfo_t *brickinfo, } int -gluster_pmap_signup (rpcsvc_request_t *req) +__gluster_pmap_signup (rpcsvc_request_t *req) { pmap_signup_req args = {0,}; pmap_signup_rsp rsp = {0,}; @@ -376,7 +391,13 @@ fail: } int -gluster_pmap_signin (rpcsvc_request_t *req) +gluster_pmap_signup (rpcsvc_request_t *req) +{ + return glusterd_big_locked_handler (req, __gluster_pmap_signup); +} + +int +__gluster_pmap_signin (rpcsvc_request_t *req) { pmap_signin_req args = {0,}; pmap_signin_rsp rsp = {0,}; @@ -407,9 +428,15 @@ fail: } +int +gluster_pmap_signin (rpcsvc_request_t *req) +{ + return glusterd_big_locked_handler (req, __gluster_pmap_signin); +} + int -gluster_pmap_signout (rpcsvc_request_t *req) +__gluster_pmap_signout (rpcsvc_request_t *req) { pmap_signout_req args = {0,}; pmap_signout_rsp rsp = {0,}; @@ -440,6 +467,12 @@ fail: return 0; } +int +gluster_pmap_signout (rpcsvc_request_t *req) +{ + return glusterd_big_locked_handler (req, __gluster_pmap_signout); +} + rpcsvc_actor_t gluster_pmap_actors[] = { [GF_PMAP_NULL] = {"NULL", GF_PMAP_NULL, NULL, NULL, 0}, [GF_PMAP_PORTBYBRICK] = {"PORTBYBRICK", GF_PMAP_PORTBYBRICK, -- cgit