[PATCH] smb: improve directory cache reuse for readdir operations

[Steve French]

merged this updated patch into cifs-2.6.git for-next, running xfstests
on it now.

Looks very promising, and we have a couple more dir lease
optimizations to try out that should also help a lot with perf, and
reducing load on servers by sending fewer metadata ops over the wire

On Wed, Jun 11, 2025 at 6:29 AM Bharath SM <bharathsm.hsk at gmail.com> wrote:
>
> Currently, cached directory contents were not reused across subsequent
> 'ls' operations because the cache validity check relied on comparing
> the ctx pointer, which changes with each readdir invocation. As a
> result, the cached dir entries was not marked as valid and the cache was
> not utilized for subsequent 'ls' operations.
>
> This change uses the file pointer, which remains consistent across all
> readdir calls for a given directory instance, to associate and validate
> the cache. As a result, cached directory contents can now be
> correctly reused, improving performance for repeated directory listings.
>
> Performance gains with local windows SMB server:
>
> Without the patch and default actimeo=1:
>  1000 directory enumeration operations on dir with 10k files took 135.0s
>
> With this patch and actimeo=0:
>  1000 directory enumeration operations on dir with 10k files took just 5.1s
>
> Signed-off-by: Bharath SM <bharathsm at microsoft.com>
> ---
>  fs/smb/client/cached_dir.h |  8 ++++----
>  fs/smb/client/readdir.c    | 28 +++++++++++++++-------------
>  2 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
> index 1dfe79d947a6..bc8a812ff95f 100644
> --- a/fs/smb/client/cached_dir.h
> +++ b/fs/smb/client/cached_dir.h
> @@ -21,10 +21,10 @@ struct cached_dirent {
>  struct cached_dirents {
>         bool is_valid:1;
>         bool is_failed:1;
> -       struct dir_context *ctx; /*
> -                                 * Only used to make sure we only take entries
> -                                 * from a single context. Never dereferenced.
> -                                 */
> +       struct file *file; /*
> +                           * Used to associate the cache with a single
> +                           * open file instance.
> +                           */
>         struct mutex de_mutex;
>         int pos;                 /* Expected ctx->pos */
>         struct list_head entries;
> diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> index f9f11cbf89be..ba0193cf9033 100644
> --- a/fs/smb/client/readdir.c
> +++ b/fs/smb/client/readdir.c
> @@ -851,9 +851,9 @@ static bool emit_cached_dirents(struct cached_dirents *cde,
>  }
>
>  static void update_cached_dirents_count(struct cached_dirents *cde,
> -                                       struct dir_context *ctx)
> +                                       struct file *file)
>  {
> -       if (cde->ctx != ctx)
> +       if (cde->file != file)
>                 return;
>         if (cde->is_valid || cde->is_failed)
>                 return;
> @@ -862,9 +862,9 @@ static void update_cached_dirents_count(struct cached_dirents *cde,
>  }
>
>  static void finished_cached_dirents_count(struct cached_dirents *cde,
> -                                       struct dir_context *ctx)
> +                                       struct dir_context *ctx, struct file *file)
>  {
> -       if (cde->ctx != ctx)
> +       if (cde->file != file)
>                 return;
>         if (cde->is_valid || cde->is_failed)
>                 return;
> @@ -877,11 +877,12 @@ static void finished_cached_dirents_count(struct cached_dirents *cde,
>  static void add_cached_dirent(struct cached_dirents *cde,
>                               struct dir_context *ctx,
>                               const char *name, int namelen,
> -                             struct cifs_fattr *fattr)
> +                             struct cifs_fattr *fattr,
> +                                 struct file *file)
>  {
>         struct cached_dirent *de;
>
> -       if (cde->ctx != ctx)
> +       if (cde->file != file)
>                 return;
>         if (cde->is_valid || cde->is_failed)
>                 return;
> @@ -911,7 +912,8 @@ static void add_cached_dirent(struct cached_dirents *cde,
>  static bool cifs_dir_emit(struct dir_context *ctx,
>                           const char *name, int namelen,
>                           struct cifs_fattr *fattr,
> -                         struct cached_fid *cfid)
> +                         struct cached_fid *cfid,
> +                         struct file *file)
>  {
>         bool rc;
>         ino_t ino = cifs_uniqueid_to_ino_t(fattr->cf_uniqueid);
> @@ -923,7 +925,7 @@ static bool cifs_dir_emit(struct dir_context *ctx,
>         if (cfid) {
>                 mutex_lock(&cfid->dirents.de_mutex);
>                 add_cached_dirent(&cfid->dirents, ctx, name, namelen,
> -                                 fattr);
> +                                 fattr, file);
>                 mutex_unlock(&cfid->dirents.de_mutex);
>         }
>
> @@ -1023,7 +1025,7 @@ static int cifs_filldir(char *find_entry, struct file *file,
>         cifs_prime_dcache(file_dentry(file), &name, &fattr);
>
>         return !cifs_dir_emit(ctx, name.name, name.len,
> -                             &fattr, cfid);
> +                             &fattr, cfid, file);
>  }
>
>
> @@ -1074,8 +1076,8 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
>          * we need to initialize scanning and storing the
>          * directory content.
>          */
> -       if (ctx->pos == 0 && cfid->dirents.ctx == NULL) {
> -               cfid->dirents.ctx = ctx;
> +       if (ctx->pos == 0 && cfid->dirents.file == NULL) {
> +               cfid->dirents.file = file;
>                 cfid->dirents.pos = 2;
>         }
>         /*
> @@ -1143,7 +1145,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
>         } else {
>                 if (cfid) {
>                         mutex_lock(&cfid->dirents.de_mutex);
> -                       finished_cached_dirents_count(&cfid->dirents, ctx);
> +                       finished_cached_dirents_count(&cfid->dirents, ctx, file);
>                         mutex_unlock(&cfid->dirents.de_mutex);
>                 }
>                 cifs_dbg(FYI, "Could not find entry\n");
> @@ -1184,7 +1186,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
>                 ctx->pos++;
>                 if (cfid) {
>                         mutex_lock(&cfid->dirents.de_mutex);
> -                       update_cached_dirents_count(&cfid->dirents, ctx);
> +                       update_cached_dirents_count(&cfid->dirents, file);
>                         mutex_unlock(&cfid->dirents.de_mutex);
>                 }
>
> --
> 2.43.0
>


-- 
Thanks,

Steve