From 4244daa61f7c8288f7d059763954b515603dd30a Mon Sep 17 00:00:00 2001 From: Enzo Matsumiya Date: Fri, 12 Sep 2025 14:36:25 -0300 Subject: smb: client: introduce cached_fid->state Add cached_fid->state to track a cfid state more accurately and flexibly. Remove bools ->has_lease, ->is_open, ->file_all_info_valid. Add states: - CFID_IS_OPEN - CFID_IS_OPENING - CFID_INVALID See cached_dir.h for more info on state flags. Note CFID_IS_OPEN covers both ->has_lease and ->is_open because, after all, we only have a lease if we opened it to begin with. Other changes: - ->file_all_info becomes a pointer and its validity becomes its allocation (NULL or not NULL) - move full cfid initialization to the end of open_cached_dir() (instead of in between calls), except CFID_IS_OPEN - remove unnecessary list lock when parsing response in open_cached_dir() Signed-off-by: Enzo Matsumiya --- fs/smb/client/cached_dir.c | 135 ++++++++++++++++++++------------------------- fs/smb/client/cached_dir.h | 51 +++++++++++++++-- fs/smb/client/cifs_debug.c | 2 +- fs/smb/client/dir.c | 2 +- fs/smb/client/smb2inode.c | 11 +++- 5 files changed, 118 insertions(+), 83 deletions(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index 125558b3b92d..c4f110fc66d5 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -15,23 +15,14 @@ static struct cached_fid *init_cached_dir(const char *path); static void smb2_close_cached_fid(struct kref *ref); -static inline bool cfid_expired(const struct cached_fid *cfid) -{ - return (cfid->last_access_time && - time_is_before_jiffies(cfid->last_access_time + HZ * dir_cache_timeout)); -} - -static inline bool cfid_is_valid(const struct cached_fid *cfid) -{ - return (cfid->has_lease && cfid->time && !cfid_expired(cfid)); -} - static inline void invalidate_cfid(struct cached_fid *cfid, bool force_expire) { - if (cfid_is_valid(cfid)) + /* only drop entries counter if it hasn't been invalidated earlier */ + if (!(cfid->state & CFID_INVALID)) cfid->cfids->num_entries--; - /* do not change other fields here! */ + /* only tag it as invalid -- don't modify state any further here! */ + cfid->state |= CFID_INVALID; cfid->time = 0; if (force_expire) cfid->last_access_time = 1; @@ -110,8 +101,7 @@ path_to_dentry(struct cifs_sb_info *cifs_sb, const char *path) while (*s && *s != sep) s++; - child = lookup_noperm_positive_unlocked(&QSTR_LEN(p, s - p), - dentry); + child = lookup_noperm_positive_unlocked(&QSTR_LEN(p, s - p), dentry); dput(dentry); dentry = child; } while (!IS_ERR(dentry)); @@ -157,7 +147,9 @@ retry_find: bool retry; /* cfid is being opened in open_cached_dir() */ - retry = (cfid->has_lease && !cfid->time && !cfid->last_access_time); + retry = (!(cfid->state & CFID_INVALID) && + (cfid->state & CFID_IS_OPENING) && + !cfid->time && !cfid->last_access_time); kref_put(&cfid->refcount, smb2_close_cached_fid); if (retry) @@ -182,6 +174,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path, struct cifs_open_parms oparms; struct smb2_create_rsp *o_rsp = NULL; struct smb2_query_info_rsp *qi_rsp = NULL; + struct smb2_file_all_info info = {}; int resp_buftype[2]; struct smb_rqst rqst[2]; struct kvec rsp_iov[2]; @@ -197,6 +190,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path, const char *npath; int retries = 0, cur_sleep = 1; __le32 lease_flags = 0; + bool has_info; if (cifs_sb->root == NULL) return -ENOENT; @@ -217,6 +211,7 @@ replay_again: cfid = NULL; *ret_cfid = NULL; server = cifs_pick_channel(ses); + has_info = false; if (!server->ops->new_lease_key) return -EIO; @@ -225,7 +220,7 @@ replay_again: if (!utf16_path) return -ENOMEM; - /* find_cached_dir() already checks has_lease and time, so no need to check here */ + /* find_cached_dir() will return a valid cfid if found, so no need to check here again */ cfid = find_cached_dir(cfids, path, CFID_LOOKUP_PATH); if (cfid) { rc = 0; @@ -243,10 +238,11 @@ replay_again: if (!cfid) { spin_unlock(&cfids->cfid_list_lock); rc = -ENOMEM; - goto out; + goto out_err; } cfid->cfids = cfids; + cfid->tcon = tcon; cfids->num_entries++; list_add(&cfid->entry, &cfids->entries); spin_unlock(&cfids->cfid_list_lock); @@ -263,7 +259,7 @@ replay_again: npath = path_no_prefix(cifs_sb, path); if (IS_ERR(npath)) { rc = PTR_ERR(npath); - goto out; + goto out_err; } if (!npath[0]) { @@ -273,7 +269,7 @@ replay_again: if (IS_ERR(dentry)) { dentry = NULL; rc = -ENOENT; - goto out; + goto out_err; } if (dentry->d_parent && server->dialect >= SMB30_PROT_ID) { struct cached_fid *parent_cfid; @@ -282,7 +278,7 @@ replay_again: list_for_each_entry(parent_cfid, &cfids->entries, entry) { if (parent_cfid->dentry == dentry->d_parent) { cifs_dbg(FYI, "found a parent cached file handle\n"); - if (parent_cfid->has_lease && parent_cfid->time) { + if (cfid_is_valid(parent_cfid)) { lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE; memcpy(pfid->parent_lease_key, @@ -295,9 +291,6 @@ replay_again: spin_unlock(&cfids->cfid_list_lock); } } - cfid->dentry = dentry; - cfid->tcon = tcon; - dentry = NULL; /* * We do not hold the lock for the open because in case @@ -369,9 +362,8 @@ replay_again: } goto oshr_free; } - cfid->is_open = true; - spin_lock(&cfids->cfid_list_lock); + cfid->state |= CFID_IS_OPEN; o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base; oparms.fid->persistent_fid = o_rsp->PersistentFileId; @@ -379,76 +371,78 @@ replay_again: #ifdef CONFIG_CIFS_DEBUG2 oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId); #endif /* CIFS_DEBUG2 */ - - if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) { - spin_unlock(&cfids->cfid_list_lock); rc = -EINVAL; goto oshr_free; } - rc = smb2_parse_contexts(server, rsp_iov, - &oparms.fid->epoch, - oparms.fid->lease_key, + rc = smb2_parse_contexts(server, rsp_iov, &oparms.fid->epoch, oparms.fid->lease_key, &oplock, NULL, NULL); - if (rc) { - spin_unlock(&cfids->cfid_list_lock); + if (rc) goto oshr_free; - } rc = -EINVAL; - if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) { - spin_unlock(&cfids->cfid_list_lock); + if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) goto oshr_free; - } + qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base; - if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) { - spin_unlock(&cfids->cfid_list_lock); + if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) goto oshr_free; - } - if (!smb2_validate_and_copy_iov( - le16_to_cpu(qi_rsp->OutputBufferOffset), - sizeof(struct smb2_file_all_info), - &rsp_iov[1], sizeof(struct smb2_file_all_info), - (char *)&cfid->file_all_info)) - cfid->file_all_info_is_valid = true; - - cfid->time = jiffies; - cfid->last_access_time = jiffies; - spin_unlock(&cfids->cfid_list_lock); - /* At this point the directory handle is fully cached */ - rc = 0; + if (!smb2_validate_and_copy_iov(le16_to_cpu(qi_rsp->OutputBufferOffset), + sizeof(struct smb2_file_all_info), &rsp_iov[1], + sizeof(struct smb2_file_all_info), + (char *)&info)) + has_info = true; + + rc = 0; oshr_free: SMB2_open_free(&rqst[0]); SMB2_query_info_free(&rqst[1]); free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base); free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base); -out: +out_err: /* cfid invalidated in the mean time, drop it below */ - if (!rc && !cfid_is_valid(cfid)) + if (!rc && (cfid->state & CFID_INVALID)) rc = -ENOENT; if (rc) { + dput(dentry); + if (cfid) { spin_lock(&cfids->cfid_list_lock); /* we don't know what caused the invalidation, so don't force cleanup */ invalidate_cfid(cfid, false); + cfid->state &= ~CFID_IS_OPENING; spin_unlock(&cfids->cfid_list_lock); - dput(cfid->dentry); - cfid->dentry = NULL; - kref_put(&cfid->refcount, smb2_close_cached_fid); } + cfid = NULL; } else { - *ret_cfid = cfid; + cfid->state |= CFID_IS_OPEN; + if (has_info) { + cfid->info = kmemdup(&info, sizeof(info), GFP_ATOMIC); + if (!cfid->info) { + cfid->state &= ~CFID_IS_OPENING; + rc = -ENOMEM; + goto out_err; + } + } + cfid->dentry = dentry; + cfid->time = jiffies; + cfid->last_access_time = jiffies; + /* must be the last step -- concurrent lookups are now able to find this cfid */ + cfid->state &= ~CFID_IS_OPENING; + /* At this point the directory handle is fully cached */ + atomic_inc(&tcon->num_remote_opens); } +out: + *ret_cfid = cfid; kfree(utf16_path); - if (is_replayable_error(rc) && - smb2_should_replay(tcon, &retries, &cur_sleep)) + if (is_replayable_error(rc) && smb2_should_replay(tcon, &retries, &cur_sleep)) goto replay_again; return rc; @@ -479,6 +473,8 @@ static void smb2_close_cached_fid(struct kref *ref) kfree(dirent); } + kfree(cfid->info); + cfid->info = NULL; kfree(cfid->path); cfid->path = NULL; kfree(cfid); @@ -527,9 +523,8 @@ static void invalidate_all_cfids(struct cached_fids *cfids, bool closed) spin_lock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &cfids->entries, entry) { invalidate_cfid(cfid, true); - cfid->has_lease = false; if (closed) - cfid->is_open = false; + cfid->state &= ~CFID_IS_OPEN; } spin_unlock(&cfids->cfid_list_lock); @@ -600,15 +595,7 @@ static struct cached_fid *init_cached_dir(const char *path) /* this is caller/lease ref */ kref_get(&cfid->refcount); - /* - * Set @cfid->has_lease to true during construction so that the lease - * reference can be put in cached_dir_lease_break() due to a potential - * lease break right after the request is sent or while @cfid is still - * being cached, or if a reconnection is triggered during construction. - * Concurrent processes won't be to use it yet due to @cfid->time being - * zero. - */ - cfid->has_lease = true; + cfid->state = CFID_IS_OPENING; return cfid; } @@ -649,11 +636,11 @@ static void cfids_laundromat_worker(struct work_struct *work) dput(dentry); - if (cfid->is_open) { + if (cfid->state & CFID_IS_OPEN) { int rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid, cfid->fid.volatile_fid); - cfid->is_open = false; + cfid->state &= ~CFID_IS_OPEN; /* SMB2_close should handle -EBUSY or -EAGAIN */ if (rc) diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h index 6ebb34353144..73e28b04b519 100644 --- a/fs/smb/client/cached_dir.h +++ b/fs/smb/client/cached_dir.h @@ -33,9 +33,7 @@ struct cached_fid { struct list_head entry; struct cached_fids *cfids; const char *path; - bool has_lease:1; - bool is_open:1; - bool file_all_info_is_valid:1; + int state; unsigned long time; /* jiffies of when lease was taken */ unsigned long last_access_time; /* jiffies of when last accessed */ struct kref refcount; @@ -43,7 +41,7 @@ struct cached_fid { spinlock_t fid_lock; struct cifs_tcon *tcon; struct dentry *dentry; - struct smb2_file_all_info file_all_info; + struct smb2_file_all_info *info; struct cached_dirents dirents; }; @@ -66,6 +64,51 @@ enum { CFID_LOOKUP_LEASEKEY, }; +/* + * cfid state flags + * + * CFID_IS_OPENING: cfid was initialized and is being opened in open_cached_dir() + * CFID_IS_OPEN: remote fid opened on the server and we got a lease for it + * CFID_INVALID: cfid must not be used externally (but may be used internally) + * + * Note that CFID_IS_OPEN by itself doesn't determine validity of a cfid, e.g. + * (CFID_IS_OPEN | CFID_INVALID) means the remote handle for this directory is still open on the + * server (and needs closing), but the cfid object must not be used (expired, dropped, etc). + */ +#define CFID_IS_OPENING 0x1 +#define CFID_IS_OPEN 0x2 +#define CFID_INVALID 0x4 + +static inline bool cfid_expired(const struct cached_fid *cfid) +{ + if (!cfid) + return true; + + return (cfid->last_access_time && + time_is_before_jiffies(cfid->last_access_time + dir_cache_timeout * HZ)); +} + +static inline bool cfid_is_valid(const struct cached_fid *cfid) +{ + if (!cfid) + return false; + + /* cfid is about to go down in these cases, so check it first */ + if (cfid->last_access_time == 1) + return false; + + if (cfid->state & (CFID_IS_OPENING | CFID_INVALID)) + return false; + + if (!(cfid->state & CFID_IS_OPEN)) + return false; + + if (!cfid->time || !cfid->last_access_time) + return false; + + return !cfid_expired(cfid); +} + extern struct cached_fids *init_cached_dirs(void); extern struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void *key, int mode); extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path, diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index 2337cf795db3..9850d06caa3b 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -314,7 +314,7 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v) ses->Suid, cfid->fid.persistent_fid, cfid->path); - if (cfid->file_all_info_is_valid) + if (cfid->info) seq_printf(m, "\tvalid file info"); if (cfid->dirents.is_valid) seq_printf(m, ", valid dirents"); diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c index 5223edf6d11a..2c6450e1c97c 100644 --- a/fs/smb/client/dir.c +++ b/fs/smb/client/dir.c @@ -322,7 +322,7 @@ retry_open: list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) { if (parent_cfid->dentry == direntry->d_parent) { cifs_dbg(FYI, "found a parent cached file handle\n"); - if (parent_cfid->has_lease && parent_cfid->time) { + if (cfid_is_valid(parent_cfid)) { lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE; memcpy(fid->parent_lease_key, diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index 62d6adf50ad1..192645a891c8 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -970,14 +970,19 @@ int smb2_query_path_info(const unsigned int xid, /* If it is a root and its handle is cached then use it */ if (!rc) { - if (cfid->file_all_info_is_valid) { - memcpy(&data->fi, &cfid->file_all_info, - sizeof(data->fi)); + if (cfid->info) { + memcpy(&data->fi, cfid->info, sizeof(data->fi)); } else { rc = SMB2_query_info(xid, tcon, cfid->fid.persistent_fid, cfid->fid.volatile_fid, &data->fi); + if (!rc) { + cfid->info = kmemdup(&data->fi, sizeof(data->fi), + GFP_KERNEL); + if (!cfid->info) + rc = -ENOMEM; + } } close_cached_dir(cfid); return rc; -- cgit v1.2.3