smb: client: prevent lease breaks of cached parents when opening children

In SMB2_open_init() lookup for a cached parent of target path and set
ParentLeaseKey in lease context if found.

Introduce invalidate_cached_dirents() to allow revalidation of cached
dirents but still keep the cached directory.

Testing with e.g. xfstests generic/637 (small simple test) shows a
performance gain of ~17% less create/open ops, and ~83% less lease
breaks:

Before patch
Create/open:
  # tshark -Y 'smb2.create.disposition == 1' -r unpatched.pcap
  169

Lease breaks:
  # tshark -Y 'smb2.cmd == 18' -r unpatched.pcap
  12
-----------------
After patch:
Create/open:
  # tshark -Y 'smb2.create.disposition == 1' -r patched.pcap
  144

Lease breaks:
  # tshark -Y 'smb2.cmd == 18' -r patched.pcap
  2

Other:
- set oparms->cifs_sb in open_cached_dir() as we need it in
  check_cached_parent(); use CIFS_OPARMS() too
- introduce CFID_LOOKUP_PARENT lookup mode (for string paths only)
- add cached_fids->dirsep to save dir separator, used by parent lookups

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

5 files changed, 136 insertions, 76 deletions

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index d04dc65fb8a6..22709b0bfb3d 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -69,11 +69,29 @@ static struct cached_fid *find_cfid(struct cached_fids *cfids, const void *key,
 				    bool wait_open)
 {
 	struct cached_fid *cfid, *found;
+	const char *parent_path = NULL;
 	bool match;
 
 	if (!cfids || !key)
 		return NULL;
 
+	if (mode == CFID_LOOKUP_PARENT) {
+		const char *path = key;
+
+		if (!*path)
+			return NULL;
+
+		parent_path = strrchr(path, cfids->dirsep);
+		if (!parent_path)
+			return NULL;
+
+		parent_path = kstrndup(path, parent_path - path, GFP_KERNEL);
+		if (WARN_ON_ONCE(!parent_path))
+			return NULL;
+
+		key = parent_path;
+		mode = CFID_LOOKUP_PATH;
+	}
 retry_find:
 	found = NULL;
 
@@ -112,6 +130,8 @@ retry_find:
 		kref_get(&found->refcount);
 	}
 
+	kfree(parent_path);
+
 	return found;
 }
 
@@ -224,7 +244,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
 	struct cached_fids *cfids;
 	const char *npath;
 	int retries = 0, cur_sleep = 1;
-	__le32 lease_flags = 0;
 
 	if (cifs_sb->root == NULL)
 		return -ENOENT;
@@ -234,9 +253,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
 
 	ses = tcon->ses;
 	cfids = tcon->cfids;
-
 	if (!cfids)
 		return -EOPNOTSUPP;
+
+	if (!cfids->dirsep)
+		cfids->dirsep = CIFS_DIR_SEP(cifs_sb);
 replay_again:
 	/* reinitialize for possible replay */
 	flags = 0;
@@ -304,25 +325,6 @@ replay_again:
 			rc = -ENOENT;
 			goto out;
 		}
-		if (dentry->d_parent && server->dialect >= SMB30_PROT_ID) {
-			struct cached_fid *parent_cfid;
-
-			spin_lock(&cfids->cfid_list_lock);
-			list_for_each_entry(parent_cfid, &cfids->entries, entry) {
-				if (parent_cfid->dentry == dentry->d_parent) {
-					if (!is_valid_cached_dir(parent_cfid))
-						break;
-
-					cifs_dbg(FYI, "found a parent cached file handle\n");
-
-					lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
-					memcpy(pfid->parent_lease_key, parent_cfid->fid.lease_key,
-					       SMB2_LEASE_KEY_SIZE);
-					break;
-				}
-			}
-			spin_unlock(&cfids->cfid_list_lock);
-		}
 	}
 	cfid->dentry = dentry;
 	cfid->tcon = tcon;
@@ -349,20 +351,13 @@ replay_again:
 	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,
-		.lease_flags = lease_flags,
-		.replay = !!(retries),
-	};
-
-	rc = SMB2_open_init(tcon, server,
-			    &rqst[0], &oplock, &oparms, utf16_path);
+	oparms = CIFS_OPARMS(cifs_sb, tcon, path,
+			     FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA, FILE_OPEN,
+			     cifs_create_options(cifs_sb, CREATE_NOT_FILE), 0);
+	oparms.fid = pfid;
+	oparms.replay = !!retries;
+
+	rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
 	if (rc)
 		goto oshr_free;
 	smb2_set_next_command(tcon, &rqst[0]);
@@ -477,21 +472,35 @@ out:
 	return rc;
 }
 
-static void
-smb2_close_cached_fid(struct kref *ref)
+static void __invalidate_cached_dirents(struct cached_fid *cfid)
 {
-	struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
 	struct cached_dirent *de, *q;
 
-	drop_cfid(cfid);
+	if (!cfid)
+		return;
 
-	/* Delete all cached dirent names */
+	mutex_lock(&cfid->dirents.de_mutex);
 	list_for_each_entry_safe(de, q, &cfid->dirents.entries, entry) {
 		list_del(&de->entry);
 		kfree(de->name);
 		kfree(de);
 	}
 
+	cfid->dirents.is_valid = false;
+	cfid->dirents.is_failed = false;
+	cfid->dirents.file = NULL;
+	cfid->dirents.pos = 0;
+	mutex_unlock(&cfid->dirents.de_mutex);
+}
+
+static void
+smb2_close_cached_fid(struct kref *ref)
+{
+	struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+
+	__invalidate_cached_dirents(cfid);
+	drop_cfid(cfid);
+
 	kfree(cfid->file_all_info);
 	cfid->file_all_info = NULL;
 	kfree(cfid->path);
@@ -531,6 +540,26 @@ bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode)
 	return true;
 }
 
+/*
+ * Invalidate cached dirents from @key's parent, regardless if @key itself is a cached dir.
+ *
+ * Lease breaks don't necessarily require this, and would require finding the child to begin with,
+ * so skip such cases.
+ */
+void invalidate_cached_dirents(struct cached_fids *cfids, const void *key, int mode)
+{
+	struct cached_fid *cfid = NULL;
+
+	if (mode == CFID_LOOKUP_LEASEKEY)
+		return;
+
+	cfid = find_cfid(cfids, key, mode, false);
+	if (cfid) {
+		__invalidate_cached_dirents(cfid);
+		kref_put(&cfid->refcount, smb2_close_cached_fid);
+	}
+}
+
 void close_cached_dir(struct cached_fid *cfid)
 {
 	kref_put(&cfid->refcount, smb2_close_cached_fid);
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 81969921ff88..de1231bdb0d9 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -59,11 +59,15 @@ struct cached_fids {
 	/* aggregate accounting for all cached dirents under this tcon */
 	atomic_long_t total_dirents_entries;
 	atomic64_t total_dirents_bytes;
+
+	/* convenience for parent lookups */
+	int dirsep;
 };
 
 /* Lookup modes for find_cached_dir() */
 enum {
 	CFID_LOOKUP_PATH,
+	CFID_LOOKUP_PARENT,
 	CFID_LOOKUP_DENTRY,
 	CFID_LOOKUP_LEASEKEY,
 };
@@ -86,6 +90,7 @@ extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char
 			   struct cifs_sb_info *cifs_sb, struct cached_fid **cfid);
 extern void close_cached_dir(struct cached_fid *cfid);
 extern bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode);
+extern void invalidate_cached_dirents(struct cached_fids *cfids, const void *key, int mode);
 extern void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
 extern void invalidate_all_cached_dirs(struct cached_fids *cfids);
 #endif			/* _CACHED_DIR_H */
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 5466f4968a84..5903c1fc96d4 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -190,9 +190,7 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
 	int disposition;
 	struct TCP_Server_Info *server = tcon->ses->server;
 	struct cifs_open_parms oparms;
-	struct cached_fid *parent_cfid = NULL;
 	int rdwr_for_fscache = 0;
-	__le32 lease_flags = 0;
 
 	*oplock = 0;
 	if (tcon->ses->server->oplocks)
@@ -314,27 +312,8 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
 	if (!tcon->unix_ext && (mode & S_IWUGO) == 0)
 		create_options |= CREATE_OPTION_READONLY;
 
-
 retry_open:
-	if (tcon->cfids && direntry->d_parent && server->dialect >= SMB30_PROT_ID) {
-		parent_cfid = NULL;
-		spin_lock(&tcon->cfids->cfid_list_lock);
-		list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
-			if (parent_cfid->dentry == direntry->d_parent) {
-				if (!is_valid_cached_dir(parent_cfid))
-					break;
-
-				cifs_dbg(FYI, "found a parent cached file handle\n");
-
-				lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
-				memcpy(fid->parent_lease_key, parent_cfid->fid.lease_key,
-				       SMB2_LEASE_KEY_SIZE);
-				parent_cfid->dirents.is_valid = false;
-				break;
-			}
-		}
-		spin_unlock(&tcon->cfids->cfid_list_lock);
-	}
+	invalidate_cached_dirents(tcon->cfids, direntry->d_parent, CFID_LOOKUP_DENTRY);
 
 	oparms = (struct cifs_open_parms) {
 		.tcon = tcon,
@@ -344,7 +323,6 @@ retry_open:
 		.disposition = disposition,
 		.path = full_path,
 		.fid = fid,
-		.lease_flags = lease_flags,
 		.mode = mode,
 	};
 	rc = server->ops->open(xid, &oparms, oplock, buf);
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index c3c77fc0c11d..97780e660281 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1120,6 +1120,8 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode,
 {
 	struct cifs_open_parms oparms;
 
+	invalidate_cached_dirents(tcon->cfids, name, CFID_LOOKUP_PARENT);
+
 	oparms = CIFS_OPARMS(cifs_sb, tcon, name, FILE_WRITE_ATTRIBUTES,
 			     FILE_CREATE, CREATE_NOT_FILE, mode);
 	return smb2_compound_op(xid, tcon, cifs_sb,
@@ -1141,6 +1143,8 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name,
 	u32 dosattrs;
 	int tmprc;
 
+	invalidate_cached_dirents(tcon->cfids, name, CFID_LOOKUP_PARENT);
+
 	in_iov.iov_base = &data;
 	in_iov.iov_len = sizeof(data);
 	cifs_i = CIFS_I(inode);
@@ -1196,6 +1200,13 @@ smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 	if (!utf16_path)
 		return -ENOMEM;
 
+	if (dentry) {
+		inode = d_inode(dentry);
+		invalidate_cached_dirents(tcon->cfids, dentry->d_parent, CFID_LOOKUP_DENTRY);
+	} else {
+		invalidate_cached_dirents(tcon->cfids, name, CFID_LOOKUP_PARENT);
+	}
+
 	if (smb3_encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 again:
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index acbfbb40932a..366dcb996f59 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2419,7 +2419,8 @@ add_lease_context(struct TCP_Server_Info *server,
 	if (iov[num].iov_base == NULL)
 		return -ENOMEM;
 	iov[num].iov_len = server->vals->create_lease_size;
-	req->RequestedOplockLevel = SMB2_OPLOCK_LEVEL_LEASE;
+	/* keep the requested oplock level in case of just setting ParentLeaseKey */
+	req->RequestedOplockLevel = *oplock;
 	*num_iovec = num + 1;
 	return 0;
 }
@@ -3001,6 +3002,34 @@ err_free_path:
 	return rc;
 }
 
+/*
+ * When opening a path, set ParentLeaseKey in @oparms if its parent is cached.
+ * We only have RH caching for dirs, so skip this on mkdir, unlink, rmdir.
+ *
+ * Ref: MS-SMB2 3.3.5.9 and MS-FSA 2.1.5.1
+ *
+ * Return: 0 if ParentLeaseKey was set in @oparms, -errno otherwise.
+ */
+static int check_cached_parent(struct cached_fids *cfids, struct cifs_open_parms *oparms)
+{
+	struct cached_fid *cfid;
+
+	if (!cfids || !oparms || !oparms->cifs_sb || !*oparms->path)
+		return -EINVAL;
+
+	cfid = find_cached_dir(cfids, oparms->path, CFID_LOOKUP_PARENT);
+	if (!cfid)
+		return -ENOENT;
+
+	cifs_dbg(FYI, "%s: found cached parent for path: %s\n", __func__, oparms->path);
+
+	memcpy(oparms->fid->parent_lease_key, cfid->fid.lease_key, SMB2_LEASE_KEY_SIZE);
+	oparms->lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
+	close_cached_dir(cfid);
+
+	return 0;
+}
+
 int
 SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 	       struct smb_rqst *rqst, __u8 *oplock,
@@ -3077,20 +3106,28 @@ SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 	iov[1].iov_len = uni_path_len;
 	iov[1].iov_base = path;
 
-	if ((!server->oplocks) || (tcon->no_lease))
+	if (!server->oplocks || tcon->no_lease)
 		*oplock = SMB2_OPLOCK_LEVEL_NONE;
 
-	if (!(server->capabilities & SMB2_GLOBAL_CAP_LEASING) ||
-	    *oplock == SMB2_OPLOCK_LEVEL_NONE)
-		req->RequestedOplockLevel = *oplock;
-	else if (!(server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING) &&
-		  (oparms->create_options & CREATE_NOT_FILE))
-		req->RequestedOplockLevel = *oplock; /* no srv lease support */
-	else {
-		rc = add_lease_context(server, req, iov, &n_iov,
-				       oparms->fid->lease_key, oplock,
-				       oparms->fid->parent_lease_key,
-				       oparms->lease_flags);
+	req->RequestedOplockLevel = *oplock;
+
+	/*
+	 * MS-SMB2 "Product Behavior" says Windows only checks/sets ParentLeaseKey when a lease is
+	 * requested for the child/target.
+	 * Practically speaking, adding the lease context with ParentLeaseKey set, even with oplock
+	 * none, works fine.
+	 * As a precaution, however, only set it for oplocks != none.
+	 */
+	if ((server->capabilities & SMB2_GLOBAL_CAP_LEASING) &&
+	    *oplock != SMB2_OPLOCK_LEVEL_NONE) {
+		rc = -EOPNOTSUPP;
+		if (server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING)
+			rc = check_cached_parent(tcon->cfids, oparms);
+
+		if (!rc || *oplock != SMB2_OPLOCK_LEVEL_NONE)
+			rc = add_lease_context(server, req, iov, &n_iov, oparms->fid->lease_key,
+					       oplock, oparms->fid->parent_lease_key,
+					       oparms->lease_flags);
 		if (rc)
 			return rc;
 	}