* 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