From 1dde7f9f6a555c7e461ec67ce4f9858ef7decc8d Mon Sep 17 00:00:00 2001 From: Soumya Koduri Date: Thu, 4 Jun 2015 11:25:35 +0530 Subject: [PATCH 30/57] Upcall/cache-invalidation: Ignore fops with frame->root->client not set This is a backport of the fix reviewed upstream on - http://review.gluster.org/10909 Server-side internally generated fops like 'quota/marker' will not have any client associated with the frame. Hence we need a check for clients to be valid before processing for upcall cache invalidation. Also fixed an issue with initializing reaper-thread. Added a testcase to test the fix. BUG: 1224183 Change-Id: I5042238cf52380e06c69adb3bee874df9b445f95 Signed-off-by: Soumya Koduri Reviewed-on: http://review.gluster.org/10909 Reviewed-by: Kaleb KEITHLEY Reviewed-by: jiffin tony Thottan Reviewed-by: Niels de Vos Reviewed-on: https://code.engineering.redhat.com/gerrit/50369 Reviewed-by: Jiffin Thottan Tested-by: Niels de Vos --- tests/bugs/upcall/bug-1227204.t | 29 +++++++++++++++++++++++ xlators/features/upcall/src/upcall-internal.c | 25 +++++++++++++++---- xlators/features/upcall/src/upcall.c | 4 ++- xlators/mgmt/glusterd/src/glusterd-volume-set.c | 2 +- 4 files changed, 53 insertions(+), 7 deletions(-) create mode 100755 tests/bugs/upcall/bug-1227204.t diff --git a/tests/bugs/upcall/bug-1227204.t b/tests/bugs/upcall/bug-1227204.t new file mode 100755 index 0000000..fc393b1 --- /dev/null +++ b/tests/bugs/upcall/bug-1227204.t @@ -0,0 +1,29 @@ +#!/bin/bash + +# This regression test tries to ensure that quota limit-usage set work with +# features.cache-invalidation on. + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup; + +TEST glusterd +TEST pidof glusterd; +TEST $CLI volume info; + +TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{1,2,3,4,5,6}; +TEST $CLI volume start $V0; + +TEST $CLI volume set $V0 features.cache-invalidation on; +TEST $CLI volume quota $V0 enable; + +TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0; + +TEST mkdir -p $M0/1/2; +TEST $CLI volume quota $V0 limit-usage /1/2 100MB 70%; + +TEST $CLI volume status $V0 +TEST $CLI volume stop $V0 + +cleanup; diff --git a/xlators/features/upcall/src/upcall-internal.c b/xlators/features/upcall/src/upcall-internal.c index 19c9f9e..e13097c 100644 --- a/xlators/features/upcall/src/upcall-internal.c +++ b/xlators/features/upcall/src/upcall-internal.c @@ -206,10 +206,13 @@ __upcall_inode_ctx_set (inode_t *inode, xlator_t *this) INIT_LIST_HEAD (&inode_ctx->client_list); inode_ctx->destroy = 0; - ret = __inode_ctx_set (inode, this, (uint64_t *) inode_ctx); - if (ret) + ctx = (long) inode_ctx; + ret = __inode_ctx_set (inode, this, &ctx); + if (ret) { gf_log (this->name, GF_LOG_DEBUG, "failed to set inode ctx (%p)", inode); + goto out; + } /* add this inode_ctx to the global list */ LOCK (&priv->inode_ctx_lk); @@ -241,7 +244,7 @@ __upcall_inode_ctx_get (inode_t *inode, xlator_t *this) goto out; } - inode_ctx = (upcall_inode_ctx_t *)(long) ctx; + inode_ctx = (upcall_inode_ctx_t *) (long) (ctx); out: return inode_ctx; @@ -377,10 +380,10 @@ upcall_cleanup_inode_ctx (xlator_t *this, inode_t *inode) } pthread_mutex_unlock (&inode_ctx->client_list_lock); - pthread_mutex_destroy (&inode_ctx->client_list_lock); - /* Mark the inode_ctx to be destroyed */ inode_ctx->destroy = 1; + gf_msg_debug ("upcall", 0, "set upcall_inode_ctx (%p) to destroy mode", + inode_ctx); } out: @@ -422,8 +425,12 @@ upcall_reaper_thread (void *data) LOCK (&priv->inode_ctx_lk); { /* client list would have been cleaned up*/ + gf_msg_debug ("upcall", 0, "Freeing upcall_inode_ctx (%p)", + inode_ctx); list_del_init (&inode_ctx->inode_ctx_list); + pthread_mutex_destroy (&inode_ctx->client_list_lock); GF_FREE (inode_ctx); + inode_ctx = NULL; } UNLOCK (&priv->inode_ctx_lk); } @@ -474,6 +481,14 @@ upcall_cache_invalidate (call_frame_t *frame, xlator_t *this, client_t *client, if (!is_cache_invalidation_enabled(this)) return; + /* server-side generated fops like quota/marker will not have any + * client associated with them. Ignore such fops. + */ + if (!client) { + gf_msg_debug ("upcall", 0, "Internal fop - client NULL"); + return; + } + up_inode_ctx = ((upcall_local_t *)frame->local)->upcall_inode_ctx; if (!up_inode_ctx) diff --git a/xlators/features/upcall/src/upcall.c b/xlators/features/upcall/src/upcall.c index 5f6c29e..331db52 100644 --- a/xlators/features/upcall/src/upcall.c +++ b/xlators/features/upcall/src/upcall.c @@ -1581,8 +1581,8 @@ upcall_local_wipe (xlator_t *this, upcall_local_t *local) { if (local) { inode_unref (local->inode); - mem_put (local); loc_wipe (&local->rename_oldloc); + mem_put (local); } } @@ -1634,6 +1634,7 @@ reconfigure (xlator_t *this, dict_t *options) " Disabling cache_invalidation", strerror(errno)); } + priv->reaper_init_done = 1; } out: @@ -1680,6 +1681,7 @@ init (xlator_t *this) " Disabling cache_invalidation", strerror(errno)); } + priv->reaper_init_done = 1; } out: if (ret) { diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c index 9b9f544..8dea069 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c @@ -1863,7 +1863,7 @@ struct volopt_map_entry glusterd_volopt_map[] = { /* Upcall translator options */ { .key = "features.cache-invalidation", .voltype = "features/upcall", - .value = "off", + .value = "off", .op_version = GD_OP_VERSION_3_7_0, }, { .key = "features.cache-invalidation-timeout", -- 1.7.1