smb: client: wait for concurrent caching of dirents in cifs_readdir()

The file struct passed down to cifs_readdir() is a stack variable, which
means it makes no sense to keep/track it across a cached dir lifetime.

Instead, use it to track concurrent accesses to the same cached path,
and wait for the previous one to finish filling/emitting.

Without this patch, virtually every 'ls' will issue a Find request,
even when we have both directory and dirents cached and valid.

With this patch, the chances of cache hits increases a lot, so on
highly concurrent scenarios, the amount of network calls are
drastically reduced.

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

2 files changed, 40 insertions, 24 deletions

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 22709b0bfb3d..96c7154d8031 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -649,6 +649,9 @@ static struct cached_fid *init_cached_dir(const char *path)
 	/* this is caller ref */
 	kref_get(&cfid->refcount);
 
+	/* initial cached dirents position */
+	cfid->dirents.pos = 2;
+
 	return cfid;
 }
 
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 9470723a4579..32dcbb702b14 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -860,22 +860,16 @@ static bool emit_cached_dirents(struct cached_dirents *cde,
 	return true;
 }
 
-static void update_cached_dirents_count(struct cached_dirents *cde,
-					struct file *file)
+static void update_cached_dirents_count(struct cached_dirents *cde)
 {
-	if (cde->file != file)
-		return;
 	if (cde->is_valid || cde->is_failed)
 		return;
 
 	cde->pos++;
 }
 
-static void finished_cached_dirents_count(struct cached_dirents *cde,
-					struct dir_context *ctx, struct file *file)
+static void finished_cached_dirents_count(struct cached_dirents *cde, struct dir_context *ctx)
 {
-	if (cde->file != file)
-		return;
 	if (cde->is_valid || cde->is_failed)
 		return;
 	if (ctx->pos != cde->pos)
@@ -929,8 +923,8 @@ static bool cifs_dir_emit(struct dir_context *ctx,
 			  struct file *file)
 {
 	size_t delta_bytes = 0;
-	bool rc, added = false;
 	ino_t ino = cifs_uniqueid_to_ino_t(fattr->cf_uniqueid);
+	bool rc;
 
 	rc = dir_emit(ctx, name, namelen, ino, fattr->cf_dtype);
 	if (!rc)
@@ -941,11 +935,10 @@ static bool cifs_dir_emit(struct dir_context *ctx,
 		delta_bytes = sizeof(struct cached_dirent) + (size_t)namelen + 1;
 
 		mutex_lock(&cfid->dirents.de_mutex);
-		added = add_cached_dirent(&cfid->dirents, ctx, name, namelen,
-					  fattr, file);
+		rc = add_cached_dirent(&cfid->dirents, ctx, name, namelen, fattr, file);
 		mutex_unlock(&cfid->dirents.de_mutex);
 
-		if (added) {
+		if (rc) {
 			/* per-tcon then global for consistency with free path */
 			atomic64_add((long long)delta_bytes, &cfid->cfids->total_dirents_bytes);
 			atomic_long_inc(&cfid->cfids->total_dirents_entries);
@@ -1092,20 +1085,14 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
 
 	rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid);
 	cifs_put_tlink(tlink);
-	if (rc)
+retry_cached:
+	if (!cfid || !is_valid_cached_dir(cfid)) {
+		rc = -ENOENT;
 		goto cache_not_found;
+	}
 
 	mutex_lock(&cfid->dirents.de_mutex);
 	/*
-	 * If this was reading from the start of the directory
-	 * we need to initialize scanning and storing the
-	 * directory content.
-	 */
-	if (ctx->pos == 0 && cfid->dirents.file == NULL) {
-		cfid->dirents.file = file;
-		cfid->dirents.pos = 2;
-	}
-	/*
 	 * If we already have the entire directory cached then
 	 * we can just serve the cache.
 	 */
@@ -1118,10 +1105,32 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
 		mutex_unlock(&cfid->dirents.de_mutex);
 		goto rddir2_exit;
 	}
+
+	if (cfid->dirents.is_failed) {
+		rc = -ENOENT;
+		mutex_unlock(&cfid->dirents.de_mutex);
+		goto cache_not_found;
+	}
+
+	if (!cfid->dirents.file)
+		cfid->dirents.file = file;
+	else if (cfid->dirents.file != file)
+		rc = -EAGAIN;
 	mutex_unlock(&cfid->dirents.de_mutex);
 
+	/* someone else is filling up the dirents for this cfid, wait for them to finish */
+	if (rc == -EAGAIN) {
+		rc = 0;
+		goto retry_cached;
+	}
+
 	/* keep our cfid ref, but check if still valid after network calls */
 cache_not_found:
+	if (rc && cfid) {
+		close_cached_dir(cfid);
+		cfid = NULL;
+	}
+
 	/*
 	 * Ensure FindFirst doesn't fail before doing filldir() for '.' and
 	 * '..'. Otherwise we won't be able to notify VFS in case of failure.
@@ -1171,7 +1180,7 @@ cache_not_found:
 	} else {
 		if (cfid) {
 			mutex_lock(&cfid->dirents.de_mutex);
-			finished_cached_dirents_count(&cfid->dirents, ctx, file);
+			finished_cached_dirents_count(&cfid->dirents, ctx);
 			mutex_unlock(&cfid->dirents.de_mutex);
 		}
 		cifs_dbg(FYI, "Could not find entry\n");
@@ -1212,7 +1221,7 @@ cache_not_found:
 		ctx->pos++;
 		if (cfid) {
 			mutex_lock(&cfid->dirents.de_mutex);
-			update_cached_dirents_count(&cfid->dirents, file);
+			update_cached_dirents_count(&cfid->dirents);
 			mutex_unlock(&cfid->dirents.de_mutex);
 		}
 
@@ -1231,8 +1240,12 @@ cache_not_found:
 
 rddir2_exit:
 	if (cfid) {
+		if (rc || cfid->dirents.is_failed || !cifsFile || cifsFile->srch_inf.endOfSearch)
+			cfid->dirents.file = NULL;
+
 		if (cifsFile)
 			cifsFile->invalidHandle = true;
+
 		close_cached_dir(cfid);
 	}
 	free_dentry_path(page);