diff options
| author | Pranith Kumar K <pkarampu@redhat.com> | 2016-03-28 16:31:12 +0530 | 
|---|---|---|
| committer | Pranith Kumar Karampuri <pkarampu@redhat.com> | 2016-03-31 05:46:32 -0700 | 
| commit | b2a5eed9b17a82ec4b6366b0107fe2271328c16a (patch) | |
| tree | 044ae61060c37e7a1885b033b8999dbd5be8a6a6 | |
| parent | 16a3f0d020d23978b22a07354c25c654c88163a2 (diff) | |
cluster/afr: Don't lookup/forget inodes
Problem:
All inodes that are looked-up are always forgotten without fail in
afr removing the benefits of them being in lru. This same code can
cause crashes if between inode_lookup, inode_forget in afr if the
top xlator does inode_forget(0).
Fix:
Don't use lookup/forget in afr. No benefits are there at the moment
for keeping this code. It is impossible to prevent top xlators to
do inode_forget(0). Found similar instances in ec
and removed them even though those code paths are not going to
be executed in any place other than heal-daemon.
BUG: 1321554
Change-Id: Ia4cb236178f7f129cc898d53f0bbd26f494a2a8d
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/13834
Smoke: Gluster Build System <jenkins@build.gluster.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anuradha Talur <atalur@redhat.com>
| -rw-r--r-- | xlators/cluster/afr/src/afr-common.c | 7 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heal-common.c | 20 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heal.h | 3 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heald.c | 7 | ||||
| -rw-r--r-- | xlators/cluster/ec/src/ec-heald.c | 12 | 
5 files changed, 6 insertions, 43 deletions
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 8c04cfbff2f..f4d74c57a5d 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -1948,12 +1948,11 @@ afr_lookup_sh_metadata_wrap (void *opaque)          if (first == -1)                  goto out; -        inode = afr_inode_link (local->inode,&replies[first].poststat); +        inode = inode_link (local->inode, NULL, NULL, &replies[first].poststat);          if(!inode)                  goto out;          afr_selfheal_metadata (frame, this, inode); -        inode_forget (inode, 1);          inode_unref (inode);          afr_local_replies_wipe (local, this->private); @@ -4834,10 +4833,8 @@ out:          AFR_STACK_UNWIND (getxattr, frame, ret, op_errno, dict, NULL);          if (dict)                 dict_unref (dict); -        if (inode) { -                inode_forget (inode, 1); +        if (inode)                  inode_unref (inode); -        }          GF_FREE (substr);          return ret;  } diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index 151960d2516..3f4e9703546 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -1153,20 +1153,6 @@ afr_is_entry_set (xlator_t *this, dict_t *xdata)  	return afr_is_pending_set (this, xdata, AFR_ENTRY_TRANSACTION);  } - -inode_t* -afr_inode_link (inode_t *inode, struct iatt *iatt) -{ -	inode_t *linked_inode = NULL; - -	linked_inode = inode_link (inode, NULL, NULL, iatt); - -	if (linked_inode) -		inode_lookup (linked_inode); -        return linked_inode; -} - -  /*   * This function inspects the looked up replies (in an unlocked manner)   * and decides whether a locked verification and possible healing is @@ -1293,7 +1279,7 @@ afr_selfheal_unlocked_inspect (call_frame_t *frame, xlator_t *this,  	}  	if (valid_cnt > 0 && link_inode) { -		*link_inode = afr_inode_link (inode, &first); +		*link_inode = inode_link (inode, NULL, NULL, &first);                  if (!*link_inode) {                          ret = -EINVAL;                          goto out; @@ -1454,10 +1440,8 @@ afr_selfheal_do (call_frame_t *frame, xlator_t *this, uuid_t gfid)                  ret = 0;  out: -        if (inode) { -                inode_forget (inode, 1); +        if (inode)                  inode_unref (inode); -        }          return ret;  }  /* diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h index b0f545f2816..0bee7a023ad 100644 --- a/xlators/cluster/afr/src/afr-self-heal.h +++ b/xlators/cluster/afr/src/afr-self-heal.h @@ -189,9 +189,6 @@ afr_selfheal_newentry_mark (call_frame_t *frame, xlator_t *this, inode_t *inode,                              int source, struct afr_reply *replies,                              unsigned char *sources, unsigned char *newentry); -inode_t* -afr_inode_link (inode_t *inode, struct iatt *iatt); -  unsigned int  afr_success_count (struct afr_reply *replies, unsigned int count); diff --git a/xlators/cluster/afr/src/afr-self-heald.c b/xlators/cluster/afr/src/afr-self-heald.c index 228ea388662..9439bba63ff 100644 --- a/xlators/cluster/afr/src/afr-self-heald.c +++ b/xlators/cluster/afr/src/afr-self-heald.c @@ -159,10 +159,8 @@ afr_shd_inode_find (xlator_t *this, xlator_t *subvol, uuid_t gfid)  	struct iatt iatt = {0, };  	inode = inode_find (this->itable, gfid); -	if (inode) { -                inode_lookup (inode); +	if (inode)  		goto out; -        }  	loc.inode = inode_new (this->itable);  	if (!loc.inode) @@ -174,8 +172,6 @@ afr_shd_inode_find (xlator_t *this, xlator_t *subvol, uuid_t gfid)  		goto out;  	inode = inode_link (loc.inode, NULL, NULL, &iatt); -	if (inode) -		inode_lookup (inode);  out:  	loc_wipe (&loc);  	return inode; @@ -444,7 +440,6 @@ afr_shd_index_sweep (struct subvol_healer *healer, char *vgfid)          ret = syncop_dir_scan (subvol, &loc, GF_CLIENT_PID_SELF_HEALD,                                 healer, afr_shd_index_heal); -        inode_forget (loc.inode, 1);          loc_wipe (&loc);          if (ret == 0) diff --git a/xlators/cluster/ec/src/ec-heald.c b/xlators/cluster/ec/src/ec-heald.c index 2e5098a3612..0e8076826c6 100644 --- a/xlators/cluster/ec/src/ec-heald.c +++ b/xlators/cluster/ec/src/ec-heald.c @@ -137,10 +137,8 @@ ec_shd_inode_find (xlator_t *this, xlator_t *subvol,  	*inode =  NULL;          *inode = inode_find (this->itable, gfid); -        if (*inode) { -                inode_lookup (*inode); +        if (*inode)                  goto out; -        }          loc.inode = inode_new (this->itable);          if (!loc.inode) { @@ -157,8 +155,6 @@ ec_shd_inode_find (xlator_t *this, xlator_t *subvol,          if (!*inode) {                  ret = -ENOMEM;                  goto out; -        } else { -                inode_lookup (*inode);          }  out:          loc_wipe (&loc); @@ -267,8 +263,6 @@ out:                          uuid_utoa(loc.gfid));                  ec_shd_index_purge (subvol, parent->inode, entry->d_name);          } -        if (loc.inode) -                inode_forget (loc.inode, 0);          loc_wipe (&loc);          return 0; @@ -296,8 +290,6 @@ ec_shd_index_sweep (struct subvol_healer *healer)          ret = syncop_dir_scan (subvol, &loc, GF_CLIENT_PID_SELF_HEALD,                                 healer, ec_shd_index_heal);  out: -        if (loc.inode) -                inode_forget (loc.inode, 0);          loc_wipe (&loc);          return ret; @@ -336,8 +328,6 @@ ec_shd_full_heal (xlator_t *subvol, gf_dirent_t *entry, loc_t *parent,          ret = 0;  out: -        if (loc.inode) -                inode_forget (loc.inode, 0);          loc_wipe (&loc);          return ret;  }  | 
