From 2f31a7673b559f7764f9bb44a89e161cb4242711 Mon Sep 17 00:00:00 2001
From: vmallika <vmallika@redhat.com>
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 <vmallika@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/52658
Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>
Tested-by: Raghavendra Gowdappa <rgowdapp@redhat.com>
---
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