diff options
Diffstat (limited to 'xlators')
| -rw-r--r-- | xlators/cluster/afr/src/afr-common.c | 228 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-inode-read.c | 6 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heal-common.c | 5 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heal-data.c | 12 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heal-metadata.c | 2 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr.h | 3 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/pump.c | 2 |
7 files changed, 155 insertions, 103 deletions
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 605c641a143..d7904deec97 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -504,8 +504,8 @@ afr_spb_choice_timeout_cancel (xlator_t *this, inode_t *inode) LOCK(&inode->lock); { - __afr_inode_ctx_get (this, inode, &ctx); - if (!ctx) { + ret = __afr_inode_ctx_get (this, inode, &ctx); + if (ret < 0 || !ctx) { gf_msg (this->name, GF_LOG_WARNING, 0, AFR_MSG_SPLIT_BRAIN_CHOICE_ERROR, "Failed to cancel split-brain choice timer."); @@ -538,14 +538,18 @@ afr_set_split_brain_choice_cbk (void *data) int afr_set_split_brain_choice (int ret, call_frame_t *frame, void *opaque) { - int op_errno = ENOMEM; - afr_private_t *priv = NULL; - afr_inode_ctx_t *ctx = NULL; - inode_t *inode = NULL; - loc_t *loc = NULL; - xlator_t *this = NULL; - afr_spbc_timeout_t *data = opaque; - struct timespec delta = {0, }; + int op_errno = ENOMEM; + afr_private_t *priv = NULL; + afr_inode_ctx_t *ctx = NULL; + inode_t *inode = NULL; + loc_t *loc = NULL; + xlator_t *this = NULL; + afr_spbc_timeout_t *data = opaque; + struct timespec delta = {0, }; + gf_boolean_t timer_set = _gf_false; + gf_boolean_t timer_cancelled = _gf_false; + gf_boolean_t timer_reset = _gf_false; + int old_spb_choice = -1; if (ret) goto out; @@ -558,9 +562,11 @@ afr_set_split_brain_choice (int ret, call_frame_t *frame, void *opaque) delta.tv_sec = priv->spb_choice_timeout; delta.tv_nsec = 0; - inode = loc->inode; - if (!inode) + if (!loc->inode) { + ret = -1; + op_errno = EINVAL; goto out; + } if (!(data->d_spb || data->m_spb)) { gf_msg (this->name, GF_LOG_WARNING, 0, @@ -573,6 +579,12 @@ afr_set_split_brain_choice (int ret, call_frame_t *frame, void *opaque) goto out; } + /* + * we're ref'ing the inode before LOCK like it is done elsewhere in the + * code. If we ref after LOCK, coverity complains of possible deadlocks. + */ + inode = inode_ref (loc->inode); + LOCK(&inode->lock); { ret = __afr_inode_ctx_get (this, inode, &ctx); @@ -583,16 +595,14 @@ afr_set_split_brain_choice (int ret, call_frame_t *frame, void *opaque) goto unlock; } + old_spb_choice = ctx->spb_choice; ctx->spb_choice = data->spb_child_index; /* Possible changes in spb-choice : - * -1 to valid : ref and inject timer - * - * valid to valid : cancel timer and inject new one - * * valid to -1 : cancel timer and unref - * - * -1 to -1 : do not do anything + * valid to valid : cancel timer and inject new one + * -1 to -1 : unref and do not do anything + * -1 to valid : inject timer */ /* ctx->timer is NULL iff previous value of @@ -600,31 +610,66 @@ afr_set_split_brain_choice (int ret, call_frame_t *frame, void *opaque) */ if (ctx->timer) { if (ctx->spb_choice == -1) { - gf_timer_call_cancel (this->ctx, ctx->timer); - ctx->timer = NULL; - inode_unref (inode); + if (!gf_timer_call_cancel (this->ctx, + ctx->timer)) { + ctx->timer = NULL; + timer_cancelled = _gf_true; + } + /* If timer cancel failed here it means that the + * previous cbk will be executed which will set + * spb_choice to -1. So we can consider the + * 'valid to -1' case to be a sucess + * (i.e. ret = 0) and goto unlock. + */ goto unlock; } goto reset_timer; } else { if (ctx->spb_choice == -1) goto unlock; + goto set_timer; } - inode = inode_ref (loc->inode); - goto set_timer; - reset_timer: - gf_timer_call_cancel (this->ctx, ctx->timer); + ret = gf_timer_call_cancel (this->ctx, ctx->timer); + if (ret != 0) { + /* We need to bail out now instead of launching a new + * timer. Otherwise the cbk of the previous timer event + * will cancel the new ctx->timer. + */ + ctx->spb_choice = old_spb_choice; + ret = -1; + op_errno = EAGAIN; + goto unlock; + } ctx->timer = NULL; + timer_reset = _gf_true; set_timer: ctx->timer = gf_timer_call_after (this->ctx, delta, afr_set_split_brain_choice_cbk, inode); + if (!ctx->timer) { + ctx->spb_choice = old_spb_choice; + ret = -1; + op_errno = ENOMEM; + } + if (!timer_reset && ctx->timer) + timer_set = _gf_true; + if (timer_reset && !ctx->timer) + timer_cancelled = _gf_true; } unlock: UNLOCK(&inode->lock); + if (!timer_set) + inode_unref (inode); + if (timer_cancelled) + inode_unref (inode); + /* + * We need to invalidate the inode to prevent the kernel from serving + * reads from an older cached value despite a change in spb_choice to + * a new value. + */ inode_invalidate (inode); out: if (data) @@ -2585,8 +2630,64 @@ out: return 0; } +void +_afr_cleanup_fd_ctx (afr_fd_ctx_t *fd_ctx) +{ + int i = 0; + + + for (i = 0; i < AFR_NUM_CHANGE_LOGS; i++) + GF_FREE (fd_ctx->pre_op_done[i]); + + GF_FREE (fd_ctx->opened_on); + + GF_FREE (fd_ctx->lock_piggyback); + + GF_FREE (fd_ctx->lock_acquired); + + pthread_mutex_destroy (&fd_ctx->delay_lock); + + GF_FREE (fd_ctx); + + return; +} + +int +afr_cleanup_fd_ctx (xlator_t *this, fd_t *fd) +{ + uint64_t ctx = 0; + afr_fd_ctx_t *fd_ctx = NULL; + int ret = 0; + + ret = fd_ctx_get (fd, this, &ctx); + if (ret < 0) + goto out; + + fd_ctx = (afr_fd_ctx_t *)(long) ctx; + + if (fd_ctx) { + /*no need to take any locks*/ + if (!list_empty (&fd_ctx->eager_locked)) + gf_msg (this->name, GF_LOG_WARNING, 0, + AFR_MSG_INVALID_DATA, "%s: Stale " + "Eager-lock stubs found", + uuid_utoa (fd->inode->gfid)); + + _afr_cleanup_fd_ctx (fd_ctx); + + } + +out: + return 0; +} + +int +afr_release (xlator_t *this, fd_t *fd) +{ + afr_cleanup_fd_ctx (this, fd); -/* {{{ open */ + return 0; +} afr_fd_ctx_t * __afr_fd_ctx_get (fd_t *fd, xlator_t *this) @@ -2654,6 +2755,13 @@ __afr_fd_ctx_set (xlator_t *this, fd_t *fd) goto out; } + ret = pthread_mutex_init (&fd_ctx->delay_lock, NULL); + if (ret) { + GF_FREE (fd_ctx); + fd_ctx = NULL; + goto out; + } + for (i = 0; i < AFR_NUM_CHANGE_LOGS; i++) { fd_ctx->pre_op_done[i] = GF_CALLOC (sizeof (*fd_ctx->pre_op_done[i]), priv->child_count, @@ -2697,8 +2805,6 @@ __afr_fd_ctx_set (xlator_t *this, fd_t *fd) fd_ctx->readdir_subvol = -1; - pthread_mutex_init (&fd_ctx->delay_lock, NULL); - INIT_LIST_HEAD (&fd_ctx->eager_locked); ret = __fd_ctx_set (fd, this, (uint64_t)(long) fd_ctx); @@ -2706,24 +2812,12 @@ __afr_fd_ctx_set (xlator_t *this, fd_t *fd) gf_msg_debug (this->name, 0, "failed to set fd ctx (%p)", fd); out: + if (ret && fd_ctx) + _afr_cleanup_fd_ctx (fd_ctx); return ret; } -int -afr_fd_ctx_set (xlator_t *this, fd_t *fd) -{ - int ret = -1; - - LOCK (&fd->lock); - { - ret = __afr_fd_ctx_set (this, fd); - } - UNLOCK (&fd->lock); - - return ret; -} - /* {{{ flush */ int @@ -2817,56 +2911,6 @@ out: /* }}} */ -int -afr_cleanup_fd_ctx (xlator_t *this, fd_t *fd) -{ - uint64_t ctx = 0; - afr_fd_ctx_t *fd_ctx = NULL; - int ret = 0; - int i = 0; - - ret = fd_ctx_get (fd, this, &ctx); - if (ret < 0) - goto out; - - fd_ctx = (afr_fd_ctx_t *)(long) ctx; - - if (fd_ctx) { - //no need to take any locks - if (!list_empty (&fd_ctx->eager_locked)) - gf_msg (this->name, GF_LOG_WARNING, 0, - AFR_MSG_INVALID_DATA, "%s: Stale " - "Eager-lock stubs found", - uuid_utoa (fd->inode->gfid)); - - for (i = 0; i < AFR_NUM_CHANGE_LOGS; i++) - GF_FREE (fd_ctx->pre_op_done[i]); - - GF_FREE (fd_ctx->opened_on); - - GF_FREE (fd_ctx->lock_piggyback); - - GF_FREE (fd_ctx->lock_acquired); - - pthread_mutex_destroy (&fd_ctx->delay_lock); - - GF_FREE (fd_ctx); - } - -out: - return 0; -} - - -int -afr_release (xlator_t *this, fd_t *fd) -{ - afr_cleanup_fd_ctx (this, fd); - - return 0; -} - - /* {{{ fsync */ int diff --git a/xlators/cluster/afr/src/afr-inode-read.c b/xlators/cluster/afr/src/afr-inode-read.c index a49d2568b82..58caaa5b7ab 100644 --- a/xlators/cluster/afr/src/afr-inode-read.c +++ b/xlators/cluster/afr/src/afr-inode-read.c @@ -620,10 +620,9 @@ unlock: goto unwind; } - unwind: - // Updating child_errno with more recent 'events' op_errno = afr_final_errno (local, priv); +unwind: AFR_STACK_UNWIND (fgetxattr, frame, op_ret, op_errno, xattr, xdata); if (xattr) @@ -707,10 +706,9 @@ unlock: goto unwind; } - unwind: - // Updating child_errno with more recent 'events' op_errno = afr_final_errno (local, priv); +unwind: AFR_STACK_UNWIND (getxattr, frame, op_ret, op_errno, xattr, xdata); if (xattr) diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index 2dc3448e064..9497ea204a3 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -1527,7 +1527,10 @@ afr_selfheal_do (call_frame_t *frame, xlator_t *this, uuid_t gfid) gf_boolean_t dataheal_enabled = _gf_false; priv = this->private; - gf_string2boolean (priv->data_self_heal, &dataheal_enabled); + + ret = gf_string2boolean (priv->data_self_heal, &dataheal_enabled); + if (ret) + goto out; ret = afr_selfheal_unlocked_inspect (frame, this, gfid, &inode, &data_selfheal, diff --git a/xlators/cluster/afr/src/afr-self-heal-data.c b/xlators/cluster/afr/src/afr-self-heal-data.c index 8b6e846ae01..d2becd8fe42 100644 --- a/xlators/cluster/afr/src/afr-self-heal-data.c +++ b/xlators/cluster/afr/src/afr-self-heal-data.c @@ -94,9 +94,15 @@ __afr_can_skip_data_block_heal (call_frame_t *frame, xlator_t *this, fd_t *fd, priv = this->private; local = frame->local; + xdata = dict_new(); - if (xdata) - i = dict_set_int32 (xdata, "check-zero-filled", 1); + if (!xdata) + goto out; + if (dict_set_int32 (xdata, "check-zero-filled", 1)) { + dict_unref (xdata); + goto out; + } + wind_subvols = alloca0 (priv->child_count); for (i = 0; i < priv->child_count; i++) { if (i == source || healed_sinks[i]) @@ -135,7 +141,7 @@ __afr_can_skip_data_block_heal (call_frame_t *frame, xlator_t *this, fd_t *fd, else return _gf_true; } - +out: return _gf_false; } diff --git a/xlators/cluster/afr/src/afr-self-heal-metadata.c b/xlators/cluster/afr/src/afr-self-heal-metadata.c index 014d53b38f4..a0cdf3a4457 100644 --- a/xlators/cluster/afr/src/afr-self-heal-metadata.c +++ b/xlators/cluster/afr/src/afr-self-heal-metadata.c @@ -85,6 +85,8 @@ __afr_selfheal_metadata_do (call_frame_t *frame, xlator_t *this, inode_t *inode, afr_delete_ignorable_xattrs (old_xattr); ret = syncop_removexattr (priv->children[i], &loc, "", old_xattr, NULL); + if (ret) + healed_sinks[i] = 0; } ret = syncop_setxattr (priv->children[i], &loc, xattr, 0, NULL, diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 63be1fb9d57..597e5c90fa7 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -899,9 +899,6 @@ afr_lk_transfer_datalock (call_frame_t *dst, call_frame_t *src, char *dom, int __afr_fd_ctx_set (xlator_t *this, fd_t *fd); -int -afr_fd_ctx_set (xlator_t *this, fd_t *fd); - afr_fd_ctx_t * afr_fd_ctx_get (fd_t *fd, xlator_t *this); diff --git a/xlators/cluster/afr/src/pump.c b/xlators/cluster/afr/src/pump.c index 46809cff11a..74c404cbf6c 100644 --- a/xlators/cluster/afr/src/pump.c +++ b/xlators/cluster/afr/src/pump.c @@ -2257,6 +2257,8 @@ init (xlator_t *this) 0, AFR_MSG_CHILD_MISCONFIGURED, "There should be exactly 2 children - one source " "and one sink"); + LOCK_DESTROY (&priv->lock); + GF_FREE (priv); return -1; } priv->child_count = child_count; |