From 1add00d886388cfcf9b755500263b97f5d9d57e2 Mon Sep 17 00:00:00 2001 From: Enzo Matsumiya Date: Mon, 8 Sep 2025 15:56:34 -0300 Subject: smb: client: remove cached_fid->on_list cfids are always on some list throughout its lifetime, so we can drop this field. Decrement cfids->num_entries now based on valid -> invalid transition. Other changes: - add helpers cfid_expired() and cfid_is_valid() - check cfid_is_valid() even on success when leaving open_cached_dir() Signed-off-by: Enzo Matsumiya --- fs/smb/client/cached_dir.c | 74 +++++++++++++++++++++++++++------------------- fs/smb/client/cached_dir.h | 1 - 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index af2fe416ddcf..a32d71540bba 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -15,20 +15,29 @@ static struct cached_fid *init_cached_dir(const char *path); static void free_cached_dir(struct cached_fid *cfid); static void smb2_close_cached_fid(struct kref *ref); -static void cfids_laundromat_worker(struct work_struct *work); struct cached_dir_dentry { struct list_head entry; struct dentry *dentry; }; +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->on_list) + if (cfid_is_valid(cfid)) cfid->cfids->num_entries--; /* do not change other fields here! */ - cfid->on_list = false; cfid->time = 0; if (force_expire) cfid->last_access_time = 1; @@ -48,9 +57,9 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids, * fully cached or it may be in the process of * being deleted due to a lease break. */ - if (!cfid->time || !cfid->has_lease) { + if (!cfid_is_valid(cfid)) return NULL; - } + kref_get(&cfid->refcount); return cfid; } @@ -68,7 +77,6 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids, cfid->cfids = cfids; cfids->num_entries++; list_add(&cfid->entry, &cfids->entries); - cfid->on_list = true; kref_get(&cfid->refcount); /* * Set @cfid->has_lease to true during construction so that the lease @@ -235,6 +243,7 @@ replay_again: } else { dentry = path_to_dentry(cifs_sb, npath); if (IS_ERR(dentry)) { + dentry = NULL; rc = -ENOENT; goto out; } @@ -260,6 +269,7 @@ replay_again: } cfid->dentry = dentry; cfid->tcon = tcon; + dentry = NULL; /* * We do not hold the lock for the open because in case @@ -387,24 +397,19 @@ oshr_free: free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base); free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base); out: + /* cfid invalidated in the mean time, drop it below */ + if (!rc && !cfid_is_valid(cfid)) + rc = -ENOENT; + if (rc) { spin_lock(&cfids->cfid_list_lock); - if (cfid->on_list) { - list_del(&cfid->entry); - cfid->on_list = false; - cfids->num_entries--; - } - if (cfid->has_lease) { - /* - * We are guaranteed to have two references at this - * point. One for the caller and one for a potential - * lease. Release one here, and the second below. - */ - cfid->has_lease = false; - kref_put(&cfid->refcount, smb2_close_cached_fid); - } + /* we don't know what caused the invalidation, so don't force cleanup */ + invalidate_cfid(cfid, false); spin_unlock(&cfids->cfid_list_lock); + if (!cfid->dentry) + dput(dentry); + kref_put(&cfid->refcount, smb2_close_cached_fid); } else { *ret_cfid = cfid; @@ -431,7 +436,7 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon, spin_lock(&cfids->cfid_list_lock); list_for_each_entry(cfid, &cfids->entries, entry) { - if (dentry && cfid->dentry == dentry) { + if (cfid_is_valid(cfid) && cfid->dentry == dentry) { cifs_dbg(FYI, "found a cached file handle by dentry\n"); kref_get(&cfid->refcount); *ret_cfid = cfid; @@ -447,14 +452,19 @@ static void smb2_close_cached_fid(struct kref *ref) { struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount); + struct cached_fids *cfids = cfid->cfids; + struct dentry *dentry = NULL; - spin_lock(&cfid->cfids->cfid_list_lock); + spin_lock(&cfids->cfid_list_lock); list_del(&cfid->entry); invalidate_cfid(cfid, true); - spin_unlock(&cfid->cfids->cfid_list_lock); - - dput(cfid->dentry); + spin_lock(&cfid->fid_lock); + dentry = cfid->dentry; cfid->dentry = NULL; + spin_unlock(&cfid->fid_lock); + spin_unlock(&cfids->cfid_list_lock); + + dput(dentry); free_cached_dir(cfid); } @@ -552,14 +562,14 @@ done: void invalidate_all_cached_dirs(struct cifs_tcon *tcon) { struct cached_fids *cfids = tcon->cfids; - struct cached_fid *cfid, *q; + struct cached_fid *cfid; if (!cfids) return; /* Mark all the cfids as closed, and invalidate them for laundromat cleanup. */ spin_lock(&cfids->cfid_list_lock); - list_for_each_entry_safe(cfid, q, &cfids->entries, entry) { + list_for_each_entry(cfid, &cfids->entries, entry) { invalidate_cfid(cfid, true); cfid->is_open = false; } @@ -628,9 +638,14 @@ static struct cached_fid *init_cached_dir(const char *path) static void free_cached_dir(struct cached_fid *cfid) { struct cached_dirent *dirent, *q; + struct dentry *dentry; - dput(cfid->dentry); + spin_lock(&cfid->fid_lock); + dentry = cfid->dentry; cfid->dentry = NULL; + spin_unlock(&cfid->fid_lock); + + dput(dentry); /* * Delete all cached dirent names @@ -657,8 +672,7 @@ static void cfids_laundromat_worker(struct work_struct *work) spin_lock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &cfids->entries, entry) { - if (cfid->last_access_time && - time_after(jiffies, cfid->last_access_time + HZ * dir_cache_timeout)) { + if (cfid_expired(cfid)) { invalidate_cfid(cfid, true); list_move(&cfid->entry, &entry); } diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h index e549f019b923..feca9a361635 100644 --- a/fs/smb/client/cached_dir.h +++ b/fs/smb/client/cached_dir.h @@ -35,7 +35,6 @@ struct cached_fid { const char *path; bool has_lease:1; bool is_open:1; - bool on_list:1; bool file_all_info_is_valid:1; unsigned long time; /* jiffies of when lease was taken */ unsigned long last_access_time; /* jiffies of when last accessed */ -- cgit v1.2.3