From 85ed82893aca7dd6307100660b9bba0233d52240 Mon Sep 17 00:00:00 2001 From: Vikas Gorur Date: Tue, 1 Dec 2009 06:11:06 +0000 Subject: cluster/afr: Don't do memcpy of afr_local_t in afr_local_copy. For the background self-heal frame's local_t, copy only required members --- not a wholesale memcpy. The memcpy lead to pointers being copied and then double free'd. Signed-off-by: Vikas Gorur Signed-off-by: Anand V. Avati BUG: 320 (Improve self-heal performance) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=320 --- xlators/cluster/afr/src/afr-dir-read.c | 8 -------- xlators/cluster/afr/src/afr-self-heal-common.c | 16 +++++----------- xlators/cluster/afr/src/afr-self-heal-data.c | 3 ++- 3 files changed, 7 insertions(+), 20 deletions(-) (limited to 'xlators/cluster/afr/src') diff --git a/xlators/cluster/afr/src/afr-dir-read.c b/xlators/cluster/afr/src/afr-dir-read.c index 24227c33b..fab60b66e 100644 --- a/xlators/cluster/afr/src/afr-dir-read.c +++ b/xlators/cluster/afr/src/afr-dir-read.c @@ -59,9 +59,6 @@ afr_examine_dir_sh_unwind (call_frame_t *frame, xlator_t *this) afr_set_opendir_done (this, local->fd->inode); - /* let self-heal's local cleanup free this */ - local->cont.opendir.checksum = NULL; - AFR_STACK_UNWIND (opendir, frame, local->op_ret, local->op_errno, local->fd); @@ -153,11 +150,6 @@ out: if (__checksums_differ (local->cont.opendir.checksum, priv->child_count)) { - /* self-heal will call AFR_STACK_DESTROY and - thus unref local->fd, so ref it here */ - - local->fd = fd_ref (local->fd); - sh->need_entry_self_heal = _gf_true; sh->forced_merge = _gf_true; sh->mode = local->fd->inode->st_mode; diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index 327cab032..721f69a8d 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -1393,7 +1393,9 @@ afr_local_t *afr_local_copy (afr_local_t *l, xlator_t *this) lc = CALLOC (1, sizeof (afr_local_t)); - memcpy (lc, l, sizeof (afr_local_t)); +// memcpy (lc, l, sizeof (afr_local_t)); + + lc->self_heal = l->self_heal; if (l->loc.path) loc_copy (&lc->loc, &l->loc); @@ -1403,8 +1405,8 @@ afr_local_t *afr_local_copy (afr_local_t *l, xlator_t *this) lc->xattr_req = dict_copy_with_ref (l->xattr_req, NULL); lc->cont.lookup.inode = l->cont.lookup.inode; - if (l->cont.lookup.xattr) - lc->cont.lookup.xattr = dict_copy_with_ref (l->cont.lookup.xattr, NULL); +// if (l->cont.lookup.xattr) +// lc->cont.lookup.xattr = dict_copy_with_ref (l->cont.lookup.xattr, NULL); return lc; } @@ -1440,14 +1442,6 @@ afr_self_heal_completion_cbk (call_frame_t *bgsh_frame, xlator_t *this) UNLOCK (&priv->lock); } - /* - * XXX Hack: Due to memcpy'ing of local, some pointers will - * also have ended up in bgsh_frame's local. We shouldn't free - * them. So set them to NULL here. - */ - - local->cont.writev.vector = NULL; - AFR_STACK_DESTROY (bgsh_frame); return 0; diff --git a/xlators/cluster/afr/src/afr-self-heal-data.c b/xlators/cluster/afr/src/afr-self-heal-data.c index dfe586588..c8397b2ae 100644 --- a/xlators/cluster/afr/src/afr-self-heal-data.c +++ b/xlators/cluster/afr/src/afr-self-heal-data.c @@ -762,7 +762,8 @@ afr_self_heal_get_source (xlator_t *this, afr_local_t *local, dict_t **xattr) sh->pending_matrix[i] = CALLOC (sizeof (int32_t), priv->child_count); } - sh->sources = CALLOC (priv->child_count, sizeof (*sh->sources)); + + sh->sources = CALLOC (priv->child_count, sizeof (*sh->sources)); afr_sh_build_pending_matrix (priv, sh->pending_matrix, xattr, priv->child_count, AFR_DATA_TRANSACTION); -- cgit