[PATCH] smb: improve directory cache reuse for readdir operations
Shyam Prasad N
nspmangalore at gmail.com
Thu Jun 12 17:28:53 UTC 2025
On Wed, Jun 11, 2025 at 8:45 PM Steve French <smfrench at gmail.com> wrote:
>
> 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
>
Looks good to me. You can add my RB.
--
Regards,
Shyam
More information about the samba-technical
mailing list