smb: client: fix cfid (cont, WIP)cached

Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>

6 files changed, 377 insertions, 105 deletions

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 555d10174570..016fd80eb53c 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -112,17 +112,21 @@ static bool can_cache_dir(struct cifs_tcon *tcon)
 
 static __maybe_unused struct cached_fid *init_cached_dir(struct cifs_tcon *tcon, const char *path)
 {
-	struct cached_fids *cfids;
 	struct cached_fid *cfid;
 
 	if (!can_cache_dir(tcon))
 		return NULL;
 
-	cfids = tcon->cfids;
 	cfid = alloc_cached_dir(path);
 	if (!cfid)
 		return NULL;
 
+	cfid->path = kstrdup(path, GFP_KERNEL);
+	if (!cfid->path) {
+		kfree(cfid);
+		return NULL;
+	}
+
 	cfid->tcon = tcon;
 
 	/*
@@ -161,6 +165,76 @@ static void free_cached_dir(struct cached_fid *cfid)
 	kfree(cfid);
 }
 
+static struct unleased_key *was_lease_broken(struct cached_fids *cfids, u8 *key)
+{
+	struct unleased_key *k;
+
+	list_for_each_entry(k, &cfids->unleased, head)
+		if (!memcmp(k->leasekey, key, 16 /* SMB2_LEASE_KEY_SIZE */))
+			return k;
+
+	return NULL;
+}
+
+static int validate_cfid(struct cached_fids *cfids, struct cached_fid *cfid)
+{
+	struct cached_fid *tmp;
+	int ret = 0;
+
+	if (!cfids || !cfid || !cfid->time || !cfid->path || !cfid->dentry)
+		return -EINVAL;
+
+	spin_lock(&cfids->lock);
+	spin_lock(&cfid->lock);
+	if (!cfid->info_valid) {
+		ret = -ESTALE;
+		goto out;
+	}
+
+	list_for_each_entry(tmp, &cfids->dying, head) {
+		if (tmp == cfid) {
+			ret = -ENOENT;
+			goto out;
+		}
+	}
+
+	if (was_lease_broken(cfids, cfid->fid.lease_key))
+		ret = -ECANCELED;
+out:
+	spin_unlock(&cfid->lock);
+	spin_unlock(&cfids->lock);
+
+	return ret;
+}
+
+static void force_close_cached_dir(struct cached_fid *cfid);
+
+static int handle_invalid_cfid(struct cached_fids *cfids, struct cached_fid *cfid, int err)
+{
+	int ret = err;
+
+	switch (err) {
+	case -ENOENT:
+		break;
+	case -EINVAL:
+		if (cfid)
+			close_cached_dir(cfid);
+		break;
+	case -ESTALE:
+		// re-query info
+		break;
+	case -ECANCELED:
+		force_close_cached_dir(cfid);
+		break;
+	default:
+		pr_warn("%s: unhandled error '%d', force closing cached dir\n", __func__, err);
+		force_close_cached_dir(cfid);
+		break;
+	}
+
+	return ret;
+}
+
 /* Check if cfid is dying or non-existent/dead. */
 static __maybe_unused bool cfid_is_dying(struct cached_fid *cfid)
 {
@@ -190,19 +264,44 @@ static void cleanup_cfids(struct work_struct *work)
 	struct cached_fids *cfids = container_of(work, struct cached_fids, work);
 	struct cached_fid *cfid, *q;
 
+	pr_err("%s: (begin) cleaning up cfids (empty entries=%d, dying=%d, unleased=%d)\n", __func__,
+		list_empty(&cfids->entries), list_empty(&cfids->dying), list_empty(&cfids->unleased));
+
+	/*
+	 * First, move leasebroken open cfids to dying list.
+	 */
 	spin_lock(&cfids->lock);
+	list_for_each_entry_safe(cfid, q, &cfids->entries, head) {
+		struct unleased_key *k;
+
+		spin_lock(&cfid->lock);
+		k = was_lease_broken(cfids, cfid->fid.lease_key);
+		if (k) {
+			list_del(&k->head);
+			kfree(k);
+			list_move(&cfid->head, &cfids->dying);
+		}
+		spin_unlock(&cfid->lock);
+	}
+
 	list_for_each_entry_safe(cfid, q, &cfids->dying, head) {
 		struct dentry *dentry;
 
-		spin_lock(&cfids->lock);
+		spin_lock(&cfid->lock);
 		dentry = cfid->dentry;
 		cfid->dentry = NULL;
+		spin_unlock(&cfid->lock);
+
+		free_cached_dir(cfid);
 		spin_unlock(&cfids->lock);
 
 		dput(dentry);
 
-		free_cached_dir(cfid);
+		spin_lock(&cfids->lock);
 	}
+
+	pr_err("%s: (end) cleaning up cfids (empty entries=%d, dying=%d, unleased=%d)\n", __func__,
+		list_empty(&cfids->entries), list_empty(&cfids->dying), list_empty(&cfids->unleased));
 	spin_unlock(&cfids->lock);
 }
 
@@ -216,10 +315,11 @@ static void release_cfid(struct kref *ref)
 	struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
 	struct cached_fids *cfids = cfid->tcon->cfids;
 
-	pr_err("%s: releasing cfid=%p, path=%s, dentry=%pd\n", __func__, cfid, cfid->path, cfid->dentry);
-
 	spin_lock(&cfids->lock);
 	spin_lock(&cfid->lock);
+
+	pr_err("%s: releasing cfid=%p, path=%s, dentry=%pd\n", __func__, cfid, cfid->path, cfid->dentry);
+
 	list_move(&cfid->head, &cfids->dying);
 	cfid->time = 0;
 	spin_unlock(&cfid->lock);
@@ -301,6 +401,7 @@ struct cached_fids *init_cached_dirs(void)
 	spin_lock_init(&cfids->lock);
 	INIT_LIST_HEAD(&cfids->entries);
 	INIT_LIST_HEAD(&cfids->dying);
+	INIT_LIST_HEAD(&cfids->unleased);
 	INIT_WORK(&cfids->work, cleanup_cfids);
 
 	return cfids;
@@ -370,60 +471,57 @@ struct cached_fid *find_cached_dir(struct cifs_tcon *tcon, const char *path)
 	return cfid;
 }
 
-void add_cached_dir(struct cifs_sb_info *cifs_sb, struct cifs_tcon *tcon, const char *path,
-		    struct cifs_fid *fid, struct smb2_file_all_info *info)
+struct cached_fid *add_cached_dir(struct cifs_sb_info *cifs_sb, struct cifs_tcon *tcon,
+				  const char *path,struct cifs_fid *fid,
+				  struct smb2_file_all_info *info)
 {
 	struct cached_fid *cfid = NULL;
-	struct dentry *dentry;
 	const char *npath;
 
 	if (!cifs_sb->root)
-		return;
-
-	cfid = __find_cached_dir(tcon->cfids, path, CFID_FIND_BY_PATH);
-	pr_err("%s: path=%s, fid=%p, info=%p -> cfid=%p\n", __func__, path, fid, info, cfid);
-	if (cfid)
-		/*
-		 * XXX: maybe check if fid/info matches?
-		 */
-		return;
+		return ERR_PTR(-ENOENT);
 
 	cfid = init_cached_dir(tcon, path);
 	if (!cfid)
-		return;
+		return ERR_PTR(-ENOMEM);
 
-	pr_err("%s: new cfid %p\n", __func__, cfid);
+	/*
+	 * Skip any prefix paths in @path as lookup_positive_unlocked() ends up
+	 * calling ->lookup() which already adds those through
+	 * build_path_from_dentry().  Also, do it earlier as we might reconnect
+	 * below when trying to send compounded request and then potentially
+	 * having a different prefix path (e.g. after DFS failover).
+	 */
 	npath = path_no_prefix(cifs_sb, path);
 	if (IS_ERR(npath)) {
 		kfree(cfid);
-		return;
+		return ERR_PTR(-ENOMEM);
 	}
 
-	pr_err("%s: npath '%s'\n", __func__, npath);
-
 	if (!npath[0]) {
-		dentry = dget(cifs_sb->root);
+		cfid->dentry = dget(cifs_sb->root);
 	} else {
-		dentry = path_to_dentry(cifs_sb, npath);
-		if (IS_ERR(dentry)) {
+		cfid->dentry = path_to_dentry(cifs_sb, npath);
+		if (IS_ERR(cfid->dentry)) {
 			kfree(cfid);
-			return;
+			return ERR_PTR(-ENOMEM);
 		}
 	}
-	pr_err("%s: dentry '%pd'\n", __func__, dentry);
-	cfid->dentry = dentry;
 
 	memcpy(&cfid->fid, fid, sizeof(*fid));
 	if (info) {
 		memcpy(&cfid->info, info, sizeof(*info));
 		cfid->info_valid = true;
 	}
+
 	cfid->time = jiffies;
 
 	spin_lock(&tcon->cfids->lock);
 	list_add(&cfid->head, &tcon->cfids->entries);
 	tcon->cfids->entries_count++;
 	spin_unlock(&tcon->cfids->lock);
+
+	return cfid;
 }
 
 /*
@@ -437,10 +535,197 @@ void add_cached_dir(struct cifs_sb_info *cifs_sb, struct cifs_tcon *tcon, const
 struct cached_fid *open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
 				   struct cifs_sb_info *cifs_sb)
 {
+	struct cifs_ses *ses;
+	struct TCP_Server_Info *server;
+	struct cifs_open_parms oparms;
+	struct smb2_create_rsp *o_rsp = NULL;
+	struct smb2_query_info_rsp *qi_rsp = NULL;
+	int resp_buftype[2];
+	struct smb_rqst rqst[2];
+	struct kvec rsp_iov[2];
+	struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
+	struct kvec qi_iov[1];
+	int rc, flags = 0;
+	__le16 *utf16_path = NULL;
+	u8 oplock = SMB2_OPLOCK_LEVEL_II;
+	struct cifs_fid *pfid;
+	struct cached_fid *cfid;
+	struct cached_fids *cfids;
+	int retries = 0, cur_sleep = 1;
+	struct smb2_file_all_info fi = { 0 };
+	struct cifs_fid fid = { 0 };
+	bool info_valid = false;
+
+	if (!cifs_sb->root)
+		return ERR_PTR(-ENOENT);
+
+	if (!tcon || !tcon->cfids)
+		return ERR_PTR(-EOPNOTSUPP);
+
+	ses = tcon->ses;
+	cfids = tcon->cfids;
+replay_again:
+	/* reinitialize for possible replay */
+	flags = 0;
+	oplock = SMB2_OPLOCK_LEVEL_II;
+	server = cifs_pick_channel(ses);
+
+	if (!server->ops->new_lease_key)
+		return ERR_PTR(-EIO);
+
+	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
+	if (!utf16_path)
+		return ERR_PTR(-ENOMEM);
+
+	cfid = __find_cached_dir(cfids, path, CFID_FIND_BY_PATH);
+	pr_err("%s: cfids %p, cifd%p\n", __func__, cfids, cfid);
+	if (cfid) {
+		kfree(utf16_path);
+
+		rc = validate_cfid(cfids, cfid);
+		if (!rc)
+			return cfid;
+
+		rc = handle_invalid_cfid(cfids, cfid, rc);
+
+		return ERR_PTR(rc);
+	}
+
 	/*
-	 * TODO: fill in the blank.
+	 * We do not hold the lock for the open because in case
+	 * SMB2_open needs to reconnect.
+	 * This is safe because no other thread will be able to get a ref
+	 * to the cfid until we have finished opening the file and (possibly)
+	 * acquired a lease.
 	 */
-	return ERR_PTR(-ENOENT);
+	if (smb3_encryption_required(tcon))
+		flags |= CIFS_TRANSFORM_REQ;
+
+	pfid = &fid;
+	server->ops->new_lease_key(pfid);
+
+	memset(rqst, 0, sizeof(rqst));
+	resp_buftype[0] = resp_buftype[1] = CIFS_NO_BUFFER;
+	memset(rsp_iov, 0, sizeof(rsp_iov));
+
+	/* Open */
+	memset(&open_iov, 0, sizeof(open_iov));
+	rqst[0].rq_iov = open_iov;
+	rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
+
+	oparms = (struct cifs_open_parms) {
+		.tcon = tcon,
+		.path = path,
+		.create_options = cifs_create_options(cifs_sb, CREATE_NOT_FILE),
+		.desired_access =  FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA,
+		.disposition = FILE_OPEN,
+		.fid = pfid,
+		.replay = !!(retries),
+	};
+
+	rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
+	if (rc)
+		goto err_free;
+
+	smb2_set_next_command(tcon, &rqst[0]);
+
+	memset(&qi_iov, 0, sizeof(qi_iov));
+	rqst[1].rq_iov = qi_iov;
+	rqst[1].rq_nvec = 1;
+
+	rc = SMB2_query_info_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
+				  FILE_ALL_INFORMATION, SMB2_O_INFO_FILE, 0,
+				  sizeof(struct smb2_file_all_info) + PATH_MAX * 2, 0, NULL);
+	if (rc)
+		goto err_free;
+
+	smb2_set_related(&rqst[1]);
+
+	if (retries) {
+		smb2_set_replay(server, &rqst[0]);
+		smb2_set_replay(server, &rqst[1]);
+	}
+
+	rc = compound_send_recv(xid, ses, server, flags, 2, rqst, resp_buftype, rsp_iov);
+	if (rc) {
+		if (rc == -EREMCHG) {
+			tcon->need_reconnect = true;
+			pr_warn_once("server share %s deleted\n", tcon->tree_name);
+		}
+
+		goto err_free;
+	}
+
+	o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
+	oparms.fid->persistent_fid = o_rsp->PersistentFileId;
+	oparms.fid->volatile_fid = o_rsp->VolatileFileId;
+#ifdef CONFIG_CIFS_DEBUG2
+	oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
+#endif /* CIFS_DEBUG2 */
+
+	if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
+		rc = -EINVAL;
+		goto err_free;
+	}
+
+	rc = smb2_parse_contexts(server, rsp_iov, &oparms.fid->epoch, oparms.fid->lease_key,
+				 &oplock, NULL, NULL);
+	if (rc)
+		goto err_free;
+
+	rc = -EINVAL;
+	if (!(oplock & SMB2_LEASE_READ_CACHING_HE))
+		goto err_free;
+
+	qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
+	if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
+		goto err_free;
+
+	if (!smb2_validate_and_copy_iov(le16_to_cpu(qi_rsp->OutputBufferOffset),
+					sizeof(struct smb2_file_all_info),
+					&rsp_iov[1], sizeof(struct smb2_file_all_info),
+					(char *)&fi))
+		info_valid = true;
+
+	rc = 0;
+err_free:
+	SMB2_open_free(&rqst[0]);
+	SMB2_query_info_free(&rqst[1]);
+	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
+	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
+	kfree(utf16_path);
+
+	if (is_replayable_error(rc) &&
+	    smb2_should_replay(tcon, &retries, &cur_sleep))
+		goto replay_again;
+
+	/*
+	 * Only cache the directory properly after we got everything validated + all replays done.
+	 */
+	if (!rc) {
+		struct unleased_key *k;
+
+		spin_lock(&cfids->lock);
+		k = was_lease_broken(cfids, fid.lease_key);
+		spin_unlock(&cfids->lock);
+
+		if (!k) {
+			cfid = add_cached_dir(cifs_sb, tcon, path, &fid, info_valid ? &fi : NULL);
+			if (!IS_ERR(cfid))
+				atomic_inc(&tcon->num_remote_opens);
+		} else {
+			spin_lock(&cfids->lock);
+			list_del(&k->head);
+			kfree(k);
+			spin_unlock(&cfids->lock);
+
+			cfid = ERR_PTR(-ENOENT);
+		}
+	} else {
+		cfid = ERR_PTR(rc);
+	}
+
+	return cfid;
 }
 
 /*
@@ -506,10 +791,43 @@ void close_cached_dir_by_name(struct cached_fids *cfids, const char *path)
 void cached_dir_lease_break(struct cifs_tcon *tcon, u8 lease_key[16])
 {
 	struct cached_fid *cfid;
+	struct unleased_key *k;
 
+	k = was_lease_broken(tcon->cfids, lease_key);
 	cfid = __find_cached_dir(tcon->cfids, &lease_key[0], CFID_FIND_BY_LEASEKEY);
+
+	pr_err("%s: k=%p, cfid=%p\n", __func__, k, cfid);
+
+	/*
+	 * If we reach here, it's a case of concurrent opens where the fid received a lease break
+	 * and we don't have a cached entry for it yet.
+	 *
+	 * If we had it already stored, nothing to do -- cleanup_cfids will free it later.
+	 */
+	if (k)
+		goto out;
+
+	/* If not, we'll store it so we only cache the last valid open. */
+	k = kzalloc(sizeof(*k), GFP_KERNEL);
+	if (!k)
+		return;
+
+	INIT_LIST_HEAD(&k->head);
+
+	if (cfid) {
+		spin_lock(&cfid->lock);
+		memcpy(k->leasekey, cfid->fid.lease_key, 16);
+		spin_unlock(&cfid->lock);
+	} else {
+		memcpy(k->leasekey, lease_key, 16);
+	}
+
+	spin_lock(&tcon->cfids->lock);
+	list_add(&k->head, &tcon->cfids->unleased);
+	spin_unlock(&tcon->cfids->lock);
+out:
 	if (cfid)
-		force_close_cached_dir(cfid);
+		close_cached_dir(cfid);
 }
 
 /*
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index c989cd565040..573892dbbdc7 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -69,13 +69,21 @@ struct cached_fid {
 	struct cached_dirents dirents;	/* XXX: What are these for?  Could it be merged here? */
 };
 
+struct unleased_key {
+	struct list_head head;
+	u8 leasekey[16 /* SMB2_LEASE_KEY_SIZE */];
+	bool is_open;
+};
+
 /* default MAX_CACHED_FIDS is 16 (cf. fs_context.h, maybe move here?) */
 struct cached_fids {
 	spinlock_t lock;		/* General lock; use it to modify any field */
 
 	struct list_head entries;	/* List of cached_fid entries */
 	int entries_count;		/* Number of cached dirs, from 0 to MAX_CACHED_FIDS */
-	struct list_head dying;		/* List of closed cached_fid's that needs to be freed */
+
+	struct list_head unleased;	/* List of leasekeys broken */
+	struct list_head dying;		/* List of cached_fid's that needs to be freed */
 
 	struct work_struct work;	/* cleanup_cifds work */
 	struct workqueue_struct *wq;	/* Per-tcon (cfids) cleanup workqueue */
@@ -85,7 +93,7 @@ extern struct cached_fids *init_cached_dirs(void);
 extern void destroy_cached_dirs(struct cifs_tcon *tcon, bool teardown);
 
 extern struct cached_fid *find_cached_dir(struct cifs_tcon *tcon, const char *path);
-extern void add_cached_dir(struct cifs_sb_info *cifs_sb, struct cifs_tcon *tcon, const char *path, struct cifs_fid *fid, struct smb2_file_all_info *info);
+extern struct cached_fid *add_cached_dir(struct cifs_sb_info *cifs_sb, struct cifs_tcon *tcon, const char *path, struct cifs_fid *fid, struct smb2_file_all_info *info);
 extern struct cached_fid *open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 					  const char *path, struct cifs_sb_info *cifs_sb);
 extern struct cached_fid *open_query_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index c13b7400c46c..ef7783a5386e 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1001,7 +1001,6 @@ cifs_get_file_info(struct file *filp)
 		cifs_open_info_to_fattr(&fattr, &data, inode->i_sb);
 		if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING)
 			cifs_mark_open_handles_for_deleted_file(inode, path);
-		add_cached_dir(CIFS_FILE_SB(filp), tcon, path, &cfile->fid, &data.fi);
 		break;
 	case -EREMOTE:
 		cifs_create_junction_fattr(&fattr, inode->i_sb);
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index b3624057b6b2..72378a930c35 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -1059,16 +1059,12 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
 		tcon = tlink_tcon(cifsFile->tlink);
 	}
 
-	cfid = find_cached_dir(tcon, full_path);
+	cfid = open_cached_dir(xid, tcon, full_path, cifs_sb);
 	cifs_put_tlink(tlink);
 	if (IS_ERR(cfid)) {
-		rc = PTR_ERR(cfid);
 		cfid = NULL;
 
-		if (rc == -ENOENT)
-			goto cache_not_found;
-
-		goto rddir2_exit;
+		goto cache_not_found;
 	}
 
 	mutex_lock(&cfid->dirents.de_mutex);
@@ -1137,13 +1133,12 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
 	tcon = tlink_tcon(cifsFile->tlink);
 	rc = find_cifs_entry(xid, tcon, ctx->pos, file, full_path,
 			     &current_entry, &num_to_fill);
-	cfid = find_cached_dir(tcon, full_path);
+	if (!cfid)
+		cfid = open_cached_dir(xid, tcon, full_path, cifs_sb);
+
 	if (IS_ERR(cfid)) {
 		rc = PTR_ERR(cfid);
 		cfid = NULL;
-	}
-
-	if (rc) {
 		cifs_dbg(FYI, "fce error %d\n", rc);
 		goto rddir2_exit;
 	} else if (current_entry != NULL) {
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 7fc393d55903..d7e7842aa7aa 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -889,45 +889,23 @@ int smb2_query_path_info(const unsigned int xid,
 		if (*full_path) {
 			rc = -ENOENT;
 		} else {
-			rc = 0;
-			cfid = find_cached_dir(tcon, full_path);
+			cfid = open_cached_dir(xid, tcon, full_path, cifs_sb);
 			if (IS_ERR(cfid)) {
 				rc = PTR_ERR(cfid);
 				cfid = NULL;
 			}
 		}
 
-		pr_err("%s: path=%s, cfid=%p, rc=%d\n", __func__, full_path, cfid, rc);
-
 		/* If it is a root and its handle is cached then use it */
 		if (!rc) {
-			spin_lock(&cfid->lock);
-			pr_err("%s: cfid path=%s, dentry=%pd, info=%s, valid=%d\n", __func__,
-				cfid->path, cfid->dentry, cfid->info.FileName, cfid->info_valid);
-
 			if (cfid->info_valid) {
 				memcpy(&data->fi, &cfid->info, sizeof(data->fi));
-				spin_unlock(&cfid->lock);
 			} else {
-				u64 pfid, vfid;
-
-				pfid = cfid->fid.persistent_fid;
-				vfid = cfid->fid.volatile_fid;
-				memset(&cfid->info, 0, sizeof(cfid->info));
-				spin_unlock(&cfid->lock);
-
 				rc = SMB2_query_info(xid, tcon,
 						     cfid->fid.persistent_fid,
 						     cfid->fid.volatile_fid,
 						     &data->fi);
-				if (!rc) {
-					spin_lock(&cfid->lock);
-					memcpy(&cfid->info, &data->fi, sizeof(data->fi));
-					cfid->info_valid = true;
-					spin_unlock(&cfid->lock);
-				}
 			}
-
 			close_cached_dir(cfid);
 			return rc;
 		}
@@ -958,17 +936,6 @@ int smb2_query_path_info(const unsigned int xid,
 	switch (rc) {
 	case 0:
 		rc = parse_create_response(data, cifs_sb, full_path, &out_iov[0]);
-		if (!rc && !cfid) {
-			struct smb2_create_rsp *rsp = out_iov[0].iov_base;
-			struct cifs_fid fid = {
-				.persistent_fid = rsp->PersistentFileId,
-				.volatile_fid = rsp->VolatileFileId,
-				.access = oparms.desired_access,
-				// what else is needed for a cfid?
-			};
-
-			add_cached_dir(cifs_sb, tcon, full_path, &fid, &data->fi);
-		}
 		break;
 	case -EOPNOTSUPP:
 		/*
@@ -997,20 +964,6 @@ int smb2_query_path_info(const unsigned int xid,
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
 				      &oparms, in_iov, cmds, num_cmds,
 				      cfile, NULL, NULL, NULL);
-
-		if (!rc && !cfid) {
-			struct smb2_create_rsp *rsp = out_iov[0].iov_base;
-			struct cifs_fid fid = {
-				.persistent_fid = rsp->PersistentFileId,
-				.volatile_fid = rsp->VolatileFileId,
-				.access = oparms.desired_access,
-				// what else is needed for a cfid?
-			};
-
-			pr_err("%s: cfile=%p\n", __func__, cfile);
-
-			add_cached_dir(cifs_sb, tcon, full_path, &fid, &data->fi);
-		}
 		break;
 	case -EREMOTE:
 		break;
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 3cc97c431880..3ea2171d45dc 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -858,18 +858,17 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon,
 		.fid = &fid,
 	};
 
-	cfid = find_cached_dir(tcon, "");
+	cfid = open_cached_dir(xid, tcon, "", cifs_sb);
 	if (!IS_ERR(cfid)) {
 		memcpy(&fid, &cfid->fid, sizeof(struct cifs_fid));
 	} else {
 		cfid = NULL;
 		rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL,
 			       NULL, NULL);
+		if (rc)
+			return;
 	}
 
-	if (rc)
-		return;
-
 	SMB3_request_interfaces(xid, tcon, true /* called during  mount */);
 
 	SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid,
@@ -882,7 +881,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon,
 			FS_SECTOR_SIZE_INFORMATION); /* SMB3 specific */
 
 	if (!cfid) {
-		add_cached_dir(cifs_sb, tcon, "", &fid, NULL);
+		add_cached_dir(cifs_sb, tcon, "", oparms.fid, NULL);
 		SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
 	} else {
 		close_cached_dir(cfid);
@@ -2703,9 +2702,14 @@ replay_again:
 	 * We can only call this for things we know are directories.
 	 */
 	if (!strcmp(path, "")) {
-		cfid = find_cached_dir(tcon, path);
-		if (IS_ERR(cfid))
+		cfid = open_cached_dir(xid, tcon, path, cifs_sb);
+		if (IS_ERR(cfid)) {
+			rc = PTR_ERR(cfid);
 			cfid = NULL;
+
+			if (rc != -ENOENT)
+				goto out_free_path;
+		}
 	}
 
 	rqst[0].rq_iov = vars->open_iov;
@@ -2749,7 +2753,6 @@ replay_again:
 	}
 	if (rc)
 		goto qic_exit;
-
 	if (!cfid) {
 		smb2_set_next_command(tcon, &rqst[1]);
 		smb2_set_related(&rqst[1]);
@@ -2793,10 +2796,6 @@ replay_again:
 	*rsp = rsp_iov[1];
 	*buftype = resp_buftype[1];
 
-	/*
-	 * Don't add a cached dir here, do it on callers instead.
-	 */
-
  qic_exit:
 	SMB2_open_free(&rqst[0]);
 	SMB2_query_info_free(&rqst[1]);