summaryrefslogtreecommitdiffstats
path: root/xlators/features
diff options
context:
space:
mode:
authorvmallika <vmallika@redhat.com>2015-06-24 11:56:30 +0530
committerRaghavendra G <rgowdapp@redhat.com>2015-06-25 02:02:14 -0700
commit9b1305e549879e7698c46553bd91c722877d44cb (patch)
treeecd4541c3e022268bf9378927e09e901306a8019 /xlators/features
parenta1e32fbcfbfaf6e4c63e140b3b90a80dc748a269 (diff)
quota/marker: fix mem-leak in marker
When removing contribution xattr, we also need to free contribution node in memory. Use ref/unref mechanism to handle contribution node memory local->xdata should be freed in mq_local_unref There is another huge memory consumption happens in function mq_inspect_directory_xattr_task where dirty flag is not set. Change-Id: Ieca3ab4bf410c51259560e778bce4e81b9d888bf BUG: 1207735 Signed-off-by: vmallika <vmallika@redhat.com> Reviewed-on: http://review.gluster.org/11361 Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com> Tested-by: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Raghavendra G <rgowdapp@redhat.com> Tested-by: Raghavendra G <rgowdapp@redhat.com>
Diffstat (limited to 'xlators/features')
-rw-r--r--xlators/features/marker/src/marker-quota-helper.c81
-rw-r--r--xlators/features/marker/src/marker-quota-helper.h18
-rw-r--r--xlators/features/marker/src/marker-quota.c137
-rw-r--r--xlators/features/marker/src/marker-quota.h4
4 files changed, 157 insertions, 83 deletions
diff --git a/xlators/features/marker/src/marker-quota-helper.c b/xlators/features/marker/src/marker-quota-helper.c
index 9a0cf56a3fe..6265ffbac6e 100644
--- a/xlators/features/marker/src/marker-quota-helper.c
+++ b/xlators/features/marker/src/marker-quota-helper.c
@@ -125,6 +125,39 @@ out:
return ctx;
}
+void
+mq_contri_fini (void *data)
+{
+ inode_contribution_t *contri = data;
+
+ LOCK_DESTROY (&contri->lock);
+ GF_FREE (contri);
+}
+
+inode_contribution_t*
+mq_contri_init (inode_t *inode)
+{
+ inode_contribution_t *contri = NULL;
+ int32_t ret = 0;
+
+ QUOTA_ALLOC (contri, inode_contribution_t, ret);
+ if (ret == -1)
+ goto out;
+
+ GF_REF_INIT (contri, mq_contri_fini);
+
+ contri->contribution = 0;
+ contri->file_count = 0;
+ contri->dir_count = 0;
+ gf_uuid_copy (contri->gfid, inode->gfid);
+
+ LOCK_INIT (&contri->lock);
+ INIT_LIST_HEAD (&contri->contri_list);
+
+out:
+ return contri;
+}
+
inode_contribution_t *
mq_get_contribution_node (inode_t *inode, quota_inode_ctx_t *ctx)
{
@@ -134,35 +167,26 @@ mq_get_contribution_node (inode_t *inode, quota_inode_ctx_t *ctx)
if (!inode || !ctx)
goto out;
- list_for_each_entry (temp, &ctx->contribution_head, contri_list) {
- if (gf_uuid_compare (temp->gfid, inode->gfid) == 0) {
- contri = temp;
- goto out;
+ LOCK (&ctx->lock);
+ {
+ list_for_each_entry (temp, &ctx->contribution_head,
+ contri_list) {
+ if (gf_uuid_compare (temp->gfid, inode->gfid) == 0) {
+ contri = temp;
+ GF_REF_GET (contri);
+ break;
+ }
}
}
+ UNLOCK (&ctx->lock);
out:
return contri;
}
-
-int32_t
-mq_delete_contribution_node (dict_t *dict, char *key,
- inode_contribution_t *contribution)
-{
- if (dict_get (dict, key) != NULL)
- goto out;
-
- QUOTA_FREE_CONTRIBUTION_NODE (contribution);
-out:
- return 0;
-}
-
-
inode_contribution_t *
__mq_add_new_contribution_node (xlator_t *this, quota_inode_ctx_t *ctx,
loc_t *loc)
{
- int32_t ret = 0;
inode_contribution_t *contribution = NULL;
if (!loc->parent) {
@@ -185,17 +209,10 @@ __mq_add_new_contribution_node (xlator_t *this, quota_inode_ctx_t *ctx,
}
}
- QUOTA_ALLOC (contribution, inode_contribution_t, ret);
- if (ret == -1)
+ contribution = mq_contri_init (loc->parent);
+ if (contribution == NULL)
goto out;
- contribution->contribution = 0;
-
- gf_uuid_copy (contribution->gfid, loc->parent->gfid);
-
- LOCK_INIT (&contribution->lock);
- INIT_LIST_HEAD (&contribution->contri_list);
-
list_add_tail (&contribution->contri_list, &ctx->contribution_head);
out:
@@ -219,6 +236,8 @@ mq_add_new_contribution_node (xlator_t *this, quota_inode_ctx_t *ctx,
LOCK (&ctx->lock);
{
contribution = __mq_add_new_contribution_node (this, ctx, loc);
+ if (contribution)
+ GF_REF_GET (contribution);
}
UNLOCK (&ctx->lock);
@@ -387,6 +406,12 @@ mq_local_unref (xlator_t *this, quota_local_t *local)
if (local->fd != NULL)
fd_unref (local->fd);
+ if (local->contri)
+ GF_REF_PUT (local->contri);
+
+ if (local->xdata)
+ dict_unref (local->xdata);
+
loc_wipe (&local->loc);
loc_wipe (&local->parent_loc);
diff --git a/xlators/features/marker/src/marker-quota-helper.h b/xlators/features/marker/src/marker-quota-helper.h
index b333a0ec52f..7450a6e3604 100644
--- a/xlators/features/marker/src/marker-quota-helper.h
+++ b/xlators/features/marker/src/marker-quota-helper.h
@@ -13,10 +13,14 @@
#include "marker.h"
-#define QUOTA_FREE_CONTRIBUTION_NODE(_contribution) \
- do { \
- list_del (&_contribution->contri_list); \
- GF_FREE (_contribution); \
+#define QUOTA_FREE_CONTRIBUTION_NODE(ctx, _contribution) \
+ do { \
+ LOCK (&ctx->lock); \
+ { \
+ list_del (&_contribution->contri_list); \
+ GF_REF_PUT (_contribution); \
+ } \
+ UNLOCK (&ctx->lock); \
} while (0)
#define QUOTA_SAFE_INCREMENT(lock, var) \
@@ -62,6 +66,12 @@ mq_local_ref (quota_local_t *);
int32_t
mq_local_unref (xlator_t *, quota_local_t *);
+void
+mq_contri_fini (void *data);
+
+inode_contribution_t*
+mq_contri_init (inode_t *inode);
+
inode_contribution_t *
mq_get_contribution_node (inode_t *, quota_inode_ctx_t *);
diff --git a/xlators/features/marker/src/marker-quota.c b/xlators/features/marker/src/marker-quota.c
index c33d8a46707..90aec646116 100644
--- a/xlators/features/marker/src/marker-quota.c
+++ b/xlators/features/marker/src/marker-quota.c
@@ -882,7 +882,10 @@ mq_update_dirty_inode (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx,
local->ctx = ctx;
- local->contri = contribution;
+ if (contribution) {
+ local->contri = contribution;
+ GF_REF_GET (local->contri);
+ }
lock.l_type = F_WRLCK;
lock.l_whence = SEEK_SET;
@@ -1089,6 +1092,9 @@ free_value:
err:
dict_unref (dict);
+ if (contri)
+ GF_REF_PUT (contri);
+
out:
if (ret < 0) {
mq_xattr_creation_release_lock (frame, NULL, this, 0, 0, NULL);
@@ -1262,7 +1268,6 @@ mq_get_parent_inode_local (xlator_t *this, quota_local_t *local)
{
int32_t ret = -1;
quota_inode_ctx_t *ctx = NULL;
- inode_contribution_t *contribution = NULL;
GF_VALIDATE_OR_GOTO ("marker", this, out);
GF_VALIDATE_OR_GOTO ("marker", local, out);
@@ -1327,9 +1332,8 @@ mq_get_parent_inode_local (xlator_t *this, quota_local_t *local)
contribution will be at the end of the list. So get the proper
parent's contribution, by searching the entire list.
*/
- contribution = mq_get_contribution_node (local->loc.parent, ctx);
- GF_ASSERT (contribution != NULL);
- local->contri = contribution;
+ local->contri = mq_get_contribution_node (local->loc.parent, ctx);
+ GF_ASSERT (local->contri != NULL);
ret = 0;
out:
@@ -1992,7 +1996,10 @@ mq_prepare_txn_frame (xlator_t *this, loc_t *loc,
goto fr_destroy;
local->ctx = ctx;
- local->contri = contri;
+ if (contri) {
+ local->contri = contri;
+ GF_REF_GET (local->contri);
+ }
ret = 0;
*new_frame = frame;
@@ -2242,6 +2249,9 @@ out:
if (dict)
dict_unref (dict);
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -2569,7 +2579,8 @@ out:
}
int32_t
-mq_remove_contri (xlator_t *this, loc_t *loc, inode_contribution_t *contri)
+mq_remove_contri (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx,
+ inode_contribution_t *contri, quota_meta_t *delta)
{
int32_t ret = -1;
char contri_key[CONTRI_KEY_MAX] = {0, };
@@ -2583,7 +2594,8 @@ mq_remove_contri (xlator_t *this, loc_t *loc, inode_contribution_t *contri)
ret = syncop_removexattr (FIRST_CHILD(this), loc, contri_key, 0, NULL);
if (ret < 0) {
- if (-ret == ENOENT || -ret == ESTALE || -ret == ENODATA) {
+ if (-ret == ENOENT || -ret == ESTALE || -ret == ENODATA ||
+ -ret == ENOATTR) {
/* Remove contri in done when unlink operation is
* performed, so return success on ENOENT/ESTSLE
* rename operation removes xattr earlier,
@@ -2600,14 +2612,16 @@ mq_remove_contri (xlator_t *this, loc_t *loc, inode_contribution_t *contri)
LOCK (&contri->lock);
{
- contri->contribution = 0;
- contri->file_count = 0;
- contri->dir_count = 0;
+ contri->contribution += delta->size;
+ contri->file_count += delta->file_count;
+ contri->dir_count += delta->dir_count;
}
UNLOCK (&contri->lock);
ret = 0;
+
out:
+ QUOTA_FREE_CONTRIBUTION_NODE (ctx, contri);
return ret;
}
@@ -2804,10 +2818,12 @@ mq_start_quota_txn_v2 (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx,
gf_boolean_t status = _gf_true;
quota_meta_t delta = {0, };
+ GF_VALIDATE_OR_GOTO ("marker", contri, out);
+ GF_REF_GET (contri);
+
GF_VALIDATE_OR_GOTO ("marker", loc, out);
GF_VALIDATE_OR_GOTO ("marker", loc->inode, out);
GF_VALIDATE_OR_GOTO ("marker", ctx, out);
- GF_VALIDATE_OR_GOTO ("marker", contri, out);
ret = mq_loc_copy (&child_loc, loc);
if (ret < 0) {
@@ -2902,6 +2918,8 @@ mq_start_quota_txn_v2 (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx,
ret = -1;
goto out;
}
+
+ GF_REF_PUT (contri);
contri = mq_get_contribution_node (child_loc.parent, ctx);
GF_ASSERT (contri != NULL);
}
@@ -2918,6 +2936,8 @@ out:
loc_wipe (&child_loc);
loc_wipe (&parent_loc);
+ if (contri)
+ GF_REF_PUT (contri);
return ret;
}
@@ -3095,11 +3115,12 @@ mq_reduce_parent_size_task (void *opaque)
goto out;
dirty = _gf_true;
- ret = mq_remove_contri (this, loc, contribution);
+ mq_sub_meta (&delta, NULL);
+
+ ret = mq_remove_contri (this, loc, ctx, contribution, &delta);
if (ret < 0)
goto out;
- mq_sub_meta (&delta, NULL);
ret = mq_update_size (this, &parent_loc, &delta);
if (ret < 0)
goto out;
@@ -3116,6 +3137,9 @@ out:
loc_wipe (&parent_loc);
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -3198,6 +3222,9 @@ mq_initiate_quota_task (void *opaque)
ret = 0;
out:
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -3447,13 +3474,18 @@ mq_inspect_directory_xattr_task (void *opaque)
}
ret = dict_get_int8 (dict, QUOTA_DIRTY_KEY, &dirty);
- if (ret < 0)
- goto out;
+ if (ret < 0) {
+ /* dirty is set only on the first file write operation
+ * so ignore this error
+ */
+ ret = 0;
+ dirty = 0;
+ }
ret = _quota_dict_get_meta (this, dict, QUOTA_SIZE_KEY, &size,
IA_IFDIR, _gf_false);
if (ret < 0)
- goto out;
+ goto create_xattr;
if (!loc_is_root(loc)) {
GET_CONTRI_KEY (contri_key, contribution->gfid, ret);
@@ -3463,7 +3495,7 @@ mq_inspect_directory_xattr_task (void *opaque)
ret = _quota_dict_get_meta (this, dict, contri_key, &contri,
IA_IFDIR, _gf_false);
if (ret < 0)
- goto out;
+ goto create_xattr;
LOCK (&contribution->lock);
{
@@ -3494,11 +3526,15 @@ mq_inspect_directory_xattr_task (void *opaque)
mq_initiate_quota_blocking_txn (this, loc);
ret = 0;
-out:
+
+create_xattr:
if (ret < 0)
ret = mq_create_xattrs_blocking_txn (this, loc);
err:
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -3570,41 +3606,32 @@ mq_inspect_file_xattr_task (void *opaque)
}
UNLOCK (&ctx->lock);
- list_for_each_entry (contribution, &ctx->contribution_head,
- contri_list) {
-
- GET_CONTRI_KEY (contri_key, contribution->gfid, ret);
- if (ret < 0)
- continue;
-
- ret = _quota_dict_get_meta (this, dict, contri_key, &contri,
- IA_IFREG, _gf_true);
- if (ret < 0) {
- ret = mq_create_xattrs_blocking_txn (this, loc);
- } else {
- LOCK (&contribution->lock);
- {
- contribution->contribution = contri.size;
- contribution->file_count = contri.file_count;
- contribution->dir_count = contri.dir_count;
- }
- UNLOCK (&contribution->lock);
+ GET_CONTRI_KEY (contri_key, contribution->gfid, ret);
+ if (ret < 0)
+ goto out;
- mq_compute_delta (&delta, &size, &contri);
- if (!quota_meta_is_null (&delta)) {
- mq_initiate_quota_blocking_txn (this, loc);
- /* TODO: revist this code when fixing hardlinks
- */
- break;
- }
+ ret = _quota_dict_get_meta (this, dict, contri_key, &contri,
+ IA_IFREG, _gf_true);
+ if (ret < 0) {
+ ret = mq_create_xattrs_blocking_txn (this, loc);
+ } else {
+ LOCK (&contribution->lock);
+ {
+ contribution->contribution = contri.size;
+ contribution->file_count = contri.file_count;
+ contribution->dir_count = contri.dir_count;
}
+ UNLOCK (&contribution->lock);
- /* TODO: loc->parent might need to be assigned to corresponding
- * contribution inode. We need to handle hard links here
- */
+ mq_compute_delta (&delta, &size, &contri);
+ if (!quota_meta_is_null (&delta))
+ mq_initiate_quota_blocking_txn (this, loc);
}
+ /* TODO: revist this code when fixing hardlinks */
out:
+ if (contribution)
+ GF_REF_PUT (contribution);
return ret;
}
@@ -3947,7 +3974,10 @@ mq_reduce_parent_size (xlator_t *this, loc_t *loc, int64_t contri)
goto out;
local->ctx = ctx;
- local->contri = contribution;
+ if (contribution) {
+ local->contri = contribution;
+ GF_REF_GET (local->contri);
+ }
ret = mq_inode_loc_fill (NULL, loc->parent, &local->parent_loc);
if (ret < 0) {
@@ -3991,6 +4021,9 @@ out:
if (local != NULL)
mq_local_unref (this, local);
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -4025,6 +4058,10 @@ mq_rename_update_newpath (xlator_t *this, loc_t *loc)
mq_initiate_quota_txn (this, loc);
out:
+
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -4040,7 +4077,7 @@ mq_forget (xlator_t *this, quota_inode_ctx_t *ctx)
list_for_each_entry_safe (contri, next, &ctx->contribution_head,
contri_list) {
list_del (&contri->contri_list);
- GF_FREE (contri);
+ GF_REF_PUT (contri);
}
LOCK_DESTROY (&ctx->lock);
diff --git a/xlators/features/marker/src/marker-quota.h b/xlators/features/marker/src/marker-quota.h
index 9dfac2e3bb5..2a618976e75 100644
--- a/xlators/features/marker/src/marker-quota.h
+++ b/xlators/features/marker/src/marker-quota.h
@@ -12,6 +12,7 @@
#include "xlator.h"
#include "marker-mem-types.h"
+#include "refcount.h"
#define QUOTA_XATTR_PREFIX "trusted.glusterfs"
#define QUOTA_DIRTY_KEY "trusted.glusterfs.quota.dirty"
@@ -104,7 +105,8 @@ struct inode_contribution {
int64_t file_count;
int64_t dir_count;
uuid_t gfid;
- gf_lock_t lock;
+ gf_lock_t lock;
+ GF_REF_DECL;
};
typedef struct inode_contribution inode_contribution_t;