From c772f84e2b7925d0bb294877db3f68bd7cdfb1a3 Mon Sep 17 00:00:00 2001 From: Raghavendra G Date: Tue, 1 Nov 2016 11:54:27 +0530 Subject: 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 --- xlators/performance/quick-read/src/quick-read.c | 77 ++++++++++++++++--------- xlators/performance/quick-read/src/quick-read.h | 1 + 2 files changed, 51 insertions(+), 27 deletions(-) (limited to 'xlators/performance/quick-read') 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; -- cgit