summaryrefslogtreecommitdiffstats
path: root/xlators/features/marker
diff options
context:
space:
mode:
authorvmallika <vmallika@redhat.com>2015-06-24 11:56:30 +0530
committerRaghavendra G <rgowdapp@redhat.com>2015-06-26 22:32:37 -0700
commit08586ee518de438fe2bbbaa74ae4c9a02a5d88cf (patch)
treef8b760dec236c7ba28850a1c5d616ac43ee0a63e /xlators/features/marker
parentd1cfdc873e9c50c0ada8e57b646f14bbeb3c2ef3 (diff)
quota/marker: fix mem-leak in marker
This is a backport of http://review.gluster.org/#/c/11361/ > 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> Change-Id: I3038b41307f30867fa728054469ba917fd625e95 BUG: 1229282 Signed-off-by: vmallika <vmallika@redhat.com> Reviewed-on: http://review.gluster.org/11401 Tested-by: Gluster Build System <jenkins@build.gluster.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/marker')
-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 67801c8..a103bb9 100644
--- a/xlators/features/marker/src/marker-quota-helper.c
+++ b/xlators/features/marker/src/marker-quota-helper.c
@@ -130,6 +130,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)
{
@@ -139,35 +172,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) {
@@ -190,17 +214,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:
@@ -224,6 +241,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);
@@ -392,6 +411,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 161413d..f69447b 100644
--- a/xlators/features/marker/src/marker-quota-helper.h
+++ b/xlators/features/marker/src/marker-quota-helper.h
@@ -18,10 +18,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) \
@@ -67,6 +71,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 bb91fe2..411d21d 100644
--- a/xlators/features/marker/src/marker-quota.c
+++ b/xlators/features/marker/src/marker-quota.c
@@ -889,7 +889,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;
@@ -1096,6 +1099,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);
@@ -1269,7 +1275,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);
@@ -1334,9 +1339,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:
@@ -1999,7 +2003,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;
@@ -2249,6 +2256,9 @@ out:
if (dict)
dict_unref (dict);
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -2576,7 +2586,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, };
@@ -2590,7 +2601,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,
@@ -2607,14 +2619,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;
}
@@ -2811,10 +2825,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) {
@@ -2909,6 +2925,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);
}
@@ -2925,6 +2943,8 @@ out:
loc_wipe (&child_loc);
loc_wipe (&parent_loc);
+ if (contri)
+ GF_REF_PUT (contri);
return ret;
}
@@ -3102,11 +3122,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;
@@ -3123,6 +3144,9 @@ out:
loc_wipe (&parent_loc);
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -3205,6 +3229,9 @@ mq_initiate_quota_task (void *opaque)
ret = 0;
out:
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -3454,13 +3481,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);
@@ -3470,7 +3502,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);
{
@@ -3501,11 +3533,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;
}
@@ -3577,41 +3613,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;
}
@@ -3954,7 +3981,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) {
@@ -3998,6 +4028,9 @@ out:
if (local != NULL)
mq_local_unref (this, local);
+ if (contribution)
+ GF_REF_PUT (contribution);
+
return ret;
}
@@ -4032,6 +4065,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;
}
@@ -4047,7 +4084,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 a81db7f..2d03dfd 100644
--- a/xlators/features/marker/src/marker-quota.h
+++ b/xlators/features/marker/src/marker-quota.h
@@ -17,6 +17,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"
@@ -109,7 +110,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;