From f5ef4d808724afe4ddd477b0c0e8b3eb985319db Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Fri, 28 Nov 2014 17:24:13 +0530 Subject: glusterd: use synclock_t for synchronizing concurrent '\op_sm\' invocations In glusterd_op_sm(), we lock and unlock the gd_op_sm_lock mutex. Unfortunately, locking and unlocking can happen in different threads (task swap will occur in handler call with use of synctasks). This case is explictely covered by POSIX: the behavior is undefined. http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_lock.html When unlocking from a thread that is not owner, Linux seems to be fine (though you never know with unspecified operation), while NetBSD returns EPERM, causing a spurious error in tests/basic/pump. To fix this, we use synclock_t which was precisely meant for this. synclock is a pthread_mutex_t like synchronization object which uses the synctask handle for owner and is immune to the task being run on multiple threads during its lifetime. Change-Id: Idca15190d42f32a843088cc8236138f676377586 BUG: 1129939 Signed-off-by: Krishnan Parthasarathi Reviewed-on: http://review.gluster.org/9212 Reviewed-by: Kaushal M Tested-by: Kaushal M --- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'xlators/mgmt/glusterd/src/glusterd-op-sm.c') diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 9a9b580d3ed..3f8de08a577 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -67,7 +67,7 @@ } while (0) static struct list_head gd_op_sm_queue; -pthread_mutex_t gd_op_sm_lock; +synclock_t gd_op_sm_lock; glusterd_op_info_t opinfo = {{0},}; int32_t @@ -6572,7 +6572,9 @@ glusterd_op_sm () this = THIS; GF_ASSERT (this); - if ((lock_err = pthread_mutex_trylock (&gd_op_sm_lock))) { + ret = synclock_trylock (&gd_op_sm_lock); + if (ret) { + lock_err = errno; gf_log (this->name, GF_LOG_ERROR, "lock failed due to %s", strerror (lock_err)); goto lock_failed; @@ -6628,7 +6630,7 @@ glusterd_op_sm () "state from '%s' to '%s'", glusterd_op_sm_state_name_get(opinfo.state.state), glusterd_op_sm_state_name_get(state[event_type].next_state)); - (void ) pthread_mutex_unlock (&gd_op_sm_lock); + (void) synclock_unlock (&gd_op_sm_lock); return ret; } @@ -6657,7 +6659,7 @@ glusterd_op_sm () } - (void ) pthread_mutex_unlock (&gd_op_sm_lock); + (void) synclock_unlock (&gd_op_sm_lock); ret = 0; lock_failed: @@ -6756,6 +6758,6 @@ int glusterd_op_sm_init () { INIT_LIST_HEAD (&gd_op_sm_queue); - pthread_mutex_init (&gd_op_sm_lock, NULL); + synclock_init (&gd_op_sm_lock); return 0; } -- cgit