* Patch to add caching od directory entries for the cache dir @ 2022-05-03 7:09 Ronnie Sahlberg 2022-05-03 7:09 ` [PATCH] cifs: cache dirent names for cached directories Ronnie Sahlberg 0 siblings, 1 reply; 5+ messages in thread From: Ronnie Sahlberg @ 2022-05-03 7:09 UTC (permalink / raw) To: linux-cifs; +Cc: Steve French List, Steve, This is a wip patch that adds caching of directory entries while we hold a lease to the cached directory. This makes re-scanning a directory very cheap as long as we hold the lease. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] cifs: cache dirent names for cached directories 2022-05-03 7:09 Patch to add caching od directory entries for the cache dir Ronnie Sahlberg @ 2022-05-03 7:09 ` Ronnie Sahlberg 2022-05-03 7:19 ` Steve French 2022-05-03 23:55 ` Paulo Alcantara 0 siblings, 2 replies; 5+ messages in thread From: Ronnie Sahlberg @ 2022-05-03 7:09 UTC (permalink / raw) To: linux-cifs; +Cc: Steve French Cache the names of entries for cached-directories while we have a lease held. This allows us to avoid expensive calls to the server when re-scanning a cached directory. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/cifsglob.h | 84 ++++++++++++++-------- fs/cifs/misc.c | 2 + fs/cifs/readdir.c | 176 ++++++++++++++++++++++++++++++++++++++++++--- fs/cifs/smb2ops.c | 20 +++++- 4 files changed, 241 insertions(+), 41 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 8de977c359b1..4ec6a3df17fa 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -1009,6 +1009,58 @@ cap_unix(struct cifs_ses *ses) return ses->server->vals->cap_unix & ses->capabilities; } +/* + * common struct for holding inode info when searching for or updating an + * inode with new info + */ + +#define CIFS_FATTR_DFS_REFERRAL 0x1 +#define CIFS_FATTR_DELETE_PENDING 0x2 +#define CIFS_FATTR_NEED_REVAL 0x4 +#define CIFS_FATTR_INO_COLLISION 0x8 +#define CIFS_FATTR_UNKNOWN_NLINK 0x10 +#define CIFS_FATTR_FAKE_ROOT_INO 0x20 + +struct cifs_fattr { + u32 cf_flags; + u32 cf_cifsattrs; + u64 cf_uniqueid; + u64 cf_eof; + u64 cf_bytes; + u64 cf_createtime; + kuid_t cf_uid; + kgid_t cf_gid; + umode_t cf_mode; + dev_t cf_rdev; + unsigned int cf_nlink; + unsigned int cf_dtype; + struct timespec64 cf_atime; + struct timespec64 cf_mtime; + struct timespec64 cf_ctime; + u32 cf_cifstag; +}; + +struct cached_dirent { + struct list_head entry; + char *name; + int namelen; + loff_t pos; + + struct cifs_fattr fattr; +}; + +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 mutex de_mutex; + int pos; /* Expected ctx->pos */ + struct list_head entries; +}; + struct cached_fid { bool is_valid:1; /* Do we have a useable root fid */ bool file_all_info_is_valid:1; @@ -1021,6 +1073,7 @@ struct cached_fid { struct dentry *dentry; struct work_struct lease_break; struct smb2_file_all_info file_all_info; + struct cached_dirents dirents; }; /* @@ -1641,37 +1694,6 @@ struct file_list { struct cifsFileInfo *cfile; }; -/* - * common struct for holding inode info when searching for or updating an - * inode with new info - */ - -#define CIFS_FATTR_DFS_REFERRAL 0x1 -#define CIFS_FATTR_DELETE_PENDING 0x2 -#define CIFS_FATTR_NEED_REVAL 0x4 -#define CIFS_FATTR_INO_COLLISION 0x8 -#define CIFS_FATTR_UNKNOWN_NLINK 0x10 -#define CIFS_FATTR_FAKE_ROOT_INO 0x20 - -struct cifs_fattr { - u32 cf_flags; - u32 cf_cifsattrs; - u64 cf_uniqueid; - u64 cf_eof; - u64 cf_bytes; - u64 cf_createtime; - kuid_t cf_uid; - kgid_t cf_gid; - umode_t cf_mode; - dev_t cf_rdev; - unsigned int cf_nlink; - unsigned int cf_dtype; - struct timespec64 cf_atime; - struct timespec64 cf_mtime; - struct timespec64 cf_ctime; - u32 cf_cifstag; -}; - static inline void free_dfs_info_param(struct dfs_info3_param *param) { if (param) { diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index afaf59c22193..7fef08add3bc 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -114,6 +114,8 @@ tconInfoAlloc(void) kfree(ret_buf); return NULL; } + INIT_LIST_HEAD(&ret_buf->crfid.dirents.entries); + mutex_init(&ret_buf->crfid.dirents.de_mutex); atomic_inc(&tconInfoAllocCount); ret_buf->status = TID_NEW; diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 1929e80c09ee..8b3fbe6b3580 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -840,9 +840,112 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos, return rc; } +static bool emit_cached_dirents(struct cached_dirents *cde, + struct dir_context *ctx) +{ + struct list_head *tmp; + struct cached_dirent *dirent; + int rc; + + list_for_each(tmp, &cde->entries) { + dirent = list_entry(tmp, struct cached_dirent, entry); + if (ctx->pos >= dirent->pos) + continue; + ctx->pos = dirent->pos; + rc = dir_emit(ctx, dirent->name, dirent->namelen, + dirent->fattr.cf_uniqueid, + dirent->fattr.cf_dtype); + if (!rc) + return rc; + } + return true; +} + +static void update_cached_dirents_count(struct cached_dirents *cde, + struct dir_context *ctx) +{ + if (cde->ctx != ctx) + 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) +{ + if (cde->ctx != ctx) + return; + if (cde->is_valid || cde->is_failed) + return; + if (ctx->pos != cde->pos) + return; + + cde->is_valid = 1; +} + +static void add_cached_dirent(struct cached_dirents *cde, + struct dir_context *ctx, + const char *name, int namelen, + struct cifs_fattr *fattr) +{ + struct cached_dirent *de; + + if (cde->ctx != ctx) + return; + if (cde->is_valid || cde->is_failed) + return; + if (ctx->pos != cde->pos) { + cde->is_failed = 1; + return; + } + de = kzalloc(sizeof(*de), GFP_ATOMIC); + if (de == NULL) { + cde->is_failed = 1; + return; + } + de->name = kzalloc(namelen + 1, GFP_ATOMIC); + if (de->name == NULL) { + kfree(de); + cde->is_failed = 1; + return; + } + memcpy(de->name, name, namelen); + de->namelen = namelen; + de->pos = ctx->pos; + + memcpy(&de->fattr, fattr, sizeof(struct cifs_fattr)); + + list_add_tail(&de->entry, &cde->entries); +} + +static bool cifs_dir_emit(struct dir_context *ctx, + const char *name, int namelen, + struct cifs_fattr *fattr, + struct cached_fid *cfid) +{ + bool rc; + ino_t ino = cifs_uniqueid_to_ino_t(fattr->cf_uniqueid); + + rc = dir_emit(ctx, name, namelen, ino, fattr->cf_dtype); + if (!rc) + return rc; + + if (cfid) { + mutex_lock(&cfid->dirents.de_mutex); + add_cached_dirent(&cfid->dirents, ctx, name, namelen, + fattr); + mutex_unlock(&cfid->dirents.de_mutex); + } + + return rc; +} + static int cifs_filldir(char *find_entry, struct file *file, - struct dir_context *ctx, - char *scratch_buf, unsigned int max_len) + struct dir_context *ctx, + char *scratch_buf, unsigned int max_len, + struct cached_fid *cfid) { struct cifsFileInfo *file_info = file->private_data; struct super_block *sb = file_inode(file)->i_sb; @@ -851,7 +954,6 @@ static int cifs_filldir(char *find_entry, struct file *file, struct cifs_fattr fattr; struct qstr name; int rc = 0; - ino_t ino; rc = cifs_fill_dirent(&de, find_entry, file_info->srch_inf.info_level, file_info->srch_inf.unicode); @@ -931,8 +1033,8 @@ static int cifs_filldir(char *find_entry, struct file *file, cifs_prime_dcache(file_dentry(file), &name, &fattr); - ino = cifs_uniqueid_to_ino_t(fattr.cf_uniqueid); - return !dir_emit(ctx, name.name, name.len, ino, fattr.cf_dtype); + return !cifs_dir_emit(ctx, name.name, name.len, + &fattr, cfid); } @@ -942,7 +1044,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) unsigned int xid; int i; struct cifs_tcon *tcon; - struct cifsFileInfo *cifsFile = NULL; + struct cifsFileInfo *cifsFile; char *current_entry; int num_to_fill = 0; char *tmp_buf = NULL; @@ -950,6 +1052,8 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) unsigned int max_len; const char *full_path; void *page = alloc_dentry_path(); + struct cached_fid *cfid = NULL; + struct cifs_sb_info *cifs_sb = CIFS_FILE_SB(file); xid = get_xid(); @@ -969,6 +1073,48 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) if (rc) goto rddir2_exit; } + cifsFile = file->private_data; + tcon = tlink_tcon(cifsFile->tlink); + + rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid); + if (rc) + 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.ctx == NULL) { + cfid->dirents.ctx = ctx; + cfid->dirents.pos = 2; + } + /* + * If we already have the entire directory cached then + * we can just serve the cache. + */ + if (cfid->dirents.is_valid) { + if (!dir_emit_dots(file, ctx)){ + mutex_unlock(&cfid->dirents.de_mutex); + goto rddir2_exit; + } + emit_cached_dirents(&cfid->dirents, ctx); + mutex_unlock(&cfid->dirents.de_mutex); + goto rddir2_exit; + } + mutex_unlock(&cfid->dirents.de_mutex); + cache_not_found: + + /* Drop the cache while calling initiate_cifs_search and + * find_cifs_entry in case there will be reconnects during + * query_directory. + */ + if (cfid) { + close_cached_dir(cfid); + cfid = NULL; + } + if (!dir_emit_dots(file, ctx)) goto rddir2_exit; @@ -978,7 +1124,6 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) if it before then restart search if after then keep searching till find it */ - cifsFile = file->private_data; if (cifsFile->srch_inf.endOfSearch) { if (cifsFile->srch_inf.emptyDir) { cifs_dbg(FYI, "End of search, empty dir\n"); @@ -990,15 +1135,20 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) tcon->ses->server->close(xid, tcon, &cifsFile->fid); } */ - tcon = tlink_tcon(cifsFile->tlink); rc = find_cifs_entry(xid, tcon, ctx->pos, file, full_path, ¤t_entry, &num_to_fill); + open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid); if (rc) { cifs_dbg(FYI, "fce error %d\n", rc); goto rddir2_exit; } else if (current_entry != NULL) { cifs_dbg(FYI, "entry %lld found\n", ctx->pos); } else { + if (cfid) { + mutex_lock(&cfid->dirents.de_mutex); + finished_cached_dirents_count(&cfid->dirents, ctx); + mutex_unlock(&cfid->dirents.de_mutex); + } cifs_dbg(FYI, "Could not find entry\n"); goto rddir2_exit; } @@ -1028,7 +1178,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) */ *tmp_buf = 0; rc = cifs_filldir(current_entry, file, ctx, - tmp_buf, max_len); + tmp_buf, max_len, cfid); if (rc) { if (rc > 0) rc = 0; @@ -1036,6 +1186,12 @@ 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); + mutex_unlock(&cfid->dirents.de_mutex); + } + if (ctx->pos == cifsFile->srch_inf.index_of_last_entry) { cifs_dbg(FYI, "last entry in buf at pos %lld %s\n", @@ -1050,6 +1206,8 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) kfree(tmp_buf); rddir2_exit: + if (cfid) + close_cached_dir(cfid); free_dentry_path(page); free_xid(xid); return rc; diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index d6aaeff4a30a..10170266f44b 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -699,6 +699,8 @@ smb2_close_cached_fid(struct kref *ref) { struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount); + struct list_head *pos, *q; + struct cached_dirent *dirent; if (cfid->is_valid) { cifs_dbg(FYI, "clear cached root file handle\n"); @@ -718,6 +720,21 @@ smb2_close_cached_fid(struct kref *ref) dput(cfid->dentry); cfid->dentry = NULL; } + /* + * Delete all cached dirent names + */ + mutex_lock(&cfid->dirents.de_mutex); + list_for_each_safe(pos, q, &cfid->dirents.entries) { + dirent = list_entry(pos, struct cached_dirent, entry); + list_del(pos); + kfree(dirent->name); + kfree(dirent); + } + cfid->dirents.is_valid = 0; + cfid->dirents.is_failed = 0; + cfid->dirents.ctx = NULL; + cfid->dirents.pos = 0; + mutex_unlock(&cfid->dirents.de_mutex); } void close_cached_dir(struct cached_fid *cfid) @@ -776,7 +793,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid; struct dentry *dentry; - if (tcon->nohandlecache) + if (tcon == NULL || tcon->nohandlecache || + is_smb1_server(tcon->ses->server)) return -ENOTSUPP; if (cifs_sb->root == NULL) -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: cache dirent names for cached directories 2022-05-03 7:09 ` [PATCH] cifs: cache dirent names for cached directories Ronnie Sahlberg @ 2022-05-03 7:19 ` Steve French 2022-05-03 23:55 ` Paulo Alcantara 1 sibling, 0 replies; 5+ messages in thread From: Steve French @ 2022-05-03 7:19 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: linux-cifs tentatively put in cifs-2.6.git for-next pending more review/testing to allow it to be scanned FYI - also cleaned up minor whitespace errors On Tue, May 3, 2022 at 2:10 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > Cache the names of entries for cached-directories while we have a lease held. > This allows us to avoid expensive calls to the server when re-scanning > a cached directory. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/cifsglob.h | 84 ++++++++++++++-------- > fs/cifs/misc.c | 2 + > fs/cifs/readdir.c | 176 ++++++++++++++++++++++++++++++++++++++++++--- > fs/cifs/smb2ops.c | 20 +++++- > 4 files changed, 241 insertions(+), 41 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 8de977c359b1..4ec6a3df17fa 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -1009,6 +1009,58 @@ cap_unix(struct cifs_ses *ses) > return ses->server->vals->cap_unix & ses->capabilities; > } > > +/* > + * common struct for holding inode info when searching for or updating an > + * inode with new info > + */ > + > +#define CIFS_FATTR_DFS_REFERRAL 0x1 > +#define CIFS_FATTR_DELETE_PENDING 0x2 > +#define CIFS_FATTR_NEED_REVAL 0x4 > +#define CIFS_FATTR_INO_COLLISION 0x8 > +#define CIFS_FATTR_UNKNOWN_NLINK 0x10 > +#define CIFS_FATTR_FAKE_ROOT_INO 0x20 > + > +struct cifs_fattr { > + u32 cf_flags; > + u32 cf_cifsattrs; > + u64 cf_uniqueid; > + u64 cf_eof; > + u64 cf_bytes; > + u64 cf_createtime; > + kuid_t cf_uid; > + kgid_t cf_gid; > + umode_t cf_mode; > + dev_t cf_rdev; > + unsigned int cf_nlink; > + unsigned int cf_dtype; > + struct timespec64 cf_atime; > + struct timespec64 cf_mtime; > + struct timespec64 cf_ctime; > + u32 cf_cifstag; > +}; > + > +struct cached_dirent { > + struct list_head entry; > + char *name; > + int namelen; > + loff_t pos; > + > + struct cifs_fattr fattr; > +}; > + > +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 mutex de_mutex; > + int pos; /* Expected ctx->pos */ > + struct list_head entries; > +}; > + > struct cached_fid { > bool is_valid:1; /* Do we have a useable root fid */ > bool file_all_info_is_valid:1; > @@ -1021,6 +1073,7 @@ struct cached_fid { > struct dentry *dentry; > struct work_struct lease_break; > struct smb2_file_all_info file_all_info; > + struct cached_dirents dirents; > }; > > /* > @@ -1641,37 +1694,6 @@ struct file_list { > struct cifsFileInfo *cfile; > }; > > -/* > - * common struct for holding inode info when searching for or updating an > - * inode with new info > - */ > - > -#define CIFS_FATTR_DFS_REFERRAL 0x1 > -#define CIFS_FATTR_DELETE_PENDING 0x2 > -#define CIFS_FATTR_NEED_REVAL 0x4 > -#define CIFS_FATTR_INO_COLLISION 0x8 > -#define CIFS_FATTR_UNKNOWN_NLINK 0x10 > -#define CIFS_FATTR_FAKE_ROOT_INO 0x20 > - > -struct cifs_fattr { > - u32 cf_flags; > - u32 cf_cifsattrs; > - u64 cf_uniqueid; > - u64 cf_eof; > - u64 cf_bytes; > - u64 cf_createtime; > - kuid_t cf_uid; > - kgid_t cf_gid; > - umode_t cf_mode; > - dev_t cf_rdev; > - unsigned int cf_nlink; > - unsigned int cf_dtype; > - struct timespec64 cf_atime; > - struct timespec64 cf_mtime; > - struct timespec64 cf_ctime; > - u32 cf_cifstag; > -}; > - > static inline void free_dfs_info_param(struct dfs_info3_param *param) > { > if (param) { > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c > index afaf59c22193..7fef08add3bc 100644 > --- a/fs/cifs/misc.c > +++ b/fs/cifs/misc.c > @@ -114,6 +114,8 @@ tconInfoAlloc(void) > kfree(ret_buf); > return NULL; > } > + INIT_LIST_HEAD(&ret_buf->crfid.dirents.entries); > + mutex_init(&ret_buf->crfid.dirents.de_mutex); > > atomic_inc(&tconInfoAllocCount); > ret_buf->status = TID_NEW; > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > index 1929e80c09ee..8b3fbe6b3580 100644 > --- a/fs/cifs/readdir.c > +++ b/fs/cifs/readdir.c > @@ -840,9 +840,112 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos, > return rc; > } > > +static bool emit_cached_dirents(struct cached_dirents *cde, > + struct dir_context *ctx) > +{ > + struct list_head *tmp; > + struct cached_dirent *dirent; > + int rc; > + > + list_for_each(tmp, &cde->entries) { > + dirent = list_entry(tmp, struct cached_dirent, entry); > + if (ctx->pos >= dirent->pos) > + continue; > + ctx->pos = dirent->pos; > + rc = dir_emit(ctx, dirent->name, dirent->namelen, > + dirent->fattr.cf_uniqueid, > + dirent->fattr.cf_dtype); > + if (!rc) > + return rc; > + } > + return true; > +} > + > +static void update_cached_dirents_count(struct cached_dirents *cde, > + struct dir_context *ctx) > +{ > + if (cde->ctx != ctx) > + 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) > +{ > + if (cde->ctx != ctx) > + return; > + if (cde->is_valid || cde->is_failed) > + return; > + if (ctx->pos != cde->pos) > + return; > + > + cde->is_valid = 1; > +} > + > +static void add_cached_dirent(struct cached_dirents *cde, > + struct dir_context *ctx, > + const char *name, int namelen, > + struct cifs_fattr *fattr) > +{ > + struct cached_dirent *de; > + > + if (cde->ctx != ctx) > + return; > + if (cde->is_valid || cde->is_failed) > + return; > + if (ctx->pos != cde->pos) { > + cde->is_failed = 1; > + return; > + } > + de = kzalloc(sizeof(*de), GFP_ATOMIC); > + if (de == NULL) { > + cde->is_failed = 1; > + return; > + } > + de->name = kzalloc(namelen + 1, GFP_ATOMIC); > + if (de->name == NULL) { > + kfree(de); > + cde->is_failed = 1; > + return; > + } > + memcpy(de->name, name, namelen); > + de->namelen = namelen; > + de->pos = ctx->pos; > + > + memcpy(&de->fattr, fattr, sizeof(struct cifs_fattr)); > + > + list_add_tail(&de->entry, &cde->entries); > +} > + > +static bool cifs_dir_emit(struct dir_context *ctx, > + const char *name, int namelen, > + struct cifs_fattr *fattr, > + struct cached_fid *cfid) > +{ > + bool rc; > + ino_t ino = cifs_uniqueid_to_ino_t(fattr->cf_uniqueid); > + > + rc = dir_emit(ctx, name, namelen, ino, fattr->cf_dtype); > + if (!rc) > + return rc; > + > + if (cfid) { > + mutex_lock(&cfid->dirents.de_mutex); > + add_cached_dirent(&cfid->dirents, ctx, name, namelen, > + fattr); > + mutex_unlock(&cfid->dirents.de_mutex); > + } > + > + return rc; > +} > + > static int cifs_filldir(char *find_entry, struct file *file, > - struct dir_context *ctx, > - char *scratch_buf, unsigned int max_len) > + struct dir_context *ctx, > + char *scratch_buf, unsigned int max_len, > + struct cached_fid *cfid) > { > struct cifsFileInfo *file_info = file->private_data; > struct super_block *sb = file_inode(file)->i_sb; > @@ -851,7 +954,6 @@ static int cifs_filldir(char *find_entry, struct file *file, > struct cifs_fattr fattr; > struct qstr name; > int rc = 0; > - ino_t ino; > > rc = cifs_fill_dirent(&de, find_entry, file_info->srch_inf.info_level, > file_info->srch_inf.unicode); > @@ -931,8 +1033,8 @@ static int cifs_filldir(char *find_entry, struct file *file, > > cifs_prime_dcache(file_dentry(file), &name, &fattr); > > - ino = cifs_uniqueid_to_ino_t(fattr.cf_uniqueid); > - return !dir_emit(ctx, name.name, name.len, ino, fattr.cf_dtype); > + return !cifs_dir_emit(ctx, name.name, name.len, > + &fattr, cfid); > } > > > @@ -942,7 +1044,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > unsigned int xid; > int i; > struct cifs_tcon *tcon; > - struct cifsFileInfo *cifsFile = NULL; > + struct cifsFileInfo *cifsFile; > char *current_entry; > int num_to_fill = 0; > char *tmp_buf = NULL; > @@ -950,6 +1052,8 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > unsigned int max_len; > const char *full_path; > void *page = alloc_dentry_path(); > + struct cached_fid *cfid = NULL; > + struct cifs_sb_info *cifs_sb = CIFS_FILE_SB(file); > > xid = get_xid(); > > @@ -969,6 +1073,48 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > if (rc) > goto rddir2_exit; > } > + cifsFile = file->private_data; > + tcon = tlink_tcon(cifsFile->tlink); > + > + rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid); > + if (rc) > + 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.ctx == NULL) { > + cfid->dirents.ctx = ctx; > + cfid->dirents.pos = 2; > + } > + /* > + * If we already have the entire directory cached then > + * we can just serve the cache. > + */ > + if (cfid->dirents.is_valid) { > + if (!dir_emit_dots(file, ctx)){ > + mutex_unlock(&cfid->dirents.de_mutex); > + goto rddir2_exit; > + } > + emit_cached_dirents(&cfid->dirents, ctx); > + mutex_unlock(&cfid->dirents.de_mutex); > + goto rddir2_exit; > + } > + mutex_unlock(&cfid->dirents.de_mutex); > + cache_not_found: > + > + /* Drop the cache while calling initiate_cifs_search and > + * find_cifs_entry in case there will be reconnects during > + * query_directory. > + */ > + if (cfid) { > + close_cached_dir(cfid); > + cfid = NULL; > + } > + > > if (!dir_emit_dots(file, ctx)) > goto rddir2_exit; > @@ -978,7 +1124,6 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > if it before then restart search > if after then keep searching till find it */ > > - cifsFile = file->private_data; > if (cifsFile->srch_inf.endOfSearch) { > if (cifsFile->srch_inf.emptyDir) { > cifs_dbg(FYI, "End of search, empty dir\n"); > @@ -990,15 +1135,20 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > tcon->ses->server->close(xid, tcon, &cifsFile->fid); > } */ > > - tcon = tlink_tcon(cifsFile->tlink); > rc = find_cifs_entry(xid, tcon, ctx->pos, file, full_path, > ¤t_entry, &num_to_fill); > + open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid); > if (rc) { > cifs_dbg(FYI, "fce error %d\n", rc); > goto rddir2_exit; > } else if (current_entry != NULL) { > cifs_dbg(FYI, "entry %lld found\n", ctx->pos); > } else { > + if (cfid) { > + mutex_lock(&cfid->dirents.de_mutex); > + finished_cached_dirents_count(&cfid->dirents, ctx); > + mutex_unlock(&cfid->dirents.de_mutex); > + } > cifs_dbg(FYI, "Could not find entry\n"); > goto rddir2_exit; > } > @@ -1028,7 +1178,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > */ > *tmp_buf = 0; > rc = cifs_filldir(current_entry, file, ctx, > - tmp_buf, max_len); > + tmp_buf, max_len, cfid); > if (rc) { > if (rc > 0) > rc = 0; > @@ -1036,6 +1186,12 @@ 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); > + mutex_unlock(&cfid->dirents.de_mutex); > + } > + > if (ctx->pos == > cifsFile->srch_inf.index_of_last_entry) { > cifs_dbg(FYI, "last entry in buf at pos %lld %s\n", > @@ -1050,6 +1206,8 @@ int cifs_readdir(struct file *file, struct dir_context *ctx) > kfree(tmp_buf); > > rddir2_exit: > + if (cfid) > + close_cached_dir(cfid); > free_dentry_path(page); > free_xid(xid); > return rc; > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index d6aaeff4a30a..10170266f44b 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -699,6 +699,8 @@ smb2_close_cached_fid(struct kref *ref) > { > struct cached_fid *cfid = container_of(ref, struct cached_fid, > refcount); > + struct list_head *pos, *q; > + struct cached_dirent *dirent; > > if (cfid->is_valid) { > cifs_dbg(FYI, "clear cached root file handle\n"); > @@ -718,6 +720,21 @@ smb2_close_cached_fid(struct kref *ref) > dput(cfid->dentry); > cfid->dentry = NULL; > } > + /* > + * Delete all cached dirent names > + */ > + mutex_lock(&cfid->dirents.de_mutex); > + list_for_each_safe(pos, q, &cfid->dirents.entries) { > + dirent = list_entry(pos, struct cached_dirent, entry); > + list_del(pos); > + kfree(dirent->name); > + kfree(dirent); > + } > + cfid->dirents.is_valid = 0; > + cfid->dirents.is_failed = 0; > + cfid->dirents.ctx = NULL; > + cfid->dirents.pos = 0; > + mutex_unlock(&cfid->dirents.de_mutex); > } > > void close_cached_dir(struct cached_fid *cfid) > @@ -776,7 +793,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, > struct cifs_fid *pfid; > struct dentry *dentry; > > - if (tcon->nohandlecache) > + if (tcon == NULL || tcon->nohandlecache || > + is_smb1_server(tcon->ses->server)) > return -ENOTSUPP; > > if (cifs_sb->root == NULL) > -- > 2.30.2 > -- Thanks, Steve ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: cache dirent names for cached directories 2022-05-03 7:09 ` [PATCH] cifs: cache dirent names for cached directories Ronnie Sahlberg 2022-05-03 7:19 ` Steve French @ 2022-05-03 23:55 ` Paulo Alcantara 2022-05-04 1:44 ` ronnie sahlberg 1 sibling, 1 reply; 5+ messages in thread From: Paulo Alcantara @ 2022-05-03 23:55 UTC (permalink / raw) To: Ronnie Sahlberg, linux-cifs; +Cc: Steve French Hi Ronnie, Ronnie Sahlberg <lsahlber@redhat.com> writes: > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > index 1929e80c09ee..8b3fbe6b3580 100644 > --- a/fs/cifs/readdir.c > +++ b/fs/cifs/readdir.c > @@ -840,9 +840,112 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos, > return rc; > } > > +static bool emit_cached_dirents(struct cached_dirents *cde, > + struct dir_context *ctx) > +{ > + struct list_head *tmp; > + struct cached_dirent *dirent; > + int rc; > + > + list_for_each(tmp, &cde->entries) { > + dirent = list_entry(tmp, struct cached_dirent, entry); What about list_for_each_entry()? > +static void add_cached_dirent(struct cached_dirents *cde, > + struct dir_context *ctx, > + const char *name, int namelen, > + struct cifs_fattr *fattr) > +{ > + struct cached_dirent *de; > + > + if (cde->ctx != ctx) > + return; > + if (cde->is_valid || cde->is_failed) > + return; > + if (ctx->pos != cde->pos) { > + cde->is_failed = 1; > + return; > + } > + de = kzalloc(sizeof(*de), GFP_ATOMIC); > + if (de == NULL) { > + cde->is_failed = 1; > + return; > + } > + de->name = kzalloc(namelen + 1, GFP_ATOMIC); > + if (de->name == NULL) { > + kfree(de); > + cde->is_failed = 1; > + return; > + } > + memcpy(de->name, name, namelen); Replace the above kzalloc() & memcpy() with kstrndup()? > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index d6aaeff4a30a..10170266f44b 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -699,6 +699,8 @@ smb2_close_cached_fid(struct kref *ref) > { > struct cached_fid *cfid = container_of(ref, struct cached_fid, > refcount); > + struct list_head *pos, *q; > + struct cached_dirent *dirent; > > if (cfid->is_valid) { > cifs_dbg(FYI, "clear cached root file handle\n"); > @@ -718,6 +720,21 @@ smb2_close_cached_fid(struct kref *ref) > dput(cfid->dentry); > cfid->dentry = NULL; > } > + /* > + * Delete all cached dirent names > + */ > + mutex_lock(&cfid->dirents.de_mutex); > + list_for_each_safe(pos, q, &cfid->dirents.entries) { > + dirent = list_entry(pos, struct cached_dirent, entry); list_for_each_entry_safe()? Nice job! Looks good to me, Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: cache dirent names for cached directories 2022-05-03 23:55 ` Paulo Alcantara @ 2022-05-04 1:44 ` ronnie sahlberg 0 siblings, 0 replies; 5+ messages in thread From: ronnie sahlberg @ 2022-05-04 1:44 UTC (permalink / raw) To: Paulo Alcantara; +Cc: Ronnie Sahlberg, linux-cifs, Steve French Good idea, will resend. Thanks. On Wed, 4 May 2022 at 11:08, Paulo Alcantara <pc@cjr.nz> wrote: > > Hi Ronnie, > > Ronnie Sahlberg <lsahlber@redhat.com> writes: > > > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > > index 1929e80c09ee..8b3fbe6b3580 100644 > > --- a/fs/cifs/readdir.c > > +++ b/fs/cifs/readdir.c > > @@ -840,9 +840,112 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos, > > return rc; > > } > > > > +static bool emit_cached_dirents(struct cached_dirents *cde, > > + struct dir_context *ctx) > > +{ > > + struct list_head *tmp; > > + struct cached_dirent *dirent; > > + int rc; > > + > > + list_for_each(tmp, &cde->entries) { > > + dirent = list_entry(tmp, struct cached_dirent, entry); > > What about list_for_each_entry()? > > > +static void add_cached_dirent(struct cached_dirents *cde, > > + struct dir_context *ctx, > > + const char *name, int namelen, > > + struct cifs_fattr *fattr) > > +{ > > + struct cached_dirent *de; > > + > > + if (cde->ctx != ctx) > > + return; > > + if (cde->is_valid || cde->is_failed) > > + return; > > + if (ctx->pos != cde->pos) { > > + cde->is_failed = 1; > > + return; > > + } > > + de = kzalloc(sizeof(*de), GFP_ATOMIC); > > + if (de == NULL) { > > + cde->is_failed = 1; > > + return; > > + } > > + de->name = kzalloc(namelen + 1, GFP_ATOMIC); > > + if (de->name == NULL) { > > + kfree(de); > > + cde->is_failed = 1; > > + return; > > + } > > + memcpy(de->name, name, namelen); > > Replace the above kzalloc() & memcpy() with kstrndup()? > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > > index d6aaeff4a30a..10170266f44b 100644 > > --- a/fs/cifs/smb2ops.c > > +++ b/fs/cifs/smb2ops.c > > @@ -699,6 +699,8 @@ smb2_close_cached_fid(struct kref *ref) > > { > > struct cached_fid *cfid = container_of(ref, struct cached_fid, > > refcount); > > + struct list_head *pos, *q; > > + struct cached_dirent *dirent; > > > > if (cfid->is_valid) { > > cifs_dbg(FYI, "clear cached root file handle\n"); > > @@ -718,6 +720,21 @@ smb2_close_cached_fid(struct kref *ref) > > dput(cfid->dentry); > > cfid->dentry = NULL; > > } > > + /* > > + * Delete all cached dirent names > > + */ > > + mutex_lock(&cfid->dirents.de_mutex); > > + list_for_each_safe(pos, q, &cfid->dirents.entries) { > > + dirent = list_entry(pos, struct cached_dirent, entry); > > list_for_each_entry_safe()? > > Nice job! Looks good to me, > > Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-04 1:44 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-05-03 7:09 Patch to add caching od directory entries for the cache dir Ronnie Sahlberg 2022-05-03 7:09 ` [PATCH] cifs: cache dirent names for cached directories Ronnie Sahlberg 2022-05-03 7:19 ` Steve French 2022-05-03 23:55 ` Paulo Alcantara 2022-05-04 1:44 ` ronnie sahlberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox