|
 |
a3470f |
From 59fafc2b918846cc59daac107981211fa1568335 Mon Sep 17 00:00:00 2001
|
|
|
a3470f |
From: Susant Palai <spalai@redhat.com>
|
|
|
a3470f |
Date: Mon, 6 Aug 2018 15:02:58 +0530
|
|
|
a3470f |
Subject: [PATCH 350/351] cluster/dht: fix inode ref management in
|
|
|
a3470f |
dht_heal_path
|
|
|
a3470f |
|
|
|
a3470f |
In dht_heal_path, the inodes are created & looked up from top to down.
|
|
|
a3470f |
|
|
|
a3470f |
If the path is "a/b/c", then lookup will be done on a, then b and so
|
|
|
a3470f |
on. Here is a rough snippet of the function "dht_heal_path".
|
|
|
a3470f |
|
|
|
a3470f |
<snippet>
|
|
|
a3470f |
if (bname) { ref_count
|
|
|
a3470f |
- loc.inode = create/grep inode 1
|
|
|
a3470f |
- syncop_lookup (loc.inode)
|
|
|
a3470f |
- linked_inode = inode_link (loc.inode) 2
|
|
|
a3470f |
/*clean up current loc*/
|
|
|
a3470f |
- loc_wipe(&loc) 1
|
|
|
a3470f |
/*set up parent and bname for next child */
|
|
|
a3470f |
- loc.parent = inode
|
|
|
a3470f |
- bname = next_child_name
|
|
|
a3470f |
}
|
|
|
a3470f |
out:
|
|
|
a3470f |
- inode_ref (linked_inode) 2
|
|
|
a3470f |
- loc_wipe (&loc) 1
|
|
|
a3470f |
</snippet>
|
|
|
a3470f |
|
|
|
a3470f |
The problem with the above code is if _bname_ is empty ie the chain lookup is
|
|
|
a3470f |
done, then for the next iteration we populate loc.parent anyway. Now that
|
|
|
a3470f |
bname is empty, the loc_wipe is done in the _out_ section as well. Since, the
|
|
|
a3470f |
loc.parent was set to the previous inode, we lose a ref unwantedly. Now a
|
|
|
a3470f |
dht_local_wipe as part of the DHT_STACK_UNWIND takes away the last ref leading
|
|
|
a3470f |
to inode_destroy.
|
|
|
a3470f |
|
|
|
a3470f |
This problenm is observed currently with nfs-ganesha with the nameless lookup.
|
|
|
a3470f |
Post the inode_purge, gfapi does not get the new inode to link and hence, it links
|
|
|
a3470f |
the inode it sent in the lookup fop, which does not have any dht related context
|
|
|
a3470f |
(layout) leading to "invalid argument error" in lookup path done parallely with tar
|
|
|
a3470f |
operation.
|
|
|
a3470f |
|
|
|
a3470f |
test done in the following way:
|
|
|
a3470f |
- create two nfs client connected with two different nfs servers.
|
|
|
a3470f |
- run untar on one client and run lookup continuously on the other.
|
|
|
a3470f |
- Prior to this patch, invalid arguement was seen which is fixed with
|
|
|
a3470f |
the current patch.
|
|
|
a3470f |
|
|
|
a3470f |
> Change-Id: Ifb90c178a2f3c16604068c7da8fa562b877f5c61
|
|
|
a3470f |
> fixes: bz#1610256
|
|
|
a3470f |
> Signed-off-by: Susant Palai <spalai@redhat.com>
|
|
|
a3470f |
(backport of https://review.gluster.org/#/c/glusterfs/+/20643/)
|
|
|
a3470f |
|
|
|
a3470f |
Fixes: bz#1569657
|
|
|
a3470f |
Change-Id: I94329f2d148c31fe85a8753256e7aaf1d4d337ac
|
|
|
a3470f |
BUG: 1569657
|
|
|
a3470f |
Signed-off-by: Susant Palai <spalai@redhat.com>
|
|
|
a3470f |
Reviewed-on: https://code.engineering.redhat.com/gerrit/146966
|
|
|
a3470f |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
a3470f |
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
|
|
|
a3470f |
---
|
|
|
a3470f |
xlators/cluster/dht/src/dht-helper.c | 9 +++++++--
|
|
|
a3470f |
1 file changed, 7 insertions(+), 2 deletions(-)
|
|
|
a3470f |
|
|
|
a3470f |
diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c
|
|
|
a3470f |
index 09ca966..d35d7e8 100644
|
|
|
a3470f |
--- a/xlators/cluster/dht/src/dht-helper.c
|
|
|
a3470f |
+++ b/xlators/cluster/dht/src/dht-helper.c
|
|
|
a3470f |
@@ -1974,12 +1974,16 @@ dht_heal_path (xlator_t *this, char *path, inode_table_t *itable)
|
|
|
a3470f |
} else {
|
|
|
a3470f |
/*
|
|
|
a3470f |
* Inode is already populated in the inode table.
|
|
|
a3470f |
- * Which means we already looked up the inde and
|
|
|
a3470f |
+ * Which means we already looked up the inode and
|
|
|
a3470f |
* linked with a dentry. So that we will skip
|
|
|
a3470f |
* lookup on this entry, and proceed to next.
|
|
|
a3470f |
*/
|
|
|
a3470f |
+ linked_inode = loc.inode;
|
|
|
a3470f |
bname = strtok_r (NULL, "/", &save_ptr);
|
|
|
a3470f |
inode_unref (loc.parent);
|
|
|
a3470f |
+ if (!bname) {
|
|
|
a3470f |
+ goto out;
|
|
|
a3470f |
+ }
|
|
|
a3470f |
loc.parent = loc.inode;
|
|
|
a3470f |
gf_uuid_copy (loc.pargfid, loc.inode->gfid);
|
|
|
a3470f |
loc.inode = NULL;
|
|
|
a3470f |
@@ -2005,9 +2009,10 @@ dht_heal_path (xlator_t *this, char *path, inode_table_t *itable)
|
|
|
a3470f |
loc_wipe (&loc;;
|
|
|
a3470f |
gf_uuid_copy (loc.pargfid, linked_inode->gfid);
|
|
|
a3470f |
loc.inode = NULL;
|
|
|
a3470f |
- loc.parent = linked_inode;
|
|
|
a3470f |
|
|
|
a3470f |
bname = strtok_r (NULL, "/", &save_ptr);
|
|
|
a3470f |
+ if (bname)
|
|
|
a3470f |
+ loc.parent = linked_inode;
|
|
|
a3470f |
}
|
|
|
a3470f |
out:
|
|
|
a3470f |
inode_ref (linked_inode);
|
|
|
a3470f |
--
|
|
|
a3470f |
1.8.3.1
|
|
|
a3470f |
|