From 6a03c6c49c277d5dcf4ed9d4904361fe7ef305f7 Mon Sep 17 00:00:00 2001 From: Raghavendra G Date: Wed, 20 Jul 2011 11:07:13 +0530 Subject: performance/quick-read: Handle unwinding of frame corresponding to read fop properly, while validating cache. - there was a possibility of double unwind in case of errors. - use a new frame to do open in fd-based fops. In case of errors, qr_resume_pending_ops will be called to resume all the fops waiting on open. Hence if we use frame corresponding to fop (without creating a new one), there is a possibility of frame being freed by the time open would've returned to quick-read. Change-Id: I45a528e02b0886d22161ac24ab3e147a26d5ee7d BUG: 3168 Reviewed-on: http://review.gluster.com/53 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- xlators/performance/quick-read/src/quick-read.c | 335 ++++++++++++++++-------- 1 file changed, 220 insertions(+), 115 deletions(-) (limited to 'xlators/performance/quick-read/src/quick-read.c') diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c index 1ddc4e8f7..a674ba330 100644 --- a/xlators/performance/quick-read/src/quick-read.c +++ b/xlators/performance/quick-read/src/quick-read.c @@ -518,8 +518,6 @@ qr_open_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret, local = frame->local; if (local != NULL) { - local->op_ret = op_ret; - local->op_errno = op_errno; is_open = local->is_open; } @@ -584,6 +582,8 @@ qr_open_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret, out: if (is_open) { QR_STACK_UNWIND (open, frame, op_ret, op_errno, fd); + } else { + STACK_DESTROY (frame->root); } return 0; @@ -781,6 +781,10 @@ qr_validate_cache_cbk (call_frame_t *frame, void *cookie, xlator_t *this, unwind: /* this is actually unwind of readv */ + if ((local != NULL) && (local->stub != NULL)) { + call_stub_destroy (local->stub); + } + QR_STACK_UNWIND (readv, frame, op_ret, op_errno, NULL, -1, NULL, NULL); return 0; } @@ -817,15 +821,16 @@ int qr_validate_cache (call_frame_t *frame, xlator_t *this, fd_t *fd, call_stub_t *stub) { - int ret = -1; - int flags = 0; - uint64_t value = 0; - loc_t loc = {0, }; - char *path = NULL; - qr_local_t *local = NULL; - qr_fd_ctx_t *qr_fd_ctx = NULL; - call_stub_t *validate_stub = NULL; - char need_open = 0, can_wind = 0; + int ret = -1; + int flags = 0; + uint64_t value = 0; + loc_t loc = {0, }; + char *path = NULL; + qr_local_t *local = NULL; + qr_fd_ctx_t *qr_fd_ctx = NULL; + call_stub_t *validate_stub = NULL; + char need_open = 0, can_wind = 0, validate_cbk_called = 0; + call_frame_t *open_frame = NULL; if (frame->local == NULL) { local = GF_CALLOC (1, sizeof (*local), gf_qr_mt_qr_local_t); @@ -865,7 +870,9 @@ qr_validate_cache (call_frame_t *frame, xlator_t *this, fd_t *fd, fd); if (validate_stub == NULL) { ret = -1; - qr_fd_ctx->open_in_transit = 0; + if (need_open) { + qr_fd_ctx->open_in_transit = 0; + } goto unlock; } @@ -884,13 +891,22 @@ qr_validate_cache (call_frame_t *frame, xlator_t *this, fd_t *fd, } if (need_open) { + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + validate_cbk_called = 1; + goto out; + } + ret = qr_loc_fill (&loc, fd->inode, path); if (ret == -1) { qr_resume_pending_ops (qr_fd_ctx, -1, errno); + validate_cbk_called = 1; + STACK_DESTROY (open_frame->root); goto out; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -902,6 +918,13 @@ qr_validate_cache (call_frame_t *frame, xlator_t *this, fd_t *fd, ret = 0; out: + if ((ret < 0) && !validate_cbk_called) { + if (frame->local == NULL) { + call_stub_destroy (stub); + } + + qr_validate_cache_cbk (frame, NULL, this, -1, errno, NULL); + } return ret; } @@ -984,6 +1007,7 @@ qr_readv (call_frame_t *frame, xlator_t *this, fd_t *fd, size_t size, char just_validated = 0; qr_private_t *priv = NULL; qr_inode_table_t *table = NULL; + call_frame_t *open_frame = NULL; op_ret = 0; @@ -1130,13 +1154,7 @@ out: goto out; } - op_ret = qr_validate_cache (frame, this, fd, stub); - if (op_ret == -1) { - need_unwind = 1; - op_errno = errno; - call_stub_destroy (stub); - goto out; - } + qr_validate_cache (frame, this, fd, stub); } else { if (qr_fd_ctx) { LOCK (&qr_fd_ctx->lock); @@ -1200,7 +1218,14 @@ out: goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -1283,17 +1308,18 @@ int32_t qr_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, struct iovec *vector, int32_t count, off_t off, struct iobref *iobref) { - uint64_t value = 0; - int flags = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_inode_t *qr_inode = NULL; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t op_ret = -1, op_errno = -1, ret = -1; - char can_wind = 0, need_unwind = 0, need_open = 0; - qr_private_t *priv = NULL; - qr_inode_table_t *table = NULL; + uint64_t value = 0; + int flags = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_inode_t *qr_inode = NULL; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t op_ret = -1, op_errno = -1, ret = -1; + char can_wind = 0, need_unwind = 0, need_open = 0; + qr_private_t *priv = NULL; + qr_inode_table_t *table = NULL; + call_frame_t *open_frame = NULL; priv = this->private; table = &priv->table; @@ -1377,7 +1403,14 @@ qr_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, struct iovec *vector, goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -1441,14 +1474,15 @@ unwind: int32_t qr_fstat (call_frame_t *frame, xlator_t *this, fd_t *fd) { - qr_fd_ctx_t *qr_fd_ctx = NULL; - char need_open = 0, can_wind = 0, need_unwind = 0; - uint64_t value = 0; - int32_t ret = -1, op_ret = -1, op_errno = -1; - call_stub_t *stub = NULL; - loc_t loc = {0, }; - char *path = NULL; - int flags = 0; + qr_fd_ctx_t *qr_fd_ctx = NULL; + char need_open = 0, can_wind = 0, need_unwind = 0; + uint64_t value = 0; + int32_t ret = -1, op_ret = -1, op_errno = -1; + call_stub_t *stub = NULL; + loc_t loc = {0, }; + char *path = NULL; + int flags = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -1513,7 +1547,14 @@ qr_fstat (call_frame_t *frame, xlator_t *this, fd_t *fd) goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -1582,14 +1623,15 @@ int32_t qr_fsetattr (call_frame_t *frame, xlator_t *this, fd_t *fd, struct iatt *stbuf, int32_t valid) { - uint64_t value = 0; - int flags = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + uint64_t value = 0; + int flags = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -1655,7 +1697,14 @@ qr_fsetattr (call_frame_t *frame, xlator_t *this, fd_t *fd, goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -1722,14 +1771,15 @@ int32_t qr_fsetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *dict, int32_t flags) { - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - int open_flags = 0; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + int open_flags = 0; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -1796,7 +1846,14 @@ qr_fsetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *dict, goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, open_flags, fd, qr_fd_ctx->wbflags); @@ -1862,14 +1919,15 @@ unwind: int32_t qr_fgetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name) { - int flags = 0; - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + int flags = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; /* * FIXME: Can quick-read use the extended attributes stored in the @@ -1940,7 +1998,14 @@ qr_fgetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name) goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -2122,14 +2187,15 @@ int32_t qr_fentrylk (call_frame_t *frame, xlator_t *this, const char *volume, fd_t *fd, const char *basename, entrylk_cmd cmd, entrylk_type type) { - int flags = 0; - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + int flags = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -2197,7 +2263,14 @@ qr_fentrylk (call_frame_t *frame, xlator_t *this, const char *volume, fd_t *fd, goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -2265,14 +2338,15 @@ int32_t qr_finodelk (call_frame_t *frame, xlator_t *this, const char *volume, fd_t *fd, int32_t cmd, struct gf_flock *lock) { - int flags = 0; - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + int flags = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -2340,7 +2414,14 @@ qr_finodelk (call_frame_t *frame, xlator_t *this, const char *volume, fd_t *fd, goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -2404,14 +2485,15 @@ unwind: int32_t qr_fsync (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t flags) { - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - int open_flags = 0; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + int open_flags = 0; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -2476,7 +2558,14 @@ qr_fsync (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t flags) goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, open_flags, fd, qr_fd_ctx->wbflags); @@ -2585,15 +2674,16 @@ unwind: int32_t qr_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset) { - int flags = 0; - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_local_t *local = NULL; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + int flags = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_local_t *local = NULL; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -2661,7 +2751,14 @@ out: goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); @@ -2728,14 +2825,15 @@ int32_t qr_lk (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t cmd, struct gf_flock *lock) { - int flags = 0; - uint64_t value = 0; - call_stub_t *stub = NULL; - char *path = NULL; - loc_t loc = {0, }; - qr_fd_ctx_t *qr_fd_ctx = NULL; - int32_t ret = -1, op_ret = -1, op_errno = -1; - char need_open = 0, can_wind = 0, need_unwind = 0; + int flags = 0; + uint64_t value = 0; + call_stub_t *stub = NULL; + char *path = NULL; + loc_t loc = {0, }; + qr_fd_ctx_t *qr_fd_ctx = NULL; + int32_t ret = -1, op_ret = -1, op_errno = -1; + char need_open = 0, can_wind = 0, need_unwind = 0; + call_frame_t *open_frame = NULL; ret = fd_ctx_get (fd, this, &value); if (ret == 0) { @@ -2800,7 +2898,14 @@ qr_lk (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t cmd, goto ret; } - STACK_WIND (frame, qr_open_cbk, FIRST_CHILD(this), + open_frame = create_frame (this, this->ctx->pool); + if (open_frame == NULL) { + qr_resume_pending_ops (qr_fd_ctx, -1, ENOMEM); + qr_loc_wipe (&loc); + goto ret; + } + + STACK_WIND (open_frame, qr_open_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->open, &loc, flags, fd, qr_fd_ctx->wbflags); -- cgit