diff options
| author | Krishnan Parthasarathi <kparthas@redhat.com> | 2014-11-28 17:24:13 +0530 | 
|---|---|---|
| committer | Kaushal M <kaushal@redhat.com> | 2014-12-01 21:47:13 -0800 | 
| commit | f5ef4d808724afe4ddd477b0c0e8b3eb985319db (patch) | |
| tree | dcc3903bb728290ea7e60ef20c3ace2c0f1d1bda | |
| parent | 24581a58e897a415dfcc96f65c41d82d0f1fb49a (diff) | |
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 <kparthas@redhat.com>
Reviewed-on: http://review.gluster.org/9212
Reviewed-by: Kaushal M <kaushal@redhat.com>
Tested-by: Kaushal M <kaushal@redhat.com>
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-op-sm.c | 12 | 
1 files changed, 7 insertions, 5 deletions
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;  }  | 
