summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra G <rgowdapp@redhat.com>2016-11-01 11:54:27 +0530
committerRaghavendra G <rgowdapp@redhat.com>2018-05-28 09:58:52 +0000
commitc772f84e2b7925d0bb294877db3f68bd7cdfb1a3 (patch)
treec1e3781f769f5860185aeb1fd18ecf3ea41b3ce1
parent47440dd83592438840d3b2eb27f71403f6fba7b3 (diff)
performance/quick-read: Use generation numbers to avoid updating the cache with stale data
Thanks to Pranith for the example. Following is the race we are trying to solve with this patch. 1) We have a file with content 'abc' 2) lookup and writev which replaces 'abc' with 'def' comes. Lookup fetches abc but yet to update the cache, and then immediately writev is wound which zeros out the cache. Now lookup_cbk updates the buffer with 'abc' even though on disk it is 'def'. Now writev completes and returns to application. 3) application does a readv which will be fetched from quick-read as 'abc'. Change-Id: I9a9cab9c99652aa6d17230a4fe4dc034ec502b1b BUG: 1390050 Updates: bz#1390050 Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
-rw-r--r--xlators/performance/quick-read/src/quick-read.c77
-rw-r--r--xlators/performance/quick-read/src/quick-read.h1
2 files changed, 51 insertions, 27 deletions
diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c
index e1e3c2d0af7..c3581783365 100644
--- a/xlators/performance/quick-read/src/quick-read.c
+++ b/xlators/performance/quick-read/src/quick-read.c
@@ -15,9 +15,9 @@
#include "upcall-utils.h"
qr_inode_t *qr_inode_ctx_get (xlator_t *this, inode_t *inode);
-void __qr_inode_prune (xlator_t *this, qr_inode_table_t *table,
- qr_inode_t *qr_inode);
+void __qr_inode_prune (xlator_t *this, qr_inode_table_t *table,
+ qr_inode_t *qr_inode, uint64_t gen);
int
__qr_inode_ctx_set (xlator_t *this, inode_t *inode, qr_inode_t *qr_inode)
@@ -103,7 +103,7 @@ qr_inode_ctx_get_or_new (xlator_t *this, inode_t *inode)
ret = __qr_inode_ctx_set (this, inode, qr_inode);
if (ret) {
- __qr_inode_prune (this, &priv->table, qr_inode);
+ __qr_inode_prune (this, &priv->table, qr_inode, ~0);
GF_FREE (qr_inode);
qr_inode = NULL;
}
@@ -192,7 +192,9 @@ qr_inode_set_priority (xlator_t *this, inode_t *inode, const char *path)
/* To be called with priv->table.lock held */
void
-__qr_inode_prune (xlator_t *this, qr_inode_table_t *table, qr_inode_t *qr_inode)
+
+__qr_inode_prune (xlator_t *this, qr_inode_table_t *table, qr_inode_t *qr_inode,
+ uint64_t gen)
{
qr_private_t *priv = NULL;
@@ -201,6 +203,10 @@ __qr_inode_prune (xlator_t *this, qr_inode_table_t *table, qr_inode_t *qr_inode)
GF_FREE (qr_inode->data);
qr_inode->data = NULL;
+ /* Set gen only with valid callers */
+ if (gen != ~0)
+ qr_inode->gen = gen;
+
if (!list_empty (&qr_inode->lru)) {
table->cache_used -= qr_inode->size;
qr_inode->size = 0;
@@ -215,7 +221,7 @@ __qr_inode_prune (xlator_t *this, qr_inode_table_t *table, qr_inode_t *qr_inode)
void
-qr_inode_prune (xlator_t *this, inode_t *inode)
+qr_inode_prune (xlator_t *this, inode_t *inode, uint64_t gen)
{
qr_private_t *priv = NULL;
qr_inode_table_t *table = NULL;
@@ -230,7 +236,7 @@ qr_inode_prune (xlator_t *this, inode_t *inode)
LOCK (&table->lock);
{
- __qr_inode_prune (this, table, qr_inode);
+ __qr_inode_prune (this, table, qr_inode, gen);
}
UNLOCK (&table->lock);
}
@@ -250,7 +256,7 @@ __qr_cache_prune (xlator_t *this, qr_inode_table_t *table, qr_conf_t *conf)
size_pruned += curr->size;
- __qr_inode_prune (this, table, curr);
+ __qr_inode_prune (this, table, curr, ~0);
if (table->cache_used < conf->cache_size)
return;
@@ -306,7 +312,7 @@ out:
void
qr_content_update (xlator_t *this, qr_inode_t *qr_inode, void *data,
- struct iatt *buf)
+ struct iatt *buf, uint64_t gen)
{
qr_private_t *priv = NULL;
qr_inode_table_t *table = NULL;
@@ -316,7 +322,12 @@ qr_content_update (xlator_t *this, qr_inode_t *qr_inode, void *data,
LOCK (&table->lock);
{
- __qr_inode_prune (this, table, qr_inode);
+ /* allow for rollover of frame->root->unique */
+ if (gen && qr_inode->gen && (qr_inode->gen >= gen))
+ goto unlock;
+
+ qr_inode->gen = gen;
+ __qr_inode_prune (this, table, qr_inode, gen);
qr_inode->data = data;
qr_inode->size = buf->ia_size;
@@ -330,6 +341,7 @@ qr_content_update (xlator_t *this, qr_inode_t *qr_inode, void *data,
__qr_inode_register (this, table, qr_inode);
}
+unlock:
UNLOCK (&table->lock);
qr_cache_prune (this);
@@ -352,7 +364,8 @@ qr_mtime_equal (qr_inode_t *qr_inode, struct iatt *buf)
void
-__qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf)
+__qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf,
+ uint64_t gen)
{
qr_private_t *priv = NULL;
qr_inode_table_t *table = NULL;
@@ -362,6 +375,12 @@ __qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf)
table = &priv->table;
conf = &priv->conf;
+ /* allow for rollover of frame->root->unique */
+ if (gen && qr_inode->gen && (qr_inode->gen >= gen))
+ goto done;
+
+ qr_inode->gen = gen;
+
if (qr_size_fits (conf, buf) && qr_mtime_equal (qr_inode, buf)) {
qr_inode->buf = *buf;
@@ -369,15 +388,17 @@ __qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf)
__qr_inode_register (this, table, qr_inode);
} else {
- __qr_inode_prune (this, table, qr_inode);
+ __qr_inode_prune (this, table, qr_inode, gen);
}
+done:
return;
}
void
-qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf)
+qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf,
+ uint64_t gen)
{
qr_private_t *priv = NULL;
qr_inode_table_t *table = NULL;
@@ -387,7 +408,7 @@ qr_content_refresh (xlator_t *this, qr_inode_t *qr_inode, struct iatt *buf)
LOCK (&table->lock);
{
- __qr_content_refresh (this, qr_inode, buf);
+ __qr_content_refresh (this, qr_inode, buf, gen);
}
UNLOCK (&table->lock);
}
@@ -431,17 +452,17 @@ qr_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
frame->local = NULL;
if (op_ret == -1) {
- qr_inode_prune (this, inode);
+ qr_inode_prune (this, inode, ~0);
goto out;
}
if (dict_get (xdata, GLUSTERFS_BAD_INODE)) {
- qr_inode_prune (this, inode);
+ qr_inode_prune (this, inode, frame->root->unique);
goto out;
}
if (dict_get (xdata, "sh-failed")) {
- qr_inode_prune (this, inode);
+ qr_inode_prune (this, inode, frame->root->unique);
goto out;
}
@@ -455,7 +476,8 @@ qr_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
GF_FREE (content);
goto out;
}
- qr_content_update (this, qr_inode, content, buf);
+ qr_content_update (this, qr_inode, content, buf,
+ frame->root->unique);
} else {
/* purge old content if necessary */
qr_inode = qr_inode_ctx_get (this, inode);
@@ -463,7 +485,7 @@ qr_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
/* usual path for large files */
goto out;
- qr_content_refresh (this, qr_inode, buf);
+ qr_content_refresh (this, qr_inode, buf, frame->root->unique);
}
out:
if (inode)
@@ -539,7 +561,8 @@ qr_readdirp_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
/* no harm */
continue;
- qr_content_refresh (this, qr_inode, &entry->d_stat);
+ qr_content_refresh (this, qr_inode, &entry->d_stat,
+ frame->root->unique);
}
unwind:
@@ -661,7 +684,7 @@ qr_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, struct iovec *iov,
int count, off_t offset, uint32_t flags, struct iobref *iobref,
dict_t *xdata)
{
- qr_inode_prune (this, fd->inode);
+ qr_inode_prune (this, fd->inode, frame->root->unique);
STACK_WIND (frame, default_writev_cbk,
FIRST_CHILD (this), FIRST_CHILD (this)->fops->writev,
@@ -674,7 +697,7 @@ int
qr_truncate (call_frame_t *frame, xlator_t *this, loc_t *loc, off_t offset,
dict_t *xdata)
{
- qr_inode_prune (this, loc->inode);
+ qr_inode_prune (this, loc->inode, frame->root->unique);
STACK_WIND (frame, default_truncate_cbk,
FIRST_CHILD (this), FIRST_CHILD (this)->fops->truncate,
@@ -687,7 +710,7 @@ int
qr_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset,
dict_t *xdata)
{
- qr_inode_prune (this, fd->inode);
+ qr_inode_prune (this, fd->inode, frame->root->unique);
STACK_WIND (frame, default_ftruncate_cbk,
FIRST_CHILD (this), FIRST_CHILD (this)->fops->ftruncate,
@@ -699,7 +722,7 @@ static int
qr_fallocate (call_frame_t *frame, xlator_t *this, fd_t *fd, int keep_size,
off_t offset, size_t len, dict_t *xdata)
{
- qr_inode_prune (this, fd->inode);
+ qr_inode_prune (this, fd->inode, frame->root->unique);
STACK_WIND (frame, default_fallocate_cbk,
FIRST_CHILD (this), FIRST_CHILD (this)->fops->fallocate,
@@ -711,7 +734,7 @@ static int
qr_discard (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset,
size_t len, dict_t *xdata)
{
- qr_inode_prune (this, fd->inode);
+ qr_inode_prune (this, fd->inode, frame->root->unique);
STACK_WIND (frame, default_discard_cbk,
FIRST_CHILD (this), FIRST_CHILD (this)->fops->discard,
@@ -723,7 +746,7 @@ static int
qr_zerofill (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset,
off_t len, dict_t *xdata)
{
- qr_inode_prune (this, fd->inode);
+ qr_inode_prune (this, fd->inode, frame->root->unique);
STACK_WIND (frame, default_zerofill_cbk,
FIRST_CHILD (this), FIRST_CHILD (this)->fops->zerofill,
@@ -753,7 +776,7 @@ qr_forget (xlator_t *this, inode_t *inode)
if (!qr_inode)
return 0;
- qr_inode_prune (this, inode);
+ qr_inode_prune (this, inode, ~0);
GF_FREE (qr_inode);
@@ -1236,7 +1259,7 @@ qr_invalidate (xlator_t *this, void *data)
ret = -1;
goto out;
}
- qr_inode_prune (this, inode);
+ qr_inode_prune (this, inode, ~0);
}
out:
diff --git a/xlators/performance/quick-read/src/quick-read.h b/xlators/performance/quick-read/src/quick-read.h
index 7db483d35de..cdbf3dfbd63 100644
--- a/xlators/performance/quick-read/src/quick-read.h
+++ b/xlators/performance/quick-read/src/quick-read.h
@@ -39,6 +39,7 @@ struct qr_inode {
struct iatt buf;
struct timeval last_refresh;
struct list_head lru;
+ uint64_t gen;
};
typedef struct qr_inode qr_inode_t;