From 3aff44fab8ba7a109bd691b8675dbfd9842237a3 Mon Sep 17 00:00:00 2001 From: Vikas Gorur Date: Fri, 27 Feb 2009 22:35:25 +0530 Subject: check for fd ctx set in changelog_needed_post_op for flush() in afr Earlier the check was in afr_flush(), which caused race conditions with writev() Signed-off-by: Anand V. Avati --- xlators/cluster/afr/src/afr-transaction.c | 46 ++++++++++++--- xlators/cluster/afr/src/afr.c | 98 ++++--------------------------- 2 files changed, 49 insertions(+), 95 deletions(-) (limited to 'xlators') diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index 6a1f228a6c0..82c2ee3406f 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -88,6 +88,26 @@ __is_first_write_on_fd (xlator_t *this, fd_t *fd) } +static int +__unset_fd_ctx_if_set (xlator_t *this, fd_t *fd) +{ + int op_ret = 0; + int _ret = -1; + + LOCK (&fd->inode->lock); + { + _ret = fd_ctx_get (fd, this, NULL); + if (_ret == 0) { + fd_ctx_del (fd, this, NULL); + op_ret = 1; + } + } + UNLOCK (&fd->inode->lock); + + return op_ret; +} + + static int __changelog_enabled (afr_private_t *priv, afr_transaction_type type) { @@ -170,19 +190,31 @@ __changelog_needed_post_op (call_frame_t *frame, xlator_t *this) afr_private_t * priv = NULL; afr_local_t * local = NULL; - int ret = 0; + int op_ret = 0; afr_transaction_type type = -1; priv = this->private; local = frame->local; type = local->transaction.type; - if (__changelog_enabled (priv, type) - && (local->op != GF_FOP_WRITE) - && (local->op != GF_FOP_FTRUNCATE)) - ret = 1; - - return ret; + if (__changelog_enabled (priv, type)) { + switch (local->op) { + + case GF_FOP_WRITE: + case GF_FOP_FTRUNCATE: + op_ret = 0; + break; + + case GF_FOP_FLUSH: + op_ret = __unset_fd_ctx_if_set (this, local->fd); + break; + + default: + op_ret = 1; + } + } + + return op_ret; } diff --git a/xlators/cluster/afr/src/afr.c b/xlators/cluster/afr/src/afr.c index da6a31f6f9a..acd7f8d0236 100644 --- a/xlators/cluster/afr/src/afr.c +++ b/xlators/cluster/afr/src/afr.c @@ -761,48 +761,6 @@ afr_flush_done (call_frame_t *frame, xlator_t *this) } -int -afr_simple_flush_cbk (call_frame_t *frame, void *cookie, - xlator_t *this, int32_t op_ret, int32_t op_errno) -{ - afr_local_t *local = NULL; - - int call_count = -1; - - local = frame->local; - - LOCK (&frame->lock); - { - if (op_ret == 0) - local->op_ret = 0; - - local->op_errno = op_errno; - } - UNLOCK (&frame->lock); - - call_count = afr_frame_return (frame); - - if (call_count == 0) - AFR_STACK_UNWIND (frame, local->op_ret, local->op_errno); - - return 0; -} - - -static int -__is_fd_ctx_set (xlator_t *this, fd_t *fd) -{ - int _ret = 0; - int op_ret = 0; - - _ret = fd_ctx_get (fd, this, NULL); - if (_ret == 0) - op_ret = 1; - - return op_ret; -} - - int afr_flush (call_frame_t *frame, xlator_t *this, fd_t *fd) { @@ -810,14 +768,10 @@ afr_flush (call_frame_t *frame, xlator_t *this, fd_t *fd) afr_local_t * local = NULL; int ret = -1; - int i = 0; - int call_count = 0; int op_ret = -1; int op_errno = 0; - int transaction_needed = 0; - VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); VALIDATE_OR_GOTO (this->private, out); @@ -834,50 +788,18 @@ afr_flush (call_frame_t *frame, xlator_t *this, fd_t *fd) frame->local = local; - LOCK (&fd->inode->lock); - { - if (__is_fd_ctx_set (this, fd)) { - transaction_needed = 1; - fd_ctx_del (fd, this, NULL); - } else { - transaction_needed = 0; - } - } - UNLOCK (&fd->inode->lock); - - if (transaction_needed) { - local->op = GF_FOP_FLUSH; - local->transaction.fop = afr_flush_wind; - local->transaction.done = afr_flush_done; - - local->fd = fd_ref (fd); - - local->transaction.start = 0; - local->transaction.len = 0; - - local->transaction.pending = AFR_DATA_PENDING; - - afr_transaction (frame, this, AFR_FLUSH_TRANSACTION); - } else { - /* - * if fd's ctx is not set, then there is no need - * to erase changelog. So just send the flush - */ + local->op = GF_FOP_FLUSH; + local->transaction.fop = afr_flush_wind; + local->transaction.done = afr_flush_done; - call_count = local->call_count; + local->fd = fd_ref (fd); - for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i]) { - STACK_WIND (frame, afr_simple_flush_cbk, - priv->children[i], - priv->children[i]->fops->flush, - fd); - - if (!--call_count) - break; - } - } - } + local->transaction.start = 0; + local->transaction.len = 0; + + local->transaction.pending = AFR_DATA_PENDING; + + afr_transaction (frame, this, AFR_FLUSH_TRANSACTION); op_ret = 0; out: -- cgit