pkg/cloudfs.spec.in | 2
xlators/performance/xattr-prefetch/src/xattr-prefetch-mem-types.h | 7
xlators/performance/xattr-prefetch/src/xattr-prefetch.c | 288 +++++++---
xlators/performance/xattr-prefetch/src/xattr-prefetch.h | 23
4 files changed, 237 insertions(+), 83 deletions(-)
New commits:
commit 600baf631510c6f095cf1f3a03fba19a5f50b87f
Author: Jeff Darcy <jdarcy(a)redhat.com>
Date: Fri May 6 10:55:24 2011 -0400
Move cache to xlator, fix leak, add stats.
From review suggestions. The cache is now xlator-global and fixed size (a
mere 1024 entries which should be tunable some day). It's managed as a
four-way set-associative cache with LRU. I also fixed a memory leak and
added statistics accessible via getxattr(trusted.xattr.stats). This shows
a hit rate of nearly 99.9% for my 100K-file test despite the small cache
size, with approximately three hits and one eviction per file - exactly as
it should be as the oldest entries constantly get pushed out.
diff --git a/pkg/cloudfs.spec.in b/pkg/cloudfs.spec.in
index 31a8db9..4401777 100644
--- a/pkg/cloudfs.spec.in
+++ b/pkg/cloudfs.spec.in
@@ -46,7 +46,7 @@ with additional authentication/encryption/multi-tenancy features.
# Remove unwanted files from all the shared libraries
find %{buildroot}%{_libdir} -name '*.a' -delete
find %{buildroot}%{_libdir} -name '*.la' -delete
-find %{buildroot}%{_libdir} -name '*.so.0.0.0' | xargs strip
+#find %{buildroot}%{_libdir} -name '*.so.0.0.0' | xargs strip
%clean
%{__rm} -rf %{buildroot}
diff --git a/xlators/performance/xattr-prefetch/src/xattr-prefetch-mem-types.h
b/xlators/performance/xattr-prefetch/src/xattr-prefetch-mem-types.h
index 046b782..89a0a31 100644
--- a/xlators/performance/xattr-prefetch/src/xattr-prefetch-mem-types.h
+++ b/xlators/performance/xattr-prefetch/src/xattr-prefetch-mem-types.h
@@ -23,9 +23,10 @@
#include "mem-types.h"
enum gf_xp_mem_types_ {
- gf_xp_mt_xp_local_t = gf_common_mt_end + 1,
- gf_xp_mt_xp_inode_ctx_t,
- gf_xp_mt_xp_private_t,
+ gf_mt_xp_local_t = gf_common_mt_end + 1,
+ gf_mt_xp_inode_ctx_t,
+ gf_mt_xp_fd_ctx_t,
+ gf_mt_xp_private_t,
gf_xp_mt_loc_t,
gf_xp_mt_end
};
diff --git a/xlators/performance/xattr-prefetch/src/xattr-prefetch.c
b/xlators/performance/xattr-prefetch/src/xattr-prefetch.c
index 8aad401..389dc4f 100644
--- a/xlators/performance/xattr-prefetch/src/xattr-prefetch.c
+++ b/xlators/performance/xattr-prefetch/src/xattr-prefetch.c
@@ -1,4 +1,6 @@
/*
+* }
+* }
* Copyright (c) 2011 Red Hat, Inc.
*
* This file is part of CloudFS.
@@ -22,19 +24,121 @@
int32_t
xp_forget (xlator_t *this, inode_t *inode)
{
- struct iatt *buf = NULL;
- uint64_t value = 0;
+ xp_inode_ctx_t *inode_ctx = NULL;
+ uint32_t i = 0;
+ xp_private_t *priv = this->private;
+
+ inode_ctx = priv->table
+ + ((inode->ino >> 8) % XP_NUM_BUCKETS) * XP_BUCKET_SIZE;
+
+ LOCK(&priv->lock);
+ for (i = 0; i < XP_BUCKET_SIZE; ++i) {
+ if (inode_ctx[i].inode == inode) {
+ if (inode_ctx[i].attrs) {
+ dict_unref(inode_ctx[i].attrs);
+ inode_ctx[i].attrs = NULL;
+ }
+ inode_ctx[i].fd = NULL;
+ inode_ctx[i].inode = NULL;
+ break;
+ }
+ }
+ UNLOCK(&priv->lock);
- inode_ctx_del (inode, this, &value);
+ return 0;
+}
- if (value) {
- buf = (void *)(long)value;
- GF_FREE (buf);
- }
+int32_t
+xp_release (xlator_t *this, fd_t *fd)
+{
+ uint32_t i = 0;
+ xp_private_t *priv = this->private;
+ xp_inode_ctx_t *inode_ctx = priv->table;
+
+ LOCK(&priv->lock);
+ for (i = 0; i < XP_BUCKET_SIZE; ++i) {
+ if (inode_ctx[i].fd == fd) {
+ if (inode_ctx[i].attrs) {
+ dict_unref(inode_ctx[i].attrs);
+ inode_ctx[i].attrs = NULL;
+ }
+ inode_ctx[i].fd = NULL;
+ inode_ctx[i].inode = NULL;
+ break;
+ }
+ }
+ UNLOCK(&priv->lock);
return 0;
}
+dict_t *
+xp_get_inode_ctx (xlator_t *this, fd_t * fd, inode_t *inode, dict_t *newdata)
+{
+ time_t cutoff = time(NULL) - 2;
+ xp_inode_ctx_t *inode_ctx = NULL;
+ uint32_t i = 0;
+ dict_t *dict = NULL;
+ xp_private_t *priv = this->private;
+ uint32_t e_index = 0;
+ uint64_t e_clock = 0;
+
+ inode_ctx = priv->table
+ + ((inode->ino >> 8) % XP_NUM_BUCKETS) * XP_BUCKET_SIZE;
+ LOCK(&priv->lock);
+
+ for (i = 0; i < XP_BUCKET_SIZE; ++i) {
+ if (inode_ctx[i].inode == inode) {
+ goto found;
+ }
+ }
+
+ if (newdata) {
+ for (i = 0; i < XP_BUCKET_SIZE; ++i) {
+ if (!inode_ctx[i].inode) {
+ break;
+ }
+ if (inode_ctx[i].created <= cutoff) {
+ break;
+ }
+ }
+ if (i >= XP_BUCKET_SIZE) {
+ e_clock = inode_ctx[0].used;
+ for (i = 1; i < XP_BUCKET_SIZE; ++i) {
+ if (inode_ctx[i].used < e_clock) {
+ e_clock - inode_ctx[i].used;
+ e_index = i;
+ }
+ }
+ ++(priv->stats.evict);
+ i = e_index;
+ }
+ inode_ctx[i].fd = fd;
+ inode_ctx[i].inode = inode;
+ inode_ctx[i].created = time(NULL);
+ }
+
+found:
+ if (i < XP_BUCKET_SIZE) {
+ dict = inode_ctx[i].attrs;
+ if (newdata) {
+ if (dict) {
+ dict_unref(dict);
+ }
+ dict = newdata;
+ inode_ctx[i].attrs = dict;
+ }
+ if (dict) {
+ /* Keep one reference, give one away. */
+ dict_ref(dict);
+ }
+ inode_ctx[i].used = ++(priv->lru_clock);
+ }
+
+ UNLOCK(&priv->lock);
+ return dict;
+}
+
void
xp_local_free (xp_local_t *local)
{
@@ -54,54 +158,51 @@ xp_pre_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t
*this,
int32_t op_ret, int32_t op_errno, inode_t *inode,
struct iatt *buf, dict_t *dict, struct iatt *postparent)
{
- uint64_t value = 0;
- xp_inode_ctx_t *inode_ctx = NULL;
+ loc_t *loc = cookie;
+ fd_t *fd = NULL;
/* The cookie is the loc we built in xp_issue_one_lookup. */
/* TBD: free inode/buf/dict/postparent? */
- gf_log(this->name,GF_LOG_DEBUG,"finished pre-lookup for %s",
- ((loc_t *)cookie)->path);
+ gf_log(this->name,GF_LOG_DEBUG,"finished pre-lookup for %s",loc->path);
if (!dict) {
goto done;
}
dict_foreach(dict,xp_show_dict_val,NULL);
- if (inode_ctx_get (inode, this, &value) == 0) {
- inode_ctx = (xp_inode_ctx_t *)(long)value;
- }
- else {
- inode_ctx = GF_CALLOC(1,sizeof(*inode_ctx),
- gf_xp_mt_xp_inode_ctx_t);
- if (!inode_ctx) {
- goto done;
- }
- value = (uint64_t)inode_ctx;
- if (inode_ctx_put(inode,this,value) != 0) {
- GF_FREE(inode_ctx);
- goto done;
- }
+ fd = fd_lookup(loc->parent,frame->root->pid);
+ if (!fd) {
+ goto done;
}
- dict_ref(dict);
- inode_ctx->stbuf = *buf;
- inode_ctx->attrs = dict;
- inode_ctx->when = time(NULL);
+ (void)xp_get_inode_ctx(this,fd,inode,dict);
+ dict_unref(dict);
done:
- GF_FREE(cookie);
+ /*
+ * This was allocated along with the structure itself, but there's
+ * no flag to tell loc_wipe that so clear the pointer first.
+ */
+ loc->path = NULL;
+ loc_wipe(loc);
+ GF_FREE(loc);
STACK_DESTROY(frame->root);
return 0;
}
loc_t *
-xp_issue_one_lookup (xlator_t *this, xp_local_t *local,
+xp_issue_one_lookup (xlator_t *this, xp_local_t *local, pid_t pid,
char *path, size_t path_len, gf_dirent_t *entry)
{
loc_t *loc = NULL;
call_frame_t *frame = NULL;
xp_private_t *priv = this->private;
dict_t *dict = NULL;
+ int junk = 0; /* just to shut up warn_unused_result */
+ /*
+ * Allocate the path along with the structure to avoid the waste of
+ * calling malloc twice when once will do.
+ */
loc = GF_MALLOC(sizeof(*loc)+path_len+strlen(entry->d_name)+2,
gf_xp_mt_loc_t);
if (!loc) {
@@ -128,13 +229,15 @@ xp_issue_one_lookup (xlator_t *this, xp_local_t *local,
"could not allocate frame for %s", this->name);
goto err_no_frame;
}
+ /* Make sure these match so fd_lookup will find the right fd. */
+ frame->root->pid = pid;
dict = dict_new();
if (dict) {
- (void)dict_set_uint32(dict,"security.capability",100);
- (void)dict_set_uint32(dict,"security.selinux",100);
- (void)dict_set_uint32(dict,"system.posix_acl_access",100);
- (void)dict_set_uint32(dict,"system.posix_acl_default",100);
+ junk = dict_set_uint32(dict,"security.capability",100);
+ junk = dict_set_uint32(dict,"security.selinux",100);
+ junk = dict_set_uint32(dict,"system.posix_acl_access",100);
+ junk = dict_set_uint32(dict,"system.posix_acl_default",100);
}
STACK_WIND_COOKIE(frame, xp_pre_lookup_cbk, loc,
@@ -150,7 +253,7 @@ err_no_inode:
}
void
-xp_issue_lookups (xlator_t *this, xp_local_t *local,
+xp_issue_lookups (xlator_t *this, xp_local_t *local, pid_t pid,
gf_dirent_t *entries, int32_t n_entries)
{
gf_dirent_t *entry = NULL;
@@ -190,7 +293,7 @@ xp_issue_lookups (xlator_t *this, xp_local_t *local,
}
gf_log(this->name,GF_LOG_DEBUG,"prefetching d_name = %s",
entry->d_name);
- loc = xp_issue_one_lookup(this,local,path,path_len,entry);
+ loc = xp_issue_one_lookup(this,local,pid,path,path_len,entry);
if (!loc) {
break;
}
@@ -216,7 +319,7 @@ xp_readdir_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
}
if (op_ret >= 0) {
- xp_issue_lookups(this,local,entries,op_ret);
+ xp_issue_lookups(this,local,frame->root->pid,entries,op_ret);
}
XP_STACK_UNWIND (readdir, frame, op_ret, op_errno, entries);
@@ -230,7 +333,7 @@ xp_readdir (call_frame_t *frame, xlator_t *this, fd_t *fd, size_t
size,
{
xp_local_t *local = NULL;
- local = GF_CALLOC (1, sizeof (*local), gf_xp_mt_xp_local_t);
+ local = GF_CALLOC (1, sizeof (*local), gf_mt_xp_local_t);
if (local) {
local->fd = fd;
frame->local = local;
@@ -272,41 +375,61 @@ xp_is_prefetch_xattr (char *key)
int32_t
xp_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name)
{
- uint64_t value = 0;
- xp_inode_ctx_t *inode_ctx = NULL;
data_t *data = NULL;
- dict_t *the_dict = NULL;
+ dict_t *old_dict = NULL;
+ dict_t *new_dict = NULL;
+ xp_private_t *priv = this->private;
+ fd_t *fd = NULL;
+ int junk = 0; /* just to shut up warn_unused_result */
gf_log(this->name,GF_LOG_DEBUG,"looking up %s on %p",name,loc->inode);
- if (__inode_ctx_get(loc->inode, this, &value) == 0) {
- inode_ctx = (xp_inode_ctx_t *)(long)value;
+ if (!strcmp(name,"trusted.xattr.stats")) {
+ new_dict = dict_new();
+ if (!new_dict) {
+ XP_STACK_UNWIND(getxattr,frame,-1,ENOENT,NULL);
+ return 0;
+ }
+ junk = dict_set_static_bin(new_dict,(char *)name,
+ &priv->stats,sizeof(priv->stats));
+ XP_STACK_UNWIND(getxattr,frame,0,0,new_dict);
+ return 0;
}
- if (inode_ctx && ((time(NULL) - inode_ctx->when) >= 2)) {
- gf_log(this->name,GF_LOG_DEBUG,"context too old");
- (void)xp_forget(this,loc->inode);
- inode_ctx = NULL;
+ if (!xp_is_prefetch_xattr((char *)name)) {
+ goto not_ours;
+ }
+
+ fd = fd_lookup(loc->parent,frame->root->pid);
+ if (!fd) {
+ goto miss;
}
- if (inode_ctx && inode_ctx->attrs) {
- the_dict = dict_new();
- if (the_dict) {
- data = dict_get(inode_ctx->attrs,(char *)name);
+
+ old_dict = xp_get_inode_ctx(this,fd,loc->inode,NULL);
+ if (old_dict) {
+ new_dict = dict_new();
+ if (new_dict) {
+ data = dict_get(old_dict,(char *)name);
if (data) {
gf_log(this->name,GF_LOG_DEBUG,
"filled from cache!");
+ ++(priv->stats.data_hit);
data_ref(data);
- dict_set(the_dict,(char *)name,data);
- XP_STACK_UNWIND(getxattr,frame,0,0,the_dict);
+ dict_set(new_dict,(char *)name,data);
+ XP_STACK_UNWIND(getxattr,frame,0,0,new_dict);
return 0;
}
- else if (xp_is_prefetch_xattr((char *)name)) {
+ else {
gf_log(this->name,GF_LOG_DEBUG,
"filled (null) from cache!");
- XP_STACK_UNWIND(getxattr,frame,0,0,the_dict);
+ ++(priv->stats.null_hit);
+ XP_STACK_UNWIND(getxattr,frame,0,0,new_dict);
return 0;
}
}
}
+miss:
+ ++(priv->stats.miss);
+not_ours:
STACK_WIND (frame, xp_getxattr_cbk, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->getxattr, loc, name);
return 0;
@@ -383,16 +506,16 @@ init (xlator_t *this)
goto out;
}
- priv = GF_CALLOC (1, sizeof(xp_private_t), gf_xp_mt_xp_private_t);
+ priv = GF_CALLOC (1, sizeof(xp_private_t), gf_mt_xp_private_t);
- priv->pool.stack_mem_pool = mem_pool_new(call_stack_t,100);
+ priv->pool.stack_mem_pool = mem_pool_new(call_stack_t,1000);
if (!priv->pool.stack_mem_pool) {
gf_log(this->name,GF_LOG_ERROR,
"could not allocate call stacks");
goto err_no_stacks;
}
- priv->pool.frame_mem_pool = mem_pool_new(call_frame_t,100);
+ priv->pool.frame_mem_pool = mem_pool_new(call_frame_t,1000);
if (!priv->pool.frame_mem_pool) {
gf_log(this->name,GF_LOG_ERROR,
"could not allocate call frames");
@@ -401,6 +524,7 @@ init (xlator_t *this)
INIT_LIST_HEAD(&priv->pool.all_frames);
LOCK_INIT(&priv->pool.lock);
+ LOCK_INIT(&priv->lock);
this->private = priv;
@@ -418,24 +542,35 @@ void
fini (xlator_t *this)
{
xp_private_t *priv = NULL;
+ uint32_t i = 0;
- if (!this)
- goto out;
- else {
- priv = this->private;
- if (priv) {
- if (priv->pool.stack_mem_pool) {
- mem_pool_destroy (priv->pool.stack_mem_pool);
- }
- if (priv->pool.frame_mem_pool) {
- mem_pool_destroy (priv->pool.frame_mem_pool);
- }
- GF_FREE (priv);
- this->private = NULL;
- }
- }
-out:
- return;
+ if (!this) {
+ /* Does this ever really happen? Seems crazy. */
+ return;
+ }
+
+ priv = this->private;
+ if (!priv) {
+ return;
+ }
+
+ if (priv->pool.stack_mem_pool) {
+ mem_pool_destroy (priv->pool.stack_mem_pool);
+ }
+ if (priv->pool.frame_mem_pool) {
+ mem_pool_destroy (priv->pool.frame_mem_pool);
+ }
+
+ LOCK(&priv->lock);
+ for (i = 0; i < XP_NUM_ENTRIES; ++i) {
+ if (priv->table[i].attrs) {
+ dict_unref(priv->table[i].attrs);
+ }
+ }
+ UNLOCK(&priv->lock);
+
+ GF_FREE (priv);
+ this->private = NULL;
}
struct xlator_fops fops = {
@@ -449,5 +584,6 @@ struct xlator_fops fops = {
struct xlator_cbks cbks = {
.forget = xp_forget,
+ .release = xp_release,
};
diff --git a/xlators/performance/xattr-prefetch/src/xattr-prefetch.h
b/xlators/performance/xattr-prefetch/src/xattr-prefetch.h
index 6e98888..c0567fc 100644
--- a/xlators/performance/xattr-prefetch/src/xattr-prefetch.h
+++ b/xlators/performance/xattr-prefetch/src/xattr-prefetch.h
@@ -40,16 +40,33 @@ struct xp_local {
typedef struct xp_local xp_local_t;
struct xp_inode_ctx {
- struct iatt stbuf;
+ fd_t *fd;
+ inode_t *inode;
dict_t *attrs;
- time_t when;
+ time_t created;
+ time_t used;
};
typedef struct xp_inode_ctx xp_inode_ctx_t;
+struct xp_stats {
+ uint64_t miss;
+ uint64_t data_hit;
+ uint64_t null_hit;
+ uint64_t evict;
+};
+typedef struct xp_stats xp_stats_t;
+
+#define XP_BUCKET_SIZE 4
+#define XP_NUM_BUCKETS 256
+#define XP_NUM_ENTRIES (XP_BUCKET_SIZE * XP_NUM_BUCKETS)
+
struct xp_private {
call_pool_t pool;
uint32_t entries;
gf_lock_t lock;
+ xp_inode_ctx_t table[XP_NUM_ENTRIES];
+ uint64_t lru_clock;
+ xp_stats_t stats;
};
typedef struct xp_private xp_private_t;
@@ -58,7 +75,7 @@ void xp_local_free (xp_local_t *local);
#define XP_STACK_UNWIND(op, frame, params ...) do { \
xp_local_t *__local = frame->local; \
frame->local = NULL; \
- STACK_UNWIND_STRICT (op, frame, params); \
+ STACK_UNWIND_STRICT (op, frame, ##params); \
xp_local_free (__local); \
} while (0)