From a42137eee3c9e340ac9c82ebacca14eeb4b9d912 Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Thu, 15 Feb 2018 16:12:12 +0530 Subject: features/shard: Fix shard inode refcount when it's part of priv->lru_list. For as long as a shard's inode is in priv->lru_list, it should have a non-zero ref-count. This patch achieves it by taking a ref on the inode when it is added to lru list. When it's time for the inode to be evicted from the lru list, a corresponding unref is done. Change-Id: I289ffb41e7be5df7489c989bc1bbf53377433c86 BUG: 1468483 Signed-off-by: Krutika Dhananjay --- tests/basic/inode-leak.t | 17 ----------------- tests/bugs/shard/shard-inode-refcount-test.t | 27 +++++++++++++++++++++++++++ tests/volume.rc | 18 ++++++++++++++++++ xlators/features/shard/src/shard.c | 26 +++++++++++++++++--------- 4 files changed, 62 insertions(+), 26 deletions(-) create mode 100644 tests/bugs/shard/shard-inode-refcount-test.t diff --git a/tests/basic/inode-leak.t b/tests/basic/inode-leak.t index f73d2af6898..6bfbf572f03 100644 --- a/tests/basic/inode-leak.t +++ b/tests/basic/inode-leak.t @@ -3,23 +3,6 @@ . $(dirname $0)/../include.rc . $(dirname $0)/../volume.rc -function get_mount_active_size_value { - local vol=$1 - local statedump=$(generate_mount_statedump $vol) - sleep 1 - local val=$(grep "active_size" $statedump | cut -f2 -d'=' | tail -1) - rm -f $statedump - echo $val -} - -function get_mount_lru_size_value { - local vol=$1 - local statedump=$(generate_mount_statedump $vol) - sleep 1 - local val=$(grep "lru_size" $statedump | cut -f2 -d'=' | tail -1) - rm -f $statedump - echo $val -} cleanup TEST glusterd diff --git a/tests/bugs/shard/shard-inode-refcount-test.t b/tests/bugs/shard/shard-inode-refcount-test.t new file mode 100644 index 00000000000..635809724cf --- /dev/null +++ b/tests/bugs/shard/shard-inode-refcount-test.t @@ -0,0 +1,27 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume create $V0 $H0:$B0/${V0}0 +TEST $CLI volume set $V0 features.shard on +TEST $CLI volume set $V0 features.shard-block-size 4MB +TEST $CLI volume start $V0 + +TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0 + +TEST dd if=/dev/zero of=$M0/one-plus-five-shards bs=1M count=23 + +ACTIVE_INODES_BEFORE=$(get_mount_active_size_value $V0) +TEST rm -f $M0/one-plus-five-shards +EXPECT `expr $ACTIVE_INODES_BEFORE - 5` get_mount_active_size_value $V0 + +EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 +TEST $CLI volume stop $V0 +TEST $CLI volume delete $V0 + +cleanup diff --git a/tests/volume.rc b/tests/volume.rc index 3ee83624058..add812d4779 100644 --- a/tests/volume.rc +++ b/tests/volume.rc @@ -812,3 +812,21 @@ function get_fd_count { rm -f $statedump echo $count } + +function get_mount_active_size_value { + local vol=$1 + local statedump=$(generate_mount_statedump $vol) + sleep 1 + local val=$(grep "active_size" $statedump | cut -f2 -d'=' | tail -1) + rm -f $statedump + echo $val +} + +function get_mount_lru_size_value { + local vol=$1 + local statedump=$(generate_mount_statedump $vol) + sleep 1 + local val=$(grep "lru_size" $statedump | cut -f2 -d'=' | tail -1) + rm -f $statedump + echo $val +} diff --git a/xlators/features/shard/src/shard.c b/xlators/features/shard/src/shard.c index 03504ab2743..48a87b8f72f 100644 --- a/xlators/features/shard/src/shard.c +++ b/xlators/features/shard/src/shard.c @@ -502,6 +502,10 @@ __shard_update_shards_inode_list (inode_t *linked_inode, xlator_t *this, * by empty list), and if there is still space in the priv list, * add this ctx to the tail of the list. */ + /* For as long as an inode is in lru list, we try to + * keep it alive by holding a ref on it. + */ + inode_ref (linked_inode); gf_uuid_copy (ctx->base_gfid, base_inode->gfid); ctx->block_num = block_num; list_add_tail (&ctx->ilist, &priv->ilist_head); @@ -527,8 +531,16 @@ __shard_update_shards_inode_list (inode_t *linked_inode, xlator_t *this, /* The following unref corresponds to the ref held by * inode_find() above. */ - inode_forget (lru_inode, 0); inode_unref (lru_inode); + /* The following unref corresponds to the ref held at + * the time the shard was created or looked up + */ + inode_unref (lru_inode); + inode_forget (lru_inode, 0); + /* For as long as an inode is in lru list, we try to + * keep it alive by holding a ref on it. + */ + inode_ref (linked_inode); gf_uuid_copy (ctx->base_gfid, base_inode->gfid); ctx->block_num = block_num; list_add_tail (&ctx->ilist, &priv->ilist_head); @@ -1658,11 +1670,6 @@ shard_link_block_inode (shard_local_t *local, int block_num, inode_t *inode, buf); inode_lookup (linked_inode); list_index = block_num - local->first_block; - - /* Defer unref'ing the inodes until write is complete. These inodes are - * unref'd in the event of a failure or after successful fop completion - * in shard_local_wipe(). - */ local->inode_list[list_index] = linked_inode; LOCK(&priv->lock); @@ -2520,10 +2527,11 @@ shard_unlink_block_inode (shard_local_t *local, int shard_block_num) if (!list_empty (&ctx->ilist)) { list_del_init (&ctx->ilist); priv->inode_count--; + GF_ASSERT (priv->inode_count >= 0); + inode_unlink (inode, priv->dot_shard_inode, block_bname); + inode_unref (inode); + inode_forget (inode, 0); } - GF_ASSERT (priv->inode_count >= 0); - inode_unlink (inode, priv->dot_shard_inode, block_bname); - inode_forget (inode, 0); } UNLOCK(&priv->lock); -- cgit