From 2f31a7673b559f7764f9bb44a89e161cb4242711 Mon Sep 17 00:00:00 2001 From: vmallika Date: Thu, 9 Jul 2015 15:14:14 +0530 Subject: [PATCH 220/234] quota/marker: fix mem leak in marker This is a backport of http://review.gluster.org/#/c/11457/ Part of the fix is available in: https://code.engineering.redhat.com/gerrit/#/c/52303/ This patch optimizes the me consumption. create syntask txn only for linked inodes Change-Id: I2f9eefc115ce36b3fac29746655fa3c8cecbbcab BUG: 1224177 Signed-off-by: vmallika Reviewed-on: https://code.engineering.redhat.com/gerrit/52658 Reviewed-by: Raghavendra Gowdappa Tested-by: Raghavendra Gowdappa --- xlators/features/marker/src/marker-quota.c | 161 +++++++++++++++++----------- 1 files changed, 96 insertions(+), 65 deletions(-) diff --git a/xlators/features/marker/src/marker-quota.c b/xlators/features/marker/src/marker-quota.c index 97946f8..400e9ae 100644 --- a/xlators/features/marker/src/marker-quota.c +++ b/xlators/features/marker/src/marker-quota.c @@ -2841,6 +2841,49 @@ out: return ret; } +int32_t +mq_prevalidate_txn (xlator_t *this, loc_t *origin_loc, loc_t *loc, + quota_inode_ctx_t **ctx) +{ + int32_t ret = -1; + quota_inode_ctx_t *ctxtmp = NULL; + + if (origin_loc == NULL || origin_loc->inode == NULL || + (gf_uuid_is_null(origin_loc->gfid) && + gf_uuid_is_null(origin_loc->inode->gfid))) + goto out; + + loc_copy (loc, origin_loc); + + if (gf_uuid_is_null (loc->gfid)) + gf_uuid_copy (loc->gfid, loc->inode->gfid); + + if (ctx) + ret = mq_inode_ctx_get (loc->inode, this, ctx); + else + ret = mq_inode_ctx_get (loc->inode, this, &ctxtmp); + + if (ret < 0) { + if (ctx) { + *ctx = mq_inode_ctx_new (loc->inode, this); + if (*ctx == NULL) { + gf_log_callingfn (this->name, GF_LOG_WARNING, + "mq_inode_ctx_new failed for " + "%s", loc->path); + ret = -1; + goto out; + } + } else { + gf_log_callingfn (this->name, GF_LOG_WARNING, "ctx for " + "is NULL for %s", loc->path); + } + } + + ret = 0; +out: + return ret; +} + int mq_start_quota_txn_v2 (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx, inode_contribution_t *contri) @@ -2866,16 +2909,6 @@ mq_start_quota_txn_v2 (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx, goto out; } - if (gf_uuid_is_null (child_loc.gfid)) - gf_uuid_copy (child_loc.gfid, child_loc.inode->gfid); - - if (gf_uuid_is_null (child_loc.gfid)) { - ret = -1; - gf_log (this->name, GF_LOG_DEBUG, "UUID is null for %s", - child_loc.path); - goto out; - } - while (!__is_root_gfid (child_loc.gfid)) { /* To improve performance, abort current transaction * if one is already in progress for same inode @@ -3005,16 +3038,6 @@ mq_create_xattrs_task (void *opaque) this = args->this; THIS = this; - if (gf_uuid_is_null (loc->gfid)) - gf_uuid_copy (loc->gfid, loc->inode->gfid); - - if (gf_uuid_is_null (loc->gfid)) { - ret = -1; - gf_log (this->name, GF_LOG_DEBUG, "UUID is null for %s", - loc->path); - goto out; - } - ret = mq_inode_ctx_get (loc->inode, this, &ctx); if (ret < 0) { gf_log (this->name, GF_LOG_WARNING, "Failed to" @@ -3056,34 +3079,30 @@ out: } static int -_mq_create_xattrs_txn (xlator_t *this, loc_t *loc, gf_boolean_t spawn) +_mq_create_xattrs_txn (xlator_t *this, loc_t *origin_loc, gf_boolean_t spawn) { int32_t ret = -1; quota_inode_ctx_t *ctx = NULL; gf_boolean_t status = _gf_true; + loc_t loc = {0, }; - ret = mq_inode_ctx_get (loc->inode, this, &ctx); - if (ret < 0) { - ctx = mq_inode_ctx_new (loc->inode, this); - if (ctx == NULL) { - gf_log (this->name, GF_LOG_WARNING, - "mq_inode_ctx_new failed"); - ret = -1; - goto out; - } - } + ret = mq_prevalidate_txn (this, origin_loc, &loc, &ctx); + if (ret < 0) + goto out; ret = mq_test_and_set_ctx_create_status (ctx, &status); if (ret < 0 || status == _gf_true) goto out; - ret = mq_synctask (this, mq_create_xattrs_task, spawn, loc, 0); + ret = mq_synctask (this, mq_create_xattrs_task, spawn, &loc, 0); out: if (ret < 0 && status == _gf_false) mq_set_ctx_create_status (ctx, _gf_false); + loc_wipe (&loc); return ret; } + int mq_create_xattrs_txn (xlator_t *this, loc_t *loc) { @@ -3237,17 +3256,33 @@ out: } int32_t -mq_reduce_parent_size_txn (xlator_t *this, loc_t *loc, int64_t contri) +mq_reduce_parent_size_txn (xlator_t *this, loc_t *origin_loc, int64_t contri) { int32_t ret = -1; + loc_t loc = {0, }; GF_VALIDATE_OR_GOTO ("marker", this, out); - GF_VALIDATE_OR_GOTO ("marker", loc, out); - GF_VALIDATE_OR_GOTO ("marker", loc->inode, out); + GF_VALIDATE_OR_GOTO ("marker", origin_loc, out); + + ret = mq_prevalidate_txn (this, origin_loc, &loc, NULL); + if (ret < 0) + goto out; + + if (loc_is_root(&loc)) { + ret = 0; + goto out; + } - ret = mq_synctask (this, mq_reduce_parent_size_task, _gf_true, loc, + if (loc.parent == NULL) { + gf_log (this->name, GF_LOG_WARNING, "parent is NULL for %s, " + "aborting reduce parent size txn", loc.path); + goto out; + } + + ret = mq_synctask (this, mq_reduce_parent_size_task, _gf_true, &loc, contri); out: + loc_wipe (&loc); return ret; } @@ -3325,16 +3360,25 @@ out: } int -_mq_initiate_quota_txn (xlator_t *this, loc_t *loc, gf_boolean_t spawn) +_mq_initiate_quota_txn (xlator_t *this, loc_t *origin_loc, gf_boolean_t spawn) { int32_t ret = -1; quota_inode_ctx_t *ctx = NULL; gf_boolean_t status = _gf_true; + loc_t loc = {0,}; - ret = mq_inode_ctx_get (loc->inode, this, &ctx); - if (ret == -1) { - gf_log (this->name, GF_LOG_WARNING, - "inode ctx get failed, aborting quota txn"); + ret = mq_prevalidate_txn (this, origin_loc, &loc, &ctx); + if (ret < 0) + goto out; + + if (loc_is_root(&loc)) { + ret = 0; + goto out; + } + + if (loc.parent == NULL) { + gf_log (this->name, GF_LOG_WARNING, "parent is NULL for %s, " + "aborting updation txn", loc.path); goto out; } @@ -3342,12 +3386,13 @@ _mq_initiate_quota_txn (xlator_t *this, loc_t *loc, gf_boolean_t spawn) if (ret < 0 || status == _gf_true) goto out; - ret = mq_synctask (this, mq_initiate_quota_task, spawn, loc, 0); + ret = mq_synctask (this, mq_initiate_quota_task, spawn, &loc, 0); out: if (ret < 0 && status == _gf_false) mq_set_ctx_updation_status (ctx, _gf_false); + loc_wipe (&loc); return ret; } @@ -3407,16 +3452,6 @@ mq_update_dirty_inode_task (void *opaque) this = args->this; THIS = this; - if (gf_uuid_is_null (loc->gfid)) - gf_uuid_copy (loc->gfid, loc->inode->gfid); - - if (gf_uuid_is_null (loc->gfid)) { - ret = -1; - gf_log (this->name, GF_LOG_DEBUG, "UUID is null for %s", - loc->path); - goto out; - } - ret = mq_lock (this, loc, F_WRLCK); if (ret < 0) goto out; @@ -3707,29 +3742,25 @@ out: } int32_t -mq_xattr_state (xlator_t *this, loc_t *loc, dict_t *dict, struct iatt buf) +mq_xattr_state (xlator_t *this, loc_t *origin_loc, dict_t *dict, + struct iatt buf) { int32_t ret = -1; quota_inode_ctx_t *ctx = NULL; + loc_t loc = {0, }; - ret = mq_inode_ctx_get (loc->inode, this, &ctx); - if (ret < 0) { - ctx = mq_inode_ctx_new (loc->inode, this); - if (ctx == NULL) { - gf_log (this->name, GF_LOG_WARNING, - "mq_inode_ctx_new failed"); - ret = -1; - goto out; - } - } + ret = mq_prevalidate_txn (this, origin_loc, &loc, &ctx); + if (ret < 0) + goto out; if (((buf.ia_type == IA_IFREG) && !dht_is_linkfile (&buf, dict)) || (buf.ia_type == IA_IFLNK)) { - mq_inspect_file_xattr (this, ctx, loc, dict, buf); + mq_inspect_file_xattr (this, ctx, &loc, dict, buf); } else if (buf.ia_type == IA_IFDIR) - mq_inspect_directory_xattr (this, loc, ctx, dict, buf); + mq_inspect_directory_xattr (this, &loc, ctx, dict, buf); out: + loc_wipe (&loc); return ret; } -- 1.7.1