smb: client: introduce cached_fid->state

Add cached_fid->state to track a cfid state more accurately and
flexibly.

Remove bools ->has_lease, ->is_open, ->file_all_info_valid.

Add states:
- CFID_IS_OPEN
- CFID_IS_OPENING
- CFID_INVALID

See cached_dir.h for more info on state flags.

Note CFID_IS_OPEN covers both ->has_lease and ->is_open because,
after all, we only have a lease if we opened it to begin with.

Other changes:
- ->file_all_info becomes a pointer and its validity becomes its
  allocation (NULL or not NULL)
- move full cfid initialization to the end of open_cached_dir() (instead
  of in between calls), except CFID_IS_OPEN
- remove unnecessary list lock when parsing response in
  open_cached_dir()

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

5 files changed, 118 insertions, 83 deletions

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 125558b3b92d..c4f110fc66d5 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -15,23 +15,14 @@
 static struct cached_fid *init_cached_dir(const char *path);
 static void smb2_close_cached_fid(struct kref *ref);
 
-static inline bool cfid_expired(const struct cached_fid *cfid)
-{
-	return (cfid->last_access_time &&
-		time_is_before_jiffies(cfid->last_access_time + HZ * dir_cache_timeout));
-}
-
-static inline bool cfid_is_valid(const struct cached_fid *cfid)
-{
-	return (cfid->has_lease && cfid->time && !cfid_expired(cfid));
-}
-
 static inline void invalidate_cfid(struct cached_fid *cfid, bool force_expire)
 {
-	if (cfid_is_valid(cfid))
+	/* only drop entries counter if it hasn't been invalidated earlier */
+	if (!(cfid->state & CFID_INVALID))
 		cfid->cfids->num_entries--;
 
-	/* do not change other fields here! */
+	/* only tag it as invalid -- don't modify state any further here! */
+	cfid->state |= CFID_INVALID;
 	cfid->time = 0;
 	if (force_expire)
 		cfid->last_access_time = 1;
@@ -110,8 +101,7 @@ path_to_dentry(struct cifs_sb_info *cifs_sb, const char *path)
 		while (*s && *s != sep)
 			s++;
 
-		child = lookup_noperm_positive_unlocked(&QSTR_LEN(p, s - p),
-							dentry);
+		child = lookup_noperm_positive_unlocked(&QSTR_LEN(p, s - p), dentry);
 		dput(dentry);
 		dentry = child;
 	} while (!IS_ERR(dentry));
@@ -157,7 +147,9 @@ retry_find:
 			bool retry;
 
 			/* cfid is being opened in open_cached_dir() */
-			retry = (cfid->has_lease && !cfid->time && !cfid->last_access_time);
+			retry = (!(cfid->state & CFID_INVALID) &&
+				 (cfid->state & CFID_IS_OPENING) &&
+				 !cfid->time && !cfid->last_access_time);
 			kref_put(&cfid->refcount, smb2_close_cached_fid);
 
 			if (retry)
@@ -182,6 +174,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
 	struct cifs_open_parms oparms;
 	struct smb2_create_rsp *o_rsp = NULL;
 	struct smb2_query_info_rsp *qi_rsp = NULL;
+	struct smb2_file_all_info info = {};
 	int resp_buftype[2];
 	struct smb_rqst rqst[2];
 	struct kvec rsp_iov[2];
@@ -197,6 +190,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
 	const char *npath;
 	int retries = 0, cur_sleep = 1;
 	__le32 lease_flags = 0;
+	bool has_info;
 
 	if (cifs_sb->root == NULL)
 		return -ENOENT;
@@ -217,6 +211,7 @@ replay_again:
 	cfid = NULL;
 	*ret_cfid = NULL;
 	server = cifs_pick_channel(ses);
+	has_info = false;
 
 	if (!server->ops->new_lease_key)
 		return -EIO;
@@ -225,7 +220,7 @@ replay_again:
 	if (!utf16_path)
 		return -ENOMEM;
 
-	/* find_cached_dir() already checks has_lease and time, so no need to check here */
+	/* find_cached_dir() will return a valid cfid if found, so no need to check here again */
 	cfid = find_cached_dir(cfids, path, CFID_LOOKUP_PATH);
 	if (cfid) {
 		rc = 0;
@@ -243,10 +238,11 @@ replay_again:
 	if (!cfid) {
 		spin_unlock(&cfids->cfid_list_lock);
 		rc = -ENOMEM;
-		goto out;
+		goto out_err;
 	}
 
 	cfid->cfids = cfids;
+	cfid->tcon = tcon;
 	cfids->num_entries++;
 	list_add(&cfid->entry, &cfids->entries);
 	spin_unlock(&cfids->cfid_list_lock);
@@ -263,7 +259,7 @@ replay_again:
 	npath = path_no_prefix(cifs_sb, path);
 	if (IS_ERR(npath)) {
 		rc = PTR_ERR(npath);
-		goto out;
+		goto out_err;
 	}
 
 	if (!npath[0]) {
@@ -273,7 +269,7 @@ replay_again:
 		if (IS_ERR(dentry)) {
 			dentry = NULL;
 			rc = -ENOENT;
-			goto out;
+			goto out_err;
 		}
 		if (dentry->d_parent && server->dialect >= SMB30_PROT_ID) {
 			struct cached_fid *parent_cfid;
@@ -282,7 +278,7 @@ replay_again:
 			list_for_each_entry(parent_cfid, &cfids->entries, entry) {
 				if (parent_cfid->dentry == dentry->d_parent) {
 					cifs_dbg(FYI, "found a parent cached file handle\n");
-					if (parent_cfid->has_lease && parent_cfid->time) {
+					if (cfid_is_valid(parent_cfid)) {
 						lease_flags
 							|= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
 						memcpy(pfid->parent_lease_key,
@@ -295,9 +291,6 @@ replay_again:
 			spin_unlock(&cfids->cfid_list_lock);
 		}
 	}
-	cfid->dentry = dentry;
-	cfid->tcon = tcon;
-	dentry = NULL;
 
 	/*
 	 * We do not hold the lock for the open because in case
@@ -369,9 +362,8 @@ replay_again:
 		}
 		goto oshr_free;
 	}
-	cfid->is_open = true;
 
-	spin_lock(&cfids->cfid_list_lock);
+	cfid->state |= CFID_IS_OPEN;
 
 	o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
 	oparms.fid->persistent_fid = o_rsp->PersistentFileId;
@@ -379,76 +371,78 @@ replay_again:
 #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) {
-		spin_unlock(&cfids->cfid_list_lock);
 		rc = -EINVAL;
 		goto oshr_free;
 	}
 
-	rc = smb2_parse_contexts(server, rsp_iov,
-				 &oparms.fid->epoch,
-				 oparms.fid->lease_key,
+	rc = smb2_parse_contexts(server, rsp_iov, &oparms.fid->epoch, oparms.fid->lease_key,
 				 &oplock, NULL, NULL);
-	if (rc) {
-		spin_unlock(&cfids->cfid_list_lock);
+	if (rc)
 		goto oshr_free;
-	}
 
 	rc = -EINVAL;
-	if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) {
-		spin_unlock(&cfids->cfid_list_lock);
+	if (!(oplock & SMB2_LEASE_READ_CACHING_HE))
 		goto oshr_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)) {
-		spin_unlock(&cfids->cfid_list_lock);
+	if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
 		goto oshr_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 *)&cfid->file_all_info))
-		cfid->file_all_info_is_valid = true;
-
-	cfid->time = jiffies;
-	cfid->last_access_time = jiffies;
-	spin_unlock(&cfids->cfid_list_lock);
-	/* At this point the directory handle is fully cached */
-	rc = 0;
 
+	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 *)&info))
+		has_info = true;
+
+	rc = 0;
 oshr_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);
-out:
+out_err:
 	/* cfid invalidated in the mean time, drop it below */
-	if (!rc && !cfid_is_valid(cfid))
+	if (!rc && (cfid->state & CFID_INVALID))
 		rc = -ENOENT;
 
 	if (rc) {
+		dput(dentry);
+
 		if (cfid) {
 			spin_lock(&cfids->cfid_list_lock);
 			/* we don't know what caused the invalidation, so don't force cleanup */
 			invalidate_cfid(cfid, false);
+			cfid->state &= ~CFID_IS_OPENING;
 			spin_unlock(&cfids->cfid_list_lock);
 
-			dput(cfid->dentry);
-			cfid->dentry = NULL;
-
 			kref_put(&cfid->refcount, smb2_close_cached_fid);
 		}
+		cfid = NULL;
 	} else {
-		*ret_cfid = cfid;
+		cfid->state |= CFID_IS_OPEN;
+		if (has_info) {
+			cfid->info = kmemdup(&info, sizeof(info), GFP_ATOMIC);
+			if (!cfid->info) {
+				cfid->state &= ~CFID_IS_OPENING;
+				rc = -ENOMEM;
+				goto out_err;
+			}
+		}
+		cfid->dentry = dentry;
+		cfid->time = jiffies;
+		cfid->last_access_time = jiffies;
+		/* must be the last step -- concurrent lookups are now able to find this cfid */
+		cfid->state &= ~CFID_IS_OPENING;
+		/* At this point the directory handle is fully cached */
+
 		atomic_inc(&tcon->num_remote_opens);
 	}
+out:
+	*ret_cfid = cfid;
 	kfree(utf16_path);
 
-	if (is_replayable_error(rc) &&
-	    smb2_should_replay(tcon, &retries, &cur_sleep))
+	if (is_replayable_error(rc) && smb2_should_replay(tcon, &retries, &cur_sleep))
 		goto replay_again;
 
 	return rc;
@@ -479,6 +473,8 @@ static void smb2_close_cached_fid(struct kref *ref)
 		kfree(dirent);
 	}
 
+	kfree(cfid->info);
+	cfid->info = NULL;
 	kfree(cfid->path);
 	cfid->path = NULL;
 	kfree(cfid);
@@ -527,9 +523,8 @@ static void invalidate_all_cfids(struct cached_fids *cfids, bool closed)
 	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
 		invalidate_cfid(cfid, true);
-		cfid->has_lease = false;
 		if (closed)
-			cfid->is_open = false;
+			cfid->state &= ~CFID_IS_OPEN;
 	}
 	spin_unlock(&cfids->cfid_list_lock);
 
@@ -600,15 +595,7 @@ static struct cached_fid *init_cached_dir(const char *path)
 	/* this is caller/lease ref */
 	kref_get(&cfid->refcount);
 
-	/*
-	 * Set @cfid->has_lease to true during construction so that the lease
-	 * reference can be put in cached_dir_lease_break() due to a potential
-	 * lease break right after the request is sent or while @cfid is still
-	 * being cached, or if a reconnection is triggered during construction.
-	 * Concurrent processes won't be to use it yet due to @cfid->time being
-	 * zero.
-	 */
-	cfid->has_lease = true;
+	cfid->state = CFID_IS_OPENING;
 
 	return cfid;
 }
@@ -649,11 +636,11 @@ static void cfids_laundromat_worker(struct work_struct *work)
 
 		dput(dentry);
 
-		if (cfid->is_open) {
+		if (cfid->state & CFID_IS_OPEN) {
 			int rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
 					    cfid->fid.volatile_fid);
 
-			cfid->is_open = false;
+			cfid->state &= ~CFID_IS_OPEN;
 
 			/* SMB2_close should handle -EBUSY or -EAGAIN */
 			if (rc)
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 6ebb34353144..73e28b04b519 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -33,9 +33,7 @@ struct cached_fid {
 	struct list_head entry;
 	struct cached_fids *cfids;
 	const char *path;
-	bool has_lease:1;
-	bool is_open:1;
-	bool file_all_info_is_valid:1;
+	int state;
 	unsigned long time; /* jiffies of when lease was taken */
 	unsigned long last_access_time; /* jiffies of when last accessed */
 	struct kref refcount;
@@ -43,7 +41,7 @@ struct cached_fid {
 	spinlock_t fid_lock;
 	struct cifs_tcon *tcon;
 	struct dentry *dentry;
-	struct smb2_file_all_info file_all_info;
+	struct smb2_file_all_info *info;
 	struct cached_dirents dirents;
 };
 
@@ -66,6 +64,51 @@ enum {
 	CFID_LOOKUP_LEASEKEY,
 };
 
+/*
+ * cfid state flags
+ *
+ * CFID_IS_OPENING:	cfid was initialized and is being opened in open_cached_dir()
+ * CFID_IS_OPEN:	remote fid opened on the server and we got a lease for it
+ * CFID_INVALID:	cfid must not be used externally (but may be used internally)
+ *
+ * Note that CFID_IS_OPEN by itself doesn't determine validity of a cfid, e.g.
+ * (CFID_IS_OPEN | CFID_INVALID) means the remote handle for this directory is still open on the
+ * server (and needs closing), but the cfid object must not be used (expired, dropped, etc).
+ */
+#define CFID_IS_OPENING		0x1
+#define CFID_IS_OPEN		0x2
+#define CFID_INVALID		0x4
+
+static inline bool cfid_expired(const struct cached_fid *cfid)
+{
+	if (!cfid)
+		return true;
+
+	return (cfid->last_access_time &&
+		time_is_before_jiffies(cfid->last_access_time + dir_cache_timeout * HZ));
+}
+
+static inline bool cfid_is_valid(const struct cached_fid *cfid)
+{
+	if (!cfid)
+		return false;
+
+	/* cfid is about to go down in these cases, so check it first */
+	if (cfid->last_access_time == 1)
+		return false;
+
+	if (cfid->state & (CFID_IS_OPENING | CFID_INVALID))
+		return false;
+
+	if (!(cfid->state & CFID_IS_OPEN))
+		return false;
+
+	if (!cfid->time || !cfid->last_access_time)
+		return false;
+
+	return !cfid_expired(cfid);
+}
+
 extern struct cached_fids *init_cached_dirs(void);
 extern struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void *key, int mode);
 extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 2337cf795db3..9850d06caa3b 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -314,7 +314,7 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
 						ses->Suid,
 						cfid->fid.persistent_fid,
 						cfid->path);
-					if (cfid->file_all_info_is_valid)
+					if (cfid->info)
 						seq_printf(m, "\tvalid file info");
 					if (cfid->dirents.is_valid)
 						seq_printf(m, ", valid dirents");
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 5223edf6d11a..2c6450e1c97c 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -322,7 +322,7 @@ retry_open:
 		list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
 			if (parent_cfid->dentry == direntry->d_parent) {
 				cifs_dbg(FYI, "found a parent cached file handle\n");
-				if (parent_cfid->has_lease && parent_cfid->time) {
+				if (cfid_is_valid(parent_cfid)) {
 					lease_flags
 						|= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
 					memcpy(fid->parent_lease_key,
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 62d6adf50ad1..192645a891c8 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -970,14 +970,19 @@ int smb2_query_path_info(const unsigned int xid,
 
 		/* If it is a root and its handle is cached then use it */
 		if (!rc) {
-			if (cfid->file_all_info_is_valid) {
-				memcpy(&data->fi, &cfid->file_all_info,
-				       sizeof(data->fi));
+			if (cfid->info) {
+				memcpy(&data->fi, cfid->info, sizeof(data->fi));
 			} else {
 				rc = SMB2_query_info(xid, tcon,
 						     cfid->fid.persistent_fid,
 						     cfid->fid.volatile_fid,
 						     &data->fi);
+				if (!rc) {
+					cfid->info = kmemdup(&data->fi, sizeof(data->fi),
+							     GFP_KERNEL);
+					if (!cfid->info)
+						rc = -ENOMEM;
+				}
 			}
 			close_cached_dir(cfid);
 			return rc;