smb: client: split find and create operations for cached dirs

This allows for concurrent calls to open_cached_dir to wait while the
other finishes, which, in turn, avoids the race and a leasebreak for the
path on the second open.

Also set cfid->time, the final validity indicator, only at the very end of
open_cached_dir on success.

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

1 files changed, 75 insertions, 66 deletions

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index fe738623cf1b..19b06b875a91 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -22,10 +22,7 @@ struct cached_dir_dentry {
 	struct dentry *dentry;
 };
 
-static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
-						    const char *path,
-						    bool lookup_only,
-						    __u32 max_cached_dirs)
+static struct cached_fid *find_cached_dir(struct cached_fids *cfids, const char *path)
 {
 	struct cached_fid *cfid;
 
@@ -33,37 +30,34 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 	list_for_each_entry(cfid, &cfids->entries, entry) {
 		if (!strcmp(cfid->path, path)) {
 			/*
-			 * If it doesn't have a lease it is either not yet
-			 * fully cached or it may be in the process of
-			 * being deleted due to a lease break.
+			 * If this cfid doesn't have a lease, it might have gotten a leasebreak from
+			 * another open to the same path, so continue the loop trying to find this
+			 * newer instance.
 			 */
-			if (!cfid->time || !cfid->has_lease) {
-				spin_unlock(&cfids->cfid_list_lock);
-				return NULL;
-			}
+			if (!cfid->has_lease)
+				continue;
+
 			kref_get(&cfid->refcount);
 			spin_unlock(&cfids->cfid_list_lock);
 			return cfid;
 		}
 	}
-	if (lookup_only) {
-		spin_unlock(&cfids->cfid_list_lock);
-		return NULL;
-	}
-	if (cfids->num_entries >= max_cached_dirs) {
-		spin_unlock(&cfids->cfid_list_lock);
-		return NULL;
-	}
+	spin_unlock(&cfids->cfid_list_lock);
+
+	return NULL;
+}
+
+static struct cached_fid *alloc_cached_dir(struct cached_fids *cfids, const char *path)
+{
+	struct cached_fid *cfid;
+
 	cfid = init_cached_dir(path);
-	if (cfid == NULL) {
-		spin_unlock(&cfids->cfid_list_lock);
+	if (!cfid)
 		return NULL;
-	}
+
 	cfid->cfids = cfids;
-	cfids->num_entries++;
-	list_add(&cfid->entry, &cfids->entries);
-	cfid->on_list = true;
 	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
@@ -74,7 +68,12 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 	 */
 	cfid->has_lease = true;
 
+	spin_lock(&cfids->cfid_list_lock);
+	list_add(&cfid->entry, &cfids->entries);
+	cfid->on_list = true;
+	cfids->num_entries++;
 	spin_unlock(&cfids->cfid_list_lock);
+
 	return cfid;
 }
 
@@ -173,7 +172,51 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 
 	if (cfids == NULL)
 		return -EOPNOTSUPP;
+find_again:
+	cfid = find_cached_dir(cfids, path);
+	if (cfid || lookup_only) {
+		/* If just a lookup, nothing else to do */
+		if (!cfid)
+			return -ENOENT;
+
+		/*
+		 * Return cached fid if it is valid (has a lease and has a time).
+		 * Otherwise, it is either a new entry or laundromat worker removed it
+		 * from @cfids->entries.  Caller will put last reference if the latter.
+		 */
+		if (cfid->has_lease) {
+			if (cfid->time) {
+				*ret_cfid = cfid;
+				kfree(utf16_path);
+				return 0;
+			}
 
+			/*
+			 * has_lease && !time means a concurrent open to same path -- allow others
+			 * to finish and try looking up again
+			 */
+			cond_resched();
+			goto find_again;
+		}
+
+		return -ENOENT;
+	}
+
+	/*
+	 * cfid for @path not found or gone, alloc/open a new one.
+	 * Differentiate between "can't add to list" and "no memory to alloc" so callers handle it
+	 * properly.
+	 */
+	spin_lock(&cfids->cfid_list_lock);
+	if (cfids->num_entries >= tcon->max_cached_dirs) {
+		spin_unlock(&cfids->cfid_list_lock);
+		return -ENOENT;
+	}
+	spin_unlock(&cfids->cfid_list_lock);
+
+	cfid = alloc_cached_dir(cfids, path);
+	if (!cfid)
+		return -ENOMEM;
 replay_again:
 	/* reinitialize for possible replay */
 	flags = 0;
@@ -187,25 +230,6 @@ replay_again:
 	if (!utf16_path)
 		return -ENOMEM;
 
-	cfid = find_or_create_cached_dir(cfids, path, lookup_only, tcon->max_cached_dirs);
-	if (cfid == NULL) {
-		kfree(utf16_path);
-		return -ENOENT;
-	}
-	/*
-	 * Return cached fid if it is valid (has a lease and has a time).
-	 * Otherwise, it is either a new entry or laundromat worker removed it
-	 * from @cfids->entries.  Caller will put last reference if the latter.
-	 */
-	spin_lock(&cfids->cfid_list_lock);
-	if (cfid->has_lease && cfid->time) {
-		spin_unlock(&cfids->cfid_list_lock);
-		*ret_cfid = cfid;
-		kfree(utf16_path);
-		return 0;
-	}
-	spin_unlock(&cfids->cfid_list_lock);
-
 	/*
 	 * Skip any prefix paths in @path as lookup_positive_unlocked() ends up
 	 * calling ->lookup() which already adds those through
@@ -303,8 +327,6 @@ replay_again:
 	}
 	cfid->is_open = true;
 
-	spin_lock(&cfids->cfid_list_lock);
-
 	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;
@@ -312,9 +334,7 @@ replay_again:
 	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;
 	}
@@ -323,21 +343,17 @@ replay_again:
 				 &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),
@@ -345,11 +361,7 @@ replay_again:
 				(char *)&cfid->file_all_info))
 		cfid->file_all_info_is_valid = true;
 
-	cfid->time = jiffies;
-	spin_unlock(&cfids->cfid_list_lock);
-	/* At this point the directory handle is fully cached */
 	rc = 0;
-
 oshr_free:
 	SMB2_open_free(&rqst[0]);
 	SMB2_query_info_free(&rqst[1]);
@@ -358,11 +370,6 @@ oshr_free:
 out:
 	if (rc) {
 		spin_lock(&cfids->cfid_list_lock);
-		if (cfid->on_list) {
-			list_del(&cfid->entry);
-			cfid->on_list = false;
-			cfids->num_entries--;
-		}
 		if (cfid->has_lease) {
 			/*
 			 * We are guaranteed to have two references at this
@@ -376,6 +383,8 @@ out:
 
 		kref_put(&cfid->refcount, smb2_close_cached_fid);
 	} else {
+		/* At this point the directory handle is fully cached */
+		cfid->time = jiffies;
 		*ret_cfid = cfid;
 		atomic_inc(&tcon->num_remote_opens);
 	}