* [PATCH 01/20] smb: client: remove cfids_invalidation_worker
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 02/20] smb: client: remove cached_dir_offload_close/close_work Enzo Matsumiya
` (19 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
We can do the same cleanup on laundromat.
On invalidate_all_cached_dirs(), run laundromat worker with 0 timeout
and flush it for immediate + sync cleanup.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 39 ++++++++++----------------------------
fs/smb/client/cached_dir.h | 1 -
2 files changed, 10 insertions(+), 30 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index b69daeb1301b..f61fef810a23 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -553,13 +553,13 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
struct cached_fids *cfids = tcon->cfids;
struct cached_fid *cfid, *q;
- if (cfids == NULL)
+ if (!cfids)
return;
/*
* Mark all the cfids as closed, and move them to the cfids->dying list.
- * They'll be cleaned up later by cfids_invalidation_worker. Take
- * a reference to each cfid during this process.
+ * They'll be cleaned up by laundromat. Take a reference to each cfid
+ * during this process.
*/
spin_lock(&cfids->cfid_list_lock);
list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
@@ -576,12 +576,11 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
} else
kref_get(&cfid->refcount);
}
- /*
- * Queue dropping of the dentries once locks have been dropped
- */
- if (!list_empty(&cfids->dying))
- queue_work(cfid_put_wq, &cfids->invalidation_work);
spin_unlock(&cfids->cfid_list_lock);
+
+ /* run laundromat unconditionally now as there might have been previously queued work */
+ mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
+ flush_delayed_work(&cfids->laundromat_work);
}
static void
@@ -702,25 +701,6 @@ static void free_cached_dir(struct cached_fid *cfid)
kfree(cfid);
}
-static void cfids_invalidation_worker(struct work_struct *work)
-{
- struct cached_fids *cfids = container_of(work, struct cached_fids,
- invalidation_work);
- struct cached_fid *cfid, *q;
- LIST_HEAD(entry);
-
- spin_lock(&cfids->cfid_list_lock);
- /* move cfids->dying to the local list */
- list_cut_before(&entry, &cfids->dying, &cfids->dying);
- spin_unlock(&cfids->cfid_list_lock);
-
- list_for_each_entry_safe(cfid, q, &entry, entry) {
- list_del(&cfid->entry);
- /* Drop the ref-count acquired in invalidate_all_cached_dirs */
- kref_put(&cfid->refcount, smb2_close_cached_fid);
- }
-}
-
static void cfids_laundromat_worker(struct work_struct *work)
{
struct cached_fids *cfids;
@@ -731,6 +711,9 @@ static void cfids_laundromat_worker(struct work_struct *work)
cfids = container_of(work, struct cached_fids, laundromat_work.work);
spin_lock(&cfids->cfid_list_lock);
+ /* move cfids->dying to the local list */
+ list_cut_before(&entry, &cfids->dying, &cfids->dying);
+
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)) {
@@ -787,7 +770,6 @@ struct cached_fids *init_cached_dirs(void)
INIT_LIST_HEAD(&cfids->entries);
INIT_LIST_HEAD(&cfids->dying);
- INIT_WORK(&cfids->invalidation_work, cfids_invalidation_worker);
INIT_DELAYED_WORK(&cfids->laundromat_work, cfids_laundromat_worker);
queue_delayed_work(cfid_put_wq, &cfids->laundromat_work,
dir_cache_timeout * HZ);
@@ -808,7 +790,6 @@ void free_cached_dirs(struct cached_fids *cfids)
return;
cancel_delayed_work_sync(&cfids->laundromat_work);
- cancel_work_sync(&cfids->invalidation_work);
spin_lock(&cfids->cfid_list_lock);
list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 46b5a2fdf15b..a3757a736d3e 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -60,7 +60,6 @@ struct cached_fids {
int num_entries;
struct list_head entries;
struct list_head dying;
- struct work_struct invalidation_work;
struct delayed_work laundromat_work;
};
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 02/20] smb: client: remove cached_dir_offload_close/close_work
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 01/20] smb: client: remove cfids_invalidation_worker Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 03/20] smb: client: remove cached_dir_put_work/put_work Enzo Matsumiya
` (18 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Make put_work an 'async dput' and then move cfid to dying list so
laundromat can cleanup the rest.
Other changes:
- add drop_cfid() helper to drop entries counter, dput and close
synchronously, when possible
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 142 +++++++++++++++++--------------------
fs/smb/client/cached_dir.h | 1 -
2 files changed, 66 insertions(+), 77 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index f61fef810a23..8689ee4a883d 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -22,6 +22,36 @@ struct cached_dir_dentry {
struct dentry *dentry;
};
+static inline void drop_cfid(struct cached_fid *cfid)
+{
+ struct dentry *dentry = NULL;
+
+ spin_lock(&cfid->cfids->cfid_list_lock);
+ if (cfid->on_list) {
+ list_del(&cfid->entry);
+ cfid->on_list = false;
+ cfid->cfids->num_entries--;
+ }
+
+ spin_lock(&cfid->fid_lock);
+ swap(cfid->dentry, dentry);
+ spin_unlock(&cfid->fid_lock);
+ spin_unlock(&cfid->cfids->cfid_list_lock);
+
+ dput(dentry);
+
+ if (cfid->is_open) {
+ int rc;
+
+ cfid->is_open = false;
+ rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid, cfid->fid.volatile_fid);
+
+ /* SMB2_close should handle -EBUSY or -EAGAIN */
+ if (rc)
+ cifs_dbg(VFS, "close cached dir rc %d\n", rc);
+ }
+}
+
static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
const char *path,
bool lookup_only,
@@ -434,28 +464,9 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
static void
smb2_close_cached_fid(struct kref *ref)
{
- struct cached_fid *cfid = container_of(ref, struct cached_fid,
- refcount);
- int rc;
-
- spin_lock(&cfid->cfids->cfid_list_lock);
- if (cfid->on_list) {
- list_del(&cfid->entry);
- cfid->on_list = false;
- cfid->cfids->num_entries--;
- }
- spin_unlock(&cfid->cfids->cfid_list_lock);
-
- dput(cfid->dentry);
- cfid->dentry = NULL;
-
- if (cfid->is_open) {
- rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
- cfid->fid.volatile_fid);
- if (rc) /* should we retry on -EBUSY or -EAGAIN? */
- cifs_dbg(VFS, "close cached dir rc %d\n", rc);
- }
+ struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+ drop_cfid(cfid);
free_cached_dir(cfid);
}
@@ -530,6 +541,9 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
list_add_tail(&tmp_list->entry, &entry);
}
spin_unlock(&cfids->cfid_list_lock);
+
+ /* run laundromat now as it might not have been queued */
+ mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
}
spin_unlock(&cifs_sb->tlink_tree_lock);
@@ -583,30 +597,15 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
flush_delayed_work(&cfids->laundromat_work);
}
-static void
-cached_dir_offload_close(struct work_struct *work)
-{
- struct cached_fid *cfid = container_of(work,
- struct cached_fid, close_work);
- struct cifs_tcon *tcon = cfid->tcon;
-
- WARN_ON(cfid->on_list);
-
- kref_put(&cfid->refcount, smb2_close_cached_fid);
- cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cached_close);
-}
-
/*
- * Release the cached directory's dentry, and then queue work to drop cached
- * directory itself (closing on server if needed).
- *
- * Must be called with a reference to the cached_fid and a reference to the
- * tcon.
+ * Release the cached directory's dentry and schedule immediate cleanup on laundromat.
+ * Must be called with a reference to the cached_fid and a reference to the tcon.
*/
static void cached_dir_put_work(struct work_struct *work)
{
- struct cached_fid *cfid = container_of(work, struct cached_fid,
- put_work);
+ struct cached_fid *cfid = container_of(work, struct cached_fid, put_work);
+ struct cached_fids *cfids = cfid->cfids;
+ struct cifs_tcon *tcon = cfid->tcon;
struct dentry *dentry;
spin_lock(&cfid->fid_lock);
@@ -615,7 +614,16 @@ static void cached_dir_put_work(struct work_struct *work)
spin_unlock(&cfid->fid_lock);
dput(dentry);
- queue_work(serverclose_wq, &cfid->close_work);
+
+ /* move to dying list so laundromat can clean it up */
+ spin_lock(&cfids->cfid_list_lock);
+ list_move(&cfid->entry, &cfids->dying);
+ cfid->on_list = false;
+ cfids->num_entries--;
+ spin_unlock(&cfids->cfid_list_lock);
+
+ cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cached_close);
+ mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
}
bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
@@ -634,13 +642,6 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
SMB2_LEASE_KEY_SIZE)) {
cfid->has_lease = false;
cfid->time = 0;
- /*
- * We found a lease remove it from the list
- * so no threads can access it.
- */
- list_del(&cfid->entry);
- cfid->on_list = false;
- cfids->num_entries--;
++tcon->tc_count;
trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count,
@@ -667,7 +668,6 @@ static struct cached_fid *init_cached_dir(const char *path)
return NULL;
}
- INIT_WORK(&cfid->close_work, cached_dir_offload_close);
INIT_WORK(&cfid->put_work, cached_dir_put_work);
INIT_LIST_HEAD(&cfid->entry);
INIT_LIST_HEAD(&cfid->dirents.entries);
@@ -681,12 +681,8 @@ static void free_cached_dir(struct cached_fid *cfid)
{
struct cached_dirent *dirent, *q;
- WARN_ON(work_pending(&cfid->close_work));
WARN_ON(work_pending(&cfid->put_work));
- dput(cfid->dentry);
- cfid->dentry = NULL;
-
/*
* Delete all cached dirent names
*/
@@ -705,7 +701,6 @@ static void cfids_laundromat_worker(struct work_struct *work)
{
struct cached_fids *cfids;
struct cached_fid *cfid, *q;
- struct dentry *dentry;
LIST_HEAD(entry);
cfids = container_of(work, struct cached_fids, laundromat_work.work);
@@ -735,28 +730,22 @@ static void cfids_laundromat_worker(struct work_struct *work)
list_for_each_entry_safe(cfid, q, &entry, entry) {
list_del(&cfid->entry);
- spin_lock(&cfid->fid_lock);
- dentry = cfid->dentry;
- cfid->dentry = NULL;
- spin_unlock(&cfid->fid_lock);
-
- dput(dentry);
- if (cfid->is_open) {
- spin_lock(&cifs_tcp_ses_lock);
- ++cfid->tcon->tc_count;
- trace_smb3_tcon_ref(cfid->tcon->debug_id, cfid->tcon->tc_count,
- netfs_trace_tcon_ref_get_cached_laundromat);
- spin_unlock(&cifs_tcp_ses_lock);
- queue_work(serverclose_wq, &cfid->close_work);
- } else
- /*
- * Drop the ref-count from above, either the lease-ref (if there
- * was one) or the extra one acquired.
- */
- kref_put(&cfid->refcount, smb2_close_cached_fid);
+ /*
+ * If a cfid reached here, we must cleanup anything unrelated to it, i.e. dentry and
+ * remote fid.
+ *
+ * For the cfid itself, we only drop our own ref (kref_init). If there are still
+ * concurrent ref-holders, they'll drop it later (cfid is already invalid at this
+ * point, so can't be found anymore).
+ *
+ * No risk for a double list_del() here because cfid->on_list is always false at
+ * this point.
+ */
+ drop_cfid(cfid);
+ kref_put(&cfid->refcount, smb2_close_cached_fid);
}
- queue_delayed_work(cfid_put_wq, &cfids->laundromat_work,
- dir_cache_timeout * HZ);
+
+ queue_delayed_work(cfid_put_wq, &cfids->laundromat_work, dir_cache_timeout * HZ);
}
struct cached_fids *init_cached_dirs(void)
@@ -806,6 +795,7 @@ void free_cached_dirs(struct cached_fids *cfids)
list_for_each_entry_safe(cfid, q, &entry, entry) {
list_del(&cfid->entry);
+ drop_cfid(cfid);
free_cached_dir(cfid);
}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index a3757a736d3e..e5445e3a7bd3 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -45,7 +45,6 @@ struct cached_fid {
struct cifs_tcon *tcon;
struct dentry *dentry;
struct work_struct put_work;
- struct work_struct close_work;
struct smb2_file_all_info file_all_info;
struct cached_dirents dirents;
};
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 03/20] smb: client: remove cached_dir_put_work/put_work
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 01/20] smb: client: remove cfids_invalidation_worker Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 02/20] smb: client: remove cached_dir_offload_close/close_work Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 04/20] smb: client: remove cached_fids->dying list Enzo Matsumiya
` (17 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Move cfid to dying list directly on cached_dir_lease_break(), and
schedule laundromat for cleanup there too.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 56 ++++++++++++--------------------------
fs/smb/client/cached_dir.h | 1 -
2 files changed, 17 insertions(+), 40 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 8689ee4a883d..ab37a025ea1c 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -597,39 +597,11 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
flush_delayed_work(&cfids->laundromat_work);
}
-/*
- * Release the cached directory's dentry and schedule immediate cleanup on laundromat.
- * Must be called with a reference to the cached_fid and a reference to the tcon.
- */
-static void cached_dir_put_work(struct work_struct *work)
-{
- struct cached_fid *cfid = container_of(work, struct cached_fid, put_work);
- struct cached_fids *cfids = cfid->cfids;
- struct cifs_tcon *tcon = cfid->tcon;
- struct dentry *dentry;
-
- spin_lock(&cfid->fid_lock);
- dentry = cfid->dentry;
- cfid->dentry = NULL;
- spin_unlock(&cfid->fid_lock);
-
- dput(dentry);
-
- /* move to dying list so laundromat can clean it up */
- spin_lock(&cfids->cfid_list_lock);
- list_move(&cfid->entry, &cfids->dying);
- cfid->on_list = false;
- cfids->num_entries--;
- spin_unlock(&cfids->cfid_list_lock);
-
- cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cached_close);
- mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
-}
-
bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
{
struct cached_fids *cfids = tcon->cfids;
struct cached_fid *cfid;
+ bool found = false;
if (cfids == NULL)
return false;
@@ -643,16 +615,25 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
cfid->has_lease = false;
cfid->time = 0;
- ++tcon->tc_count;
- trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count,
- netfs_trace_tcon_ref_get_cached_lease_break);
- queue_work(cfid_put_wq, &cfid->put_work);
- spin_unlock(&cfids->cfid_list_lock);
- return true;
+ /*
+ * We found a lease, move it to the dying list and schedule immediate
+ * cleanup on laundromat.
+ * No need to take a ref here, as we still hold our initial one.
+ */
+ list_move(&cfid->entry, &cfids->dying);
+ cfids->num_entries--;
+ cfid->on_list = false;
+ found = true;
+ break;
}
}
spin_unlock(&cfids->cfid_list_lock);
- return false;
+
+ /* avoid unnecessary scheduling */
+ if (found)
+ mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
+
+ return found;
}
static struct cached_fid *init_cached_dir(const char *path)
@@ -668,7 +649,6 @@ static struct cached_fid *init_cached_dir(const char *path)
return NULL;
}
- INIT_WORK(&cfid->put_work, cached_dir_put_work);
INIT_LIST_HEAD(&cfid->entry);
INIT_LIST_HEAD(&cfid->dirents.entries);
mutex_init(&cfid->dirents.de_mutex);
@@ -681,8 +661,6 @@ static void free_cached_dir(struct cached_fid *cfid)
{
struct cached_dirent *dirent, *q;
- WARN_ON(work_pending(&cfid->put_work));
-
/*
* Delete all cached dirent names
*/
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index e5445e3a7bd3..5e892d53a67a 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -44,7 +44,6 @@ struct cached_fid {
spinlock_t fid_lock;
struct cifs_tcon *tcon;
struct dentry *dentry;
- struct work_struct put_work;
struct smb2_file_all_info file_all_info;
struct cached_dirents dirents;
};
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 04/20] smb: client: remove cached_fids->dying list
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (2 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 03/20] smb: client: remove cached_dir_put_work/put_work Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 05/20] smb: client: remove cached_fid->on_list Enzo Matsumiya
` (16 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Since any cleanup is now done on the local list in laundromat, the
dying list can be removed.
- entries stays on the main list until they're scheduled for cleanup
(->last_access_time == 1)
- cached_fids->num_entries is decremented only when cfid transitions
from on_list true -> false
cached_fid lifecycle on the list becomes:
- list_add() on find_or_create_cached_dir()
- list_move() to local list on laundromat
- list_del() on release callback, if on_list == true (unlikely, see
comment in the function)
Other changes:
- add invalidate_cfid() helper
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 104 +++++++++++++++++--------------------
fs/smb/client/cached_dir.h | 3 +-
2 files changed, 48 insertions(+), 59 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index ab37a025ea1c..2fea7f6c5678 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -22,16 +22,26 @@ struct cached_dir_dentry {
struct dentry *dentry;
};
+static inline void invalidate_cfid(struct cached_fid *cfid)
+{
+ /* callers must hold the list lock and do any list operations (del/move) themselves */
+ lockdep_assert_held(&cfid->cfids->cfid_list_lock);
+
+ if (cfid->on_list)
+ cfid->cfids->num_entries--;
+
+ /* do not change other fields here! */
+ cfid->on_list = false;
+ cfid->time = 0;
+ cfid->last_access_time = 1;
+}
+
static inline void drop_cfid(struct cached_fid *cfid)
{
struct dentry *dentry = NULL;
spin_lock(&cfid->cfids->cfid_list_lock);
- if (cfid->on_list) {
- list_del(&cfid->entry);
- cfid->on_list = false;
- cfid->cfids->num_entries--;
- }
+ invalidate_cfid(cfid);
spin_lock(&cfid->fid_lock);
swap(cfid->dentry, dentry);
@@ -466,6 +476,25 @@ smb2_close_cached_fid(struct kref *ref)
{
struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+ /*
+ * There's no way a cfid can reach here with ->on_list == true.
+ *
+ * This is because we hould our own ref, and whenever we put it, we invalidate the cfid
+ * (which sets ->on_list to false).
+ *
+ * So even if an external caller puts the last ref, ->on_list will already have been set to
+ * false by then by one of the invalidations that can happen concurrently, e.g. lease break,
+ * invalidate_all_cached_dirs().
+ *
+ * So this check is mostly for precaution, but since we can still take the correct actions
+ * if it's the case, do so.
+ */
+ if (WARN_ON(cfid->on_list)) {
+ list_del(&cfid->entry);
+ cfid->on_list = false;
+ cfid->cfids->num_entries--;
+ }
+
drop_cfid(cfid);
free_cached_dir(cfid);
}
@@ -481,15 +510,10 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
cifs_dbg(FYI, "no cached dir found for rmdir(%s)\n", name);
return;
}
- spin_lock(&cfid->cfids->cfid_list_lock);
- if (cfid->has_lease) {
- cfid->has_lease = false;
- kref_put(&cfid->refcount, smb2_close_cached_fid);
- }
- spin_unlock(&cfid->cfids->cfid_list_lock);
- close_cached_dir(cfid);
-}
+ drop_cfid(cfid);
+ kref_put(&cfid->refcount, smb2_close_cached_fid);
+}
void close_cached_dir(struct cached_fid *cfid)
{
@@ -521,6 +545,8 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
continue;
spin_lock(&cfids->cfid_list_lock);
list_for_each_entry(cfid, &cfids->entries, entry) {
+ invalidate_cfid(cfid);
+
tmp_list = kmalloc(sizeof(*tmp_list), GFP_ATOMIC);
if (tmp_list == NULL) {
/*
@@ -570,25 +596,11 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
if (!cfids)
return;
- /*
- * Mark all the cfids as closed, and move them to the cfids->dying list.
- * They'll be cleaned up by laundromat. Take a reference to each cfid
- * during this process.
- */
+ /* 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_move(&cfid->entry, &cfids->dying);
- cfids->num_entries--;
+ invalidate_cfid(cfid);
cfid->is_open = false;
- cfid->on_list = false;
- if (cfid->has_lease) {
- /*
- * The lease was never cancelled from the server,
- * so steal that reference.
- */
- cfid->has_lease = false;
- } else
- kref_get(&cfid->refcount);
}
spin_unlock(&cfids->cfid_list_lock);
@@ -612,17 +624,13 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
!memcmp(lease_key,
cfid->fid.lease_key,
SMB2_LEASE_KEY_SIZE)) {
- cfid->has_lease = false;
- cfid->time = 0;
-
/*
- * We found a lease, move it to the dying list and schedule immediate
- * cleanup on laundromat.
+ * We found a lease, invalidate cfid and schedule immediate cleanup on
+ * laundromat.
* No need to take a ref here, as we still hold our initial one.
*/
- list_move(&cfid->entry, &cfids->dying);
- cfids->num_entries--;
- cfid->on_list = false;
+ invalidate_cfid(cfid);
+ cfid->has_lease = false;
found = true;
break;
}
@@ -684,23 +692,11 @@ static void cfids_laundromat_worker(struct work_struct *work)
cfids = container_of(work, struct cached_fids, laundromat_work.work);
spin_lock(&cfids->cfid_list_lock);
- /* move cfids->dying to the local list */
- list_cut_before(&entry, &cfids->dying, &cfids->dying);
-
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)) {
- cfid->on_list = false;
+ invalidate_cfid(cfid);
list_move(&cfid->entry, &entry);
- cfids->num_entries--;
- if (cfid->has_lease) {
- /*
- * Our lease has not yet been cancelled from the
- * server. Steal that reference.
- */
- cfid->has_lease = false;
- } else
- kref_get(&cfid->refcount);
}
}
spin_unlock(&cfids->cfid_list_lock);
@@ -735,7 +731,6 @@ struct cached_fids *init_cached_dirs(void)
return NULL;
spin_lock_init(&cfids->cfid_list_lock);
INIT_LIST_HEAD(&cfids->entries);
- INIT_LIST_HEAD(&cfids->dying);
INIT_DELAYED_WORK(&cfids->laundromat_work, cfids_laundromat_worker);
queue_delayed_work(cfid_put_wq, &cfids->laundromat_work,
@@ -760,12 +755,7 @@ void free_cached_dirs(struct cached_fids *cfids)
spin_lock(&cfids->cfid_list_lock);
list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
- cfid->on_list = false;
- cfid->is_open = false;
- list_move(&cfid->entry, &entry);
- }
- list_for_each_entry_safe(cfid, q, &cfids->dying, entry) {
- cfid->on_list = false;
+ invalidate_cfid(cfid);
cfid->is_open = false;
list_move(&cfid->entry, &entry);
}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 5e892d53a67a..e549f019b923 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -52,12 +52,11 @@ struct cached_fid {
struct cached_fids {
/* Must be held when:
* - accessing the cfids->entries list
- * - accessing the cfids->dying list
+ * - accessing cfids->num_entries
*/
spinlock_t cfid_list_lock;
int num_entries;
struct list_head entries;
- struct list_head dying;
struct delayed_work laundromat_work;
};
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 05/20] smb: client: remove cached_fid->on_list
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (3 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 04/20] smb: client: remove cached_fids->dying list Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 06/20] smb: client: merge {close,invalidate}_all_cached_dirs() Enzo Matsumiya
` (15 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
That field is currently used to indicate when a cfid should be removed
from the entries list.
Since we now keep cfids on the list until they're going down, we can
remove the field and use the other existing fields for the same effect.
Other changes:
- cfids->num_entries follows semantics of list_*() ops
- add cfid_expired() and cfid_is_valid() helpers
- check cfid_is_valid() even on success when leaving open_cached_dir()
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 77 ++++++++++++++------------------------
fs/smb/client/cached_dir.h | 12 +++++-
fs/smb/client/dir.c | 15 ++++----
3 files changed, 47 insertions(+), 57 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 2fea7f6c5678..9b4045a57f12 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -15,7 +15,6 @@
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;
@@ -27,11 +26,10 @@ static inline void invalidate_cfid(struct cached_fid *cfid)
/* callers must hold the list lock and do any list operations (del/move) themselves */
lockdep_assert_held(&cfid->cfids->cfid_list_lock);
- 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;
cfid->last_access_time = 1;
}
@@ -76,9 +74,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;
}
@@ -96,7 +94,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
@@ -263,6 +260,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
} else {
dentry = path_to_dentry(cifs_sb, npath);
if (IS_ERR(dentry)) {
+ dentry = NULL;
rc = -ENOENT;
goto out;
}
@@ -272,14 +270,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
spin_lock(&cfids->cfid_list_lock);
list_for_each_entry(parent_cfid, &cfids->entries, entry) {
if (parent_cfid->dentry == dentry->d_parent) {
+ if (!cfid_is_valid(parent_cfid))
+ break;
+
cifs_dbg(FYI, "found a parent cached file handle\n");
- if (parent_cfid->has_lease && parent_cfid->time) {
- lease_flags
- |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
- memcpy(pfid->parent_lease_key,
- parent_cfid->fid.lease_key,
- SMB2_LEASE_KEY_SIZE);
- }
+ lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
+ memcpy(pfid->parent_lease_key, parent_cfid->fid.lease_key,
+ SMB2_LEASE_KEY_SIZE);
break;
}
}
@@ -288,6 +285,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
}
cfid->dentry = dentry;
cfid->tcon = tcon;
+ dentry = NULL;
/*
* We do not hold the lock for the open because in case
@@ -415,24 +413,12 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
out:
- 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);
- }
- spin_unlock(&cfids->cfid_list_lock);
+ /* cfid invalidated in the mean time, drop it below */
+ if (!rc && !cfid_is_valid(cfid))
+ rc = -ENOENT;
+ if (rc) {
+ drop_cfid(cfid);
kref_put(&cfid->refcount, smb2_close_cached_fid);
} else {
*ret_cfid = cfid;
@@ -459,7 +445,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;
@@ -477,23 +463,20 @@ smb2_close_cached_fid(struct kref *ref)
struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
/*
- * There's no way a cfid can reach here with ->on_list == true.
+ * There's no way a valid cfid can reach here.
*
- * This is because we hould our own ref, and whenever we put it, we invalidate the cfid
- * (which sets ->on_list to false).
+ * This is because we hould our own ref, and whenever we put it, we invalidate the cfid.
*
- * So even if an external caller puts the last ref, ->on_list will already have been set to
- * false by then by one of the invalidations that can happen concurrently, e.g. lease break,
+ * So even if an external caller puts the last ref, cfid will already have been invalidated
+ * by then by one of the invalidations that can happen concurrently, e.g. lease break,
* invalidate_all_cached_dirs().
*
- * So this check is mostly for precaution, but since we can still take the correct actions
- * if it's the case, do so.
+ * So this check is mostly for precaution, but since we can still take the correct action
+ * (just list_del()) if it's the case, do so.
*/
- if (WARN_ON(cfid->on_list)) {
+ if (WARN_ON(cfid_is_valid(cfid)))
+ /* remaining invalidation done by drop_cfid() below */
list_del(&cfid->entry);
- cfid->on_list = false;
- cfid->cfids->num_entries--;
- }
drop_cfid(cfid);
free_cached_dir(cfid);
@@ -591,14 +574,14 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
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);
cfid->is_open = false;
}
@@ -693,8 +676,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);
list_move(&cfid->entry, &entry);
}
@@ -712,8 +694,7 @@ static void cfids_laundromat_worker(struct work_struct *work)
* concurrent ref-holders, they'll drop it later (cfid is already invalid at this
* point, so can't be found anymore).
*
- * No risk for a double list_del() here because cfid->on_list is always false at
- * this point.
+ * No risk for a double list_del() here because cfid is only on this list now.
*/
drop_cfid(cfid);
kref_put(&cfid->refcount, smb2_close_cached_fid);
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index e549f019b923..92e95c56fd1c 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 */
@@ -60,6 +59,17 @@ struct cached_fids {
struct delayed_work laundromat_work;
};
+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));
+}
+
extern struct cached_fids *init_cached_dirs(void);
extern void free_cached_dirs(struct cached_fids *cfids);
extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 5223edf6d11a..e5372c2c799d 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -321,15 +321,14 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
spin_lock(&tcon->cfids->cfid_list_lock);
list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
if (parent_cfid->dentry == direntry->d_parent) {
+ if (!cfid_is_valid(parent_cfid))
+ break;
+
cifs_dbg(FYI, "found a parent cached file handle\n");
- if (parent_cfid->has_lease && parent_cfid->time) {
- lease_flags
- |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
- memcpy(fid->parent_lease_key,
- parent_cfid->fid.lease_key,
- SMB2_LEASE_KEY_SIZE);
- parent_cfid->dirents.is_valid = false;
- }
+ lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
+ memcpy(fid->parent_lease_key, parent_cfid->fid.lease_key,
+ SMB2_LEASE_KEY_SIZE);
+ parent_cfid->dirents.is_valid = false;
break;
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 06/20] smb: client: merge {close,invalidate}_all_cached_dirs()
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (4 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 05/20] smb: client: remove cached_fid->on_list Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 07/20] smb: client: merge free_cached_dir in release callback Enzo Matsumiya
` (14 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
close_all_cached_dirs(), invalidate_all_cached_dirs() and
free_cached_dirs() have become too similar now, merge their
functionality in a single static invalidate_all_cfids() function.
This also allows removing free_cached_dirs() altogether as it only
requires cancelling the work afterwards (done directly in
tconInfoFree()).
Other changes:
- remove struct cached_dir_dentry
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 114 ++++++++-----------------------------
fs/smb/client/cached_dir.h | 4 +-
fs/smb/client/file.c | 2 +-
fs/smb/client/misc.c | 9 ++-
fs/smb/client/smb2pdu.c | 2 +-
5 files changed, 35 insertions(+), 96 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 9b4045a57f12..36a1e1436502 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -16,11 +16,6 @@ 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);
-struct cached_dir_dentry {
- struct list_head entry;
- struct dentry *dentry;
-};
-
static inline void invalidate_cfid(struct cached_fid *cfid)
{
/* callers must hold the list lock and do any list operations (del/move) themselves */
@@ -503,6 +498,27 @@ void close_cached_dir(struct cached_fid *cfid)
kref_put(&cfid->refcount, smb2_close_cached_fid);
}
+static void invalidate_all_cfids(struct cached_fids *cfids, bool closed)
+{
+ struct cached_fid *cfid, *q;
+
+ 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) {
+ invalidate_cfid(cfid);
+ cfid->has_lease = false;
+ if (closed)
+ cfid->is_open = false;
+ }
+ spin_unlock(&cfids->cfid_list_lock);
+
+ /* run laundromat unconditionally now as there might have been previously queued work */
+ mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
+}
+
/*
* Called from cifs_kill_sb when we unmount a share
*/
@@ -510,12 +526,8 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
{
struct rb_root *root = &cifs_sb->tlink_tree;
struct rb_node *node;
- struct cached_fid *cfid;
struct cifs_tcon *tcon;
struct tcon_link *tlink;
- struct cached_fids *cfids;
- struct cached_dir_dentry *tmp_list, *q;
- LIST_HEAD(entry);
spin_lock(&cifs_sb->tlink_tree_lock);
for (node = rb_first(root); node; node = rb_next(node)) {
@@ -523,46 +535,11 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
tcon = tlink_tcon(tlink);
if (IS_ERR(tcon))
continue;
- cfids = tcon->cfids;
- if (cfids == NULL)
- continue;
- spin_lock(&cfids->cfid_list_lock);
- list_for_each_entry(cfid, &cfids->entries, entry) {
- invalidate_cfid(cfid);
-
- tmp_list = kmalloc(sizeof(*tmp_list), GFP_ATOMIC);
- if (tmp_list == NULL) {
- /*
- * If the malloc() fails, we won't drop all
- * dentries, and unmounting is likely to trigger
- * a 'Dentry still in use' error.
- */
- cifs_tcon_dbg(VFS, "Out of memory while dropping dentries\n");
- spin_unlock(&cfids->cfid_list_lock);
- spin_unlock(&cifs_sb->tlink_tree_lock);
- goto done;
- }
- spin_lock(&cfid->fid_lock);
- tmp_list->dentry = cfid->dentry;
- cfid->dentry = NULL;
- spin_unlock(&cfid->fid_lock);
- list_add_tail(&tmp_list->entry, &entry);
- }
- spin_unlock(&cfids->cfid_list_lock);
-
- /* run laundromat now as it might not have been queued */
- mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
+ invalidate_all_cfids(tcon->cfids, false);
}
spin_unlock(&cifs_sb->tlink_tree_lock);
-done:
- list_for_each_entry_safe(tmp_list, q, &entry, entry) {
- list_del(&tmp_list->entry);
- dput(tmp_list->dentry);
- kfree(tmp_list);
- }
-
/* Flush any pending work that will drop dentries */
flush_workqueue(cfid_put_wq);
}
@@ -571,24 +548,12 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
* Invalidate all cached dirs when a TCON has been reset
* due to a session loss.
*/
-void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
+void invalidate_all_cached_dirs(struct cached_fids *cfids)
{
- struct cached_fids *cfids = tcon->cfids;
- 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(cfid, &cfids->entries, entry) {
- invalidate_cfid(cfid);
- cfid->is_open = false;
- }
- spin_unlock(&cfids->cfid_list_lock);
-
- /* run laundromat unconditionally now as there might have been previously queued work */
- mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
+ invalidate_all_cfids(cfids, true);
flush_delayed_work(&cfids->laundromat_work);
}
@@ -719,34 +684,3 @@ struct cached_fids *init_cached_dirs(void)
return cfids;
}
-
-/*
- * Called from tconInfoFree when we are tearing down the tcon.
- * There are no active users or open files/directories at this point.
- */
-void free_cached_dirs(struct cached_fids *cfids)
-{
- struct cached_fid *cfid, *q;
- LIST_HEAD(entry);
-
- if (cfids == NULL)
- return;
-
- cancel_delayed_work_sync(&cfids->laundromat_work);
-
- spin_lock(&cfids->cfid_list_lock);
- list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
- invalidate_cfid(cfid);
- cfid->is_open = false;
- list_move(&cfid->entry, &entry);
- }
- spin_unlock(&cfids->cfid_list_lock);
-
- list_for_each_entry_safe(cfid, q, &entry, entry) {
- list_del(&cfid->entry);
- drop_cfid(cfid);
- free_cached_dir(cfid);
- }
-
- kfree(cfids);
-}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 92e95c56fd1c..47c0404ba84a 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -71,7 +71,6 @@ static inline bool cfid_is_valid(const struct cached_fid *cfid)
}
extern struct cached_fids *init_cached_dirs(void);
-extern void free_cached_dirs(struct cached_fids *cfids);
extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
const char *path,
struct cifs_sb_info *cifs_sb,
@@ -85,7 +84,6 @@ extern void drop_cached_dir_by_name(const unsigned int xid,
const char *name,
struct cifs_sb_info *cifs_sb);
extern void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
-extern void invalidate_all_cached_dirs(struct cifs_tcon *tcon);
+extern void invalidate_all_cached_dirs(struct cached_fids *cfids);
extern bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]);
-
#endif /* _CACHED_DIR_H */
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index cb907e18cc35..5c195ffa3ead 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -386,7 +386,7 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
}
spin_unlock(&tcon->open_file_lock);
- invalidate_all_cached_dirs(tcon);
+ invalidate_all_cached_dirs(tcon->cfids);
spin_lock(&tcon->tc_lock);
if (tcon->status == TID_IN_FILES_INVALIDATE)
tcon->status = TID_NEED_TCON;
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index da23cc12a52c..8d70333a6a3d 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -169,7 +169,14 @@ tconInfoFree(struct cifs_tcon *tcon, enum smb3_tcon_ref_trace trace)
return;
}
trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count, trace);
- free_cached_dirs(tcon->cfids);
+
+ if (tcon->cfids) {
+ invalidate_all_cached_dirs(tcon->cfids);
+ cancel_delayed_work_sync(&tcon->cfids->laundromat_work);
+ kfree(tcon->cfids);
+ tcon->cfids = NULL;
+ }
+
atomic_dec(&tconInfoAllocCount);
kfree(tcon->nativeFileSystem);
kfree_sensitive(tcon->password);
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index c3b9d3f6210f..07ba61583114 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2197,7 +2197,7 @@ SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon)
}
spin_unlock(&ses->chan_lock);
- invalidate_all_cached_dirs(tcon);
+ invalidate_all_cached_dirs(tcon->cfids);
rc = smb2_plain_req_init(SMB2_TREE_DISCONNECT, tcon, server,
(void **) &req,
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 07/20] smb: client: merge free_cached_dir in release callback
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (5 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 06/20] smb: client: merge {close,invalidate}_all_cached_dirs() Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 08/20] smb: client: split find_or_create_cached_dir() Enzo Matsumiya
` (13 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
free_cached_dir() is no longer used anywhere else.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 36a1e1436502..8c8ead6e96bd 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -13,7 +13,6 @@
#include "cached_dir.h"
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 inline void invalidate_cfid(struct cached_fid *cfid)
@@ -456,6 +455,7 @@ static void
smb2_close_cached_fid(struct kref *ref)
{
struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+ struct cached_dirent *de, *q;
/*
* There's no way a valid cfid can reach here.
@@ -474,7 +474,17 @@ smb2_close_cached_fid(struct kref *ref)
list_del(&cfid->entry);
drop_cfid(cfid);
- free_cached_dir(cfid);
+
+ /* Delete all cached dirent names */
+ list_for_each_entry_safe(de, q, &cfid->dirents.entries, entry) {
+ list_del(&de->entry);
+ kfree(de->name);
+ kfree(de);
+ }
+
+ kfree(cfid->path);
+ cfid->path = NULL;
+ kfree(cfid);
}
void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
@@ -613,24 +623,6 @@ static struct cached_fid *init_cached_dir(const char *path)
return cfid;
}
-static void free_cached_dir(struct cached_fid *cfid)
-{
- struct cached_dirent *dirent, *q;
-
- /*
- * Delete all cached dirent names
- */
- list_for_each_entry_safe(dirent, q, &cfid->dirents.entries, entry) {
- list_del(&dirent->entry);
- kfree(dirent->name);
- kfree(dirent);
- }
-
- kfree(cfid->path);
- cfid->path = NULL;
- kfree(cfid);
-}
-
static void cfids_laundromat_worker(struct work_struct *work)
{
struct cached_fids *cfids;
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 08/20] smb: client: split find_or_create_cached_dir()
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (6 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 07/20] smb: client: merge free_cached_dir in release callback Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 09/20] smb: client: enhance cached dir lookups Enzo Matsumiya
` (12 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
This patch splits the function into 2 separate ones; it not only makes
the code clearer, but also allows further and easier enhancements to
both.
So move the initialization part into init_cached_dir() and add
find_cached_dir() for lookups.
Other:
- drop_cached_dir_by_name():
* use find_cached_dir()
* remove no longer used args
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 95 +++++++++++++++++++-------------------
fs/smb/client/cached_dir.h | 5 +-
fs/smb/client/inode.c | 2 +-
fs/smb/client/smb2inode.c | 6 +--
4 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 8c8ead6e96bd..92898880d20f 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -54,10 +54,7 @@ static inline void drop_cfid(struct cached_fid *cfid)
}
}
-static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
- const char *path,
- bool lookup_only,
- __u32 max_cached_dirs)
+static struct cached_fid *find_cached_dir(struct cached_fids *cfids, const char *path)
{
struct cached_fid *cfid;
@@ -71,35 +68,13 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
if (!cfid_is_valid(cfid))
return NULL;
+ cfid->last_access_time = jiffies;
kref_get(&cfid->refcount);
return cfid;
}
}
- if (lookup_only) {
- return NULL;
- }
- if (cfids->num_entries >= max_cached_dirs) {
- return NULL;
- }
- cfid = init_cached_dir(path);
- if (cfid == NULL) {
- return NULL;
- }
- cfid->cfids = cfids;
- cfids->num_entries++;
- list_add(&cfid->entry, &cfids->entries);
- 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;
- return cfid;
+ return NULL;
}
static struct dentry *
@@ -197,9 +172,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
ses = tcon->ses;
cfids = tcon->cfids;
- if (cfids == NULL)
+ if (!cfids)
return -EOPNOTSUPP;
-
replay_again:
/* reinitialize for possible replay */
flags = 0;
@@ -214,24 +188,31 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;
spin_lock(&cfids->cfid_list_lock);
- cfid = find_or_create_cached_dir(cfids, path, lookup_only, tcon->max_cached_dirs);
- if (cfid == NULL) {
+ if (cfids->num_entries >= tcon->max_cached_dirs) {
spin_unlock(&cfids->cfid_list_lock);
kfree(utf16_path);
return -ENOENT;
}
- /*
- * Return cached fid if it is valid (has a lease and has a time).
- * Otherwise, it is either a new entry or laundromat worker removed it
- * from @cfids->entries. Caller will put last reference if the latter.
- */
- if (cfid->has_lease && cfid->time) {
- cfid->last_access_time = jiffies;
- spin_unlock(&cfids->cfid_list_lock);
+
+ /* find_cached_dir() already checks if has_lease and time, so no need to check here */
+ cfid = find_cached_dir(cfids, path);
+ if (cfid || lookup_only) {
*ret_cfid = cfid;
+ spin_unlock(&cfids->cfid_list_lock);
+ kfree(utf16_path);
+ return cfid ? 0 : -ENOENT;
+ }
+
+ cfid = init_cached_dir(path);
+ if (!cfid) {
+ spin_unlock(&cfids->cfid_list_lock);
kfree(utf16_path);
- return 0;
+ return -ENOMEM;
}
+
+ cfid->cfids = cfids;
+ cfids->num_entries++;
+ list_add(&cfid->entry, &cfids->entries);
spin_unlock(&cfids->cfid_list_lock);
pfid = &cfid->fid;
@@ -487,19 +468,22 @@ smb2_close_cached_fid(struct kref *ref)
kfree(cfid);
}
-void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
- const char *name, struct cifs_sb_info *cifs_sb)
+void drop_cached_dir_by_name(struct cached_fids *cfids, const char *name)
{
- struct cached_fid *cfid = NULL;
- int rc;
+ struct cached_fid *cfid;
- rc = open_cached_dir(xid, tcon, name, cifs_sb, true, &cfid);
- if (rc) {
+ if (!cfids)
+ return;
+
+ cfid = find_cached_dir(cfids, name);
+ if (!cfid) {
cifs_dbg(FYI, "no cached dir found for rmdir(%s)\n", name);
return;
}
drop_cfid(cfid);
+
+ /* put lookup ref */
kref_put(&cfid->refcount, smb2_close_cached_fid);
}
@@ -609,6 +593,7 @@ static struct cached_fid *init_cached_dir(const char *path)
cfid = kzalloc(sizeof(*cfid), GFP_ATOMIC);
if (!cfid)
return NULL;
+
cfid->path = kstrdup(path, GFP_ATOMIC);
if (!cfid->path) {
kfree(cfid);
@@ -619,7 +604,23 @@ static struct cached_fid *init_cached_dir(const char *path)
INIT_LIST_HEAD(&cfid->dirents.entries);
mutex_init(&cfid->dirents.de_mutex);
spin_lock_init(&cfid->fid_lock);
+
+ /* this is our ref */
kref_init(&cfid->refcount);
+
+ /* 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;
+
return cfid;
}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 47c0404ba84a..4bc93131275e 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -79,10 +79,7 @@ extern int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
struct dentry *dentry,
struct cached_fid **cfid);
extern void close_cached_dir(struct cached_fid *cfid);
-extern void drop_cached_dir_by_name(const unsigned int xid,
- struct cifs_tcon *tcon,
- const char *name,
- struct cifs_sb_info *cifs_sb);
+extern void drop_cached_dir_by_name(struct cached_fids *cfids, const char *name);
extern void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
extern void invalidate_all_cached_dirs(struct cached_fids *cfids);
extern bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index e80bf55765b6..9344a86f6d46 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2615,7 +2615,7 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
* ->i_nlink and then mark it as delete pending.
*/
if (S_ISDIR(inode->i_mode)) {
- drop_cached_dir_by_name(xid, tcon, to_name, cifs_sb);
+ drop_cached_dir_by_name(tcon->cfids, to_name);
spin_lock(&inode->i_lock);
i_size_write(inode, 0);
clear_nlink(inode);
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 7cadc8ca4f55..f462845dd167 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1162,7 +1162,7 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
{
struct cifs_open_parms oparms;
- drop_cached_dir_by_name(xid, tcon, name, cifs_sb);
+ drop_cached_dir_by_name(tcon->cfids, name);
oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE,
FILE_OPEN, CREATE_NOT_FILE, ACL_NO_MODE);
return smb2_compound_op(xid, tcon, cifs_sb,
@@ -1240,7 +1240,7 @@ int smb2_rename_path(const unsigned int xid,
struct cifsFileInfo *cfile;
__u32 co = file_create_options(source_dentry);
- drop_cached_dir_by_name(xid, tcon, from_name, cifs_sb);
+ drop_cached_dir_by_name(tcon->cfids, from_name);
cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile);
int rc = smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb,
@@ -1522,7 +1522,7 @@ int smb2_rename_pending_delete(const char *full_path,
goto out;
}
- drop_cached_dir_by_name(xid, tcon, full_path, cifs_sb);
+ drop_cached_dir_by_name(tcon->cfids, full_path);
oparms = CIFS_OPARMS(cifs_sb, tcon, full_path,
DELETE | FILE_WRITE_ATTRIBUTES,
FILE_OPEN, co, ACL_NO_MODE);
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 09/20] smb: client: enhance cached dir lookups
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (7 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 08/20] smb: client: split find_or_create_cached_dir() Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 10/20] smb: client: refactor dropping cached dirs Enzo Matsumiya
` (11 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Enable cfid lookups to be matched with the 3 modes (path, dentry,
and lease key) currently used in a single function.
Caller exposed function (find_cached_dir()) checks if cfid is
mid-creation in open_cached_dir() and retries the lookup, avoiding
opening the same path again.
Changes:
- expose find_cached_dir()
- add CFID_LOOKUP_* modes
- remove @lookup_only arg from open_cached_dir(), replace, in calllers,
with find_cached_dir() where it was true
- remove open_cached_dir_by_dentry(), replace with find_cached_dir()
- use find_cached_dir() in cached_dir_lease_break()
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 200 ++++++++++++++++++++++---------------
fs/smb/client/cached_dir.h | 17 ++--
fs/smb/client/inode.c | 11 +-
fs/smb/client/readdir.c | 4 +-
fs/smb/client/smb2inode.c | 10 +-
fs/smb/client/smb2ops.c | 15 ++-
6 files changed, 144 insertions(+), 113 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 92898880d20f..37a9bff26da7 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -54,27 +54,63 @@ static inline void drop_cfid(struct cached_fid *cfid)
}
}
-static struct cached_fid *find_cached_dir(struct cached_fids *cfids, const char *path)
+/*
+ * Find a cached dir based on @key and @mode (raw lookup).
+ * The only validation done here is if cfid is not going down (last_access_time != 1).
+ *
+ * If @wait_open is true, keep retrying until cfid transitions from 'opening' to valid/invalid.
+ *
+ * Callers must handle any other validation as needed.
+ * Returned cfid, if found, has a ref taken, regardless of state.
+ */
+static struct cached_fid *find_cfid(struct cached_fids *cfids, const void *key, int mode,
+ bool wait_open)
{
- struct cached_fid *cfid;
+ struct cached_fid *cfid, *found;
+ bool match;
+ if (!cfids || !key)
+ return NULL;
+
+retry_find:
+ found = NULL;
+
+ spin_lock(&cfids->cfid_list_lock);
list_for_each_entry(cfid, &cfids->entries, entry) {
- if (!strcmp(cfid->path, path)) {
- /*
- * If it doesn't have a lease it is either not yet
- * fully cached or it may be in the process of
- * being deleted due to a lease break.
- */
- if (!cfid_is_valid(cfid))
- return NULL;
+ /* don't even bother checking if it's going away */
+ if (cfid->last_access_time == 1)
+ continue;
- cfid->last_access_time = jiffies;
+ if (mode == CFID_LOOKUP_PATH)
+ match = !strcmp(cfid->path, (char *)key);
+
+ if (mode == CFID_LOOKUP_DENTRY)
+ match = (cfid->dentry == key);
+
+ if (mode == CFID_LOOKUP_LEASEKEY)
+ match = !memcmp(cfid->fid.lease_key, (u8 *)key, SMB2_LEASE_KEY_SIZE);
+
+ if (!match)
+ continue;
+
+ /* only get a ref here if not waiting for open */
+ if (!wait_open)
kref_get(&cfid->refcount);
- return cfid;
- }
+ found = cfid;
+ break;
}
+ spin_unlock(&cfids->cfid_list_lock);
+
+ if (wait_open && found) {
+ /* cfid is being opened in open_cached_dir(), retry lookup */
+ if (found->has_lease && !found->time && !found->last_access_time)
+ goto retry_find;
- return NULL;
+ /* we didn't get a ref above, so get one now */
+ kref_get(&found->refcount);
+ }
+
+ return found;
}
static struct dentry *
@@ -133,14 +169,38 @@ static const char *path_no_prefix(struct cifs_sb_info *cifs_sb,
return path + len;
}
+/*
+ * Find a cached dir based on @key and @mode (caller exposed).
+ * This function will retry lookup if cfid found is in opening state.
+ *
+ * Returns valid cfid (with updated last_access_time) or NULL.
+ */
+struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void *key, int mode)
+{
+ struct cached_fid *cfid;
+
+ if (!cfids || !key)
+ return NULL;
+
+ cfid = find_cfid(cfids, key, mode, true);
+ if (cfid) {
+ if (cfid_is_valid(cfid)) {
+ cfid->last_access_time = jiffies;
+ } else {
+ kref_put(&cfid->refcount, smb2_close_cached_fid);
+ cfid = NULL;
+ }
+ }
+
+ return cfid;
+}
+
/*
* Open the and cache a directory handle.
* If error then *cfid is not initialized.
*/
-int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
- const char *path,
- struct cifs_sb_info *cifs_sb,
- bool lookup_only, struct cached_fid **ret_cfid)
+int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
+ struct cifs_sb_info *cifs_sb, struct cached_fid **ret_cfid)
{
struct cifs_ses *ses;
struct TCP_Server_Info *server;
@@ -156,7 +216,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
__le16 *utf16_path = NULL;
u8 oplock = SMB2_OPLOCK_LEVEL_II;
struct cifs_fid *pfid;
- struct dentry *dentry = NULL;
+ struct dentry *dentry;
struct cached_fid *cfid;
struct cached_fids *cfids;
const char *npath;
@@ -178,6 +238,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
/* reinitialize for possible replay */
flags = 0;
oplock = SMB2_OPLOCK_LEVEL_II;
+ dentry = NULL;
+ cfid = NULL;
+ *ret_cfid = NULL;
server = cifs_pick_channel(ses);
if (!server->ops->new_lease_key)
@@ -187,27 +250,25 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
if (!utf16_path)
return -ENOMEM;
- spin_lock(&cfids->cfid_list_lock);
- if (cfids->num_entries >= tcon->max_cached_dirs) {
- spin_unlock(&cfids->cfid_list_lock);
- kfree(utf16_path);
- return -ENOENT;
+ /* find_cached_dir() already checks has_lease and time, so no need to check here */
+ cfid = find_cached_dir(cfids, path, CFID_LOOKUP_PATH);
+ if (cfid) {
+ rc = 0;
+ goto out;
}
- /* find_cached_dir() already checks if has_lease and time, so no need to check here */
- cfid = find_cached_dir(cfids, path);
- if (cfid || lookup_only) {
- *ret_cfid = cfid;
+ spin_lock(&cfids->cfid_list_lock);
+ if (cfids->num_entries >= tcon->max_cached_dirs) {
spin_unlock(&cfids->cfid_list_lock);
- kfree(utf16_path);
- return cfid ? 0 : -ENOENT;
+ rc = -ENOENT;
+ goto out;
}
cfid = init_cached_dir(path);
if (!cfid) {
spin_unlock(&cfids->cfid_list_lock);
- kfree(utf16_path);
- return -ENOMEM;
+ rc = -ENOMEM;
+ goto out;
}
cfid->cfids = cfids;
@@ -393,8 +454,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
rc = -ENOENT;
if (rc) {
- drop_cfid(cfid);
- kref_put(&cfid->refcount, smb2_close_cached_fid);
+ if (cfid) {
+ drop_cfid(cfid);
+ kref_put(&cfid->refcount, smb2_close_cached_fid);
+ }
} else {
*ret_cfid = cfid;
atomic_inc(&tcon->num_remote_opens);
@@ -408,32 +471,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
return rc;
}
-int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
- struct dentry *dentry,
- struct cached_fid **ret_cfid)
-{
- struct cached_fid *cfid;
- struct cached_fids *cfids = tcon->cfids;
-
- if (cfids == NULL)
- return -EOPNOTSUPP;
-
- spin_lock(&cfids->cfid_list_lock);
- list_for_each_entry(cfid, &cfids->entries, entry) {
- 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;
- spin_unlock(&cfids->cfid_list_lock);
- return 0;
- }
- }
- spin_unlock(&cfids->cfid_list_lock);
- return -ENOENT;
-}
-
-static void
-smb2_close_cached_fid(struct kref *ref)
+static void smb2_close_cached_fid(struct kref *ref)
{
struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
struct cached_dirent *de, *q;
@@ -475,7 +513,7 @@ void drop_cached_dir_by_name(struct cached_fids *cfids, const char *name)
if (!cfids)
return;
- cfid = find_cached_dir(cfids, name);
+ cfid = find_cached_dir(cfids, name, CFID_LOOKUP_PATH);
if (!cfid) {
cifs_dbg(FYI, "no cached dir found for rmdir(%s)\n", name);
return;
@@ -555,35 +593,31 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
{
struct cached_fids *cfids = tcon->cfids;
struct cached_fid *cfid;
- bool found = false;
if (cfids == NULL)
return false;
- spin_lock(&cfids->cfid_list_lock);
- list_for_each_entry(cfid, &cfids->entries, entry) {
- if (cfid->has_lease &&
- !memcmp(lease_key,
- cfid->fid.lease_key,
- SMB2_LEASE_KEY_SIZE)) {
- /*
- * We found a lease, invalidate cfid and schedule immediate cleanup on
- * laundromat.
- * No need to take a ref here, as we still hold our initial one.
- */
- invalidate_cfid(cfid);
- cfid->has_lease = false;
- found = true;
- break;
- }
- }
- spin_unlock(&cfids->cfid_list_lock);
+ /*
+ * Raw lookup here as we _must_ find our lease, no matter cfid state.
+ * Also, this lease break might be coming from the SMB2 open in open_cached_dir(), so no
+ * need to wait for it to finish.
+ */
+ cfid = find_cfid(cfids, lease_key, CFID_LOOKUP_LEASEKEY, false);
+ if (cfid) {
+ /* found a lease, invalidate cfid and schedule immediate cleanup on laundromat */
+ spin_lock(&cfids->cfid_list_lock);
+ invalidate_cfid(cfid);
+ cfid->has_lease = false;
+ spin_unlock(&cfids->cfid_list_lock);
- /* avoid unnecessary scheduling */
- if (found)
+ /* put lookup ref */
+ kref_put(&cfid->refcount, smb2_close_cached_fid);
mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
- return found;
+ return true;
+ }
+
+ return false;
}
static struct cached_fid *init_cached_dir(const char *path)
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 4bc93131275e..afb9af227219 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -59,6 +59,13 @@ struct cached_fids {
struct delayed_work laundromat_work;
};
+/* Lookup modes for find_cached_dir() */
+enum {
+ CFID_LOOKUP_PATH,
+ CFID_LOOKUP_DENTRY,
+ CFID_LOOKUP_LEASEKEY,
+};
+
static inline bool cfid_expired(const struct cached_fid *cfid)
{
return (cfid->last_access_time &&
@@ -71,13 +78,9 @@ static inline bool cfid_is_valid(const struct cached_fid *cfid)
}
extern struct cached_fids *init_cached_dirs(void);
-extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
- const char *path,
- struct cifs_sb_info *cifs_sb,
- bool lookup_only, struct cached_fid **cfid);
-extern int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
- struct dentry *dentry,
- struct cached_fid **cfid);
+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,
+ struct cifs_sb_info *cifs_sb, struct cached_fid **cfid);
extern void close_cached_dir(struct cached_fid *cfid);
extern void drop_cached_dir_by_name(struct cached_fids *cfids, const char *name);
extern void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 9344a86f6d46..df236c844611 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2676,7 +2676,7 @@ cifs_dentry_needs_reval(struct dentry *dentry)
struct cifsInodeInfo *cifs_i = CIFS_I(inode);
struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
- struct cached_fid *cfid = NULL;
+ struct cached_fid *cfid;
if (test_bit(CIFS_INO_DELETE_PENDING, &cifs_i->flags))
return false;
@@ -2689,13 +2689,12 @@ cifs_dentry_needs_reval(struct dentry *dentry)
if (!lookupCacheEnabled)
return true;
- if (!open_cached_dir_by_dentry(tcon, dentry->d_parent, &cfid)) {
- if (cfid->time && cifs_i->time > cfid->time) {
- close_cached_dir(cfid);
- return false;
- }
+ cfid = find_cached_dir(tcon->cfids, dentry->d_parent, CFID_LOOKUP_DENTRY);
+ if (cfid) {
close_cached_dir(cfid);
+ return false;
}
+
/*
* depending on inode type, check if attribute caching disabled for
* files or directories
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 4e5460206397..cc6762d950d2 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -1065,7 +1065,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
tcon = tlink_tcon(cifsFile->tlink);
}
- rc = open_cached_dir(xid, tcon, full_path, cifs_sb, false, &cfid);
+ rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid);
cifs_put_tlink(tlink);
if (rc)
goto cache_not_found;
@@ -1136,7 +1136,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
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, false, &cfid);
+ open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid);
if (rc) {
cifs_dbg(FYI, "fce error %d\n", rc);
goto rddir2_exit;
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index f462845dd167..c76fe1dec390 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -964,12 +964,10 @@ int smb2_query_path_info(const unsigned int xid,
* is fast enough (always using the compounded version).
*/
if (!tcon->posix_extensions) {
- if (*full_path) {
- rc = -ENOENT;
- } else {
- rc = open_cached_dir(xid, tcon, full_path,
- cifs_sb, false, &cfid);
- }
+ rc = -ENOENT;
+ if (!*full_path)
+ rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid);
+
/* If it is a root and its handle is cached then use it */
if (!rc) {
if (cfid->file_all_info_is_valid) {
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index e586f3f4b5c9..39e6dc13d2da 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -882,7 +882,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon,
.fid = &fid,
};
- rc = open_cached_dir(xid, tcon, "", cifs_sb, false, &cfid);
+ rc = open_cached_dir(xid, tcon, "", cifs_sb, &cfid);
if (rc == 0)
memcpy(&fid, &cfid->fid, sizeof(struct cifs_fid));
else
@@ -952,13 +952,10 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
bool islink;
int rc, rc2;
- rc = open_cached_dir(xid, tcon, full_path, cifs_sb, true, &cfid);
- if (!rc) {
- if (cfid->has_lease) {
- close_cached_dir(cfid);
- return 0;
- }
+ cfid = find_cached_dir(tcon->cfids, full_path, CFID_LOOKUP_PATH);
+ if (cfid) {
close_cached_dir(cfid);
+ return 0;
}
utf16_path = cifs_convert_path_to_utf16(full_path, cifs_sb);
@@ -2747,8 +2744,8 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
* We can only call this for things we know are directories.
*/
if (!strcmp(path, ""))
- open_cached_dir(xid, tcon, path, cifs_sb, false,
- &cfid); /* cfid null if open dir failed */
+ /* cfid null if open dir failed */
+ open_cached_dir(xid, tcon, path, cifs_sb, &cfid);
rqst[0].rq_iov = vars->open_iov;
rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 10/20] smb: client: refactor dropping cached dirs
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (8 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 09/20] smb: client: enhance cached dir lookups Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 11/20] smb: client: simplify cached_fid state checking Enzo Matsumiya
` (10 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
- s/drop_cached_dir_by_name/drop_cached_dir/
make it a generic find + invalidate function to replace
drop_cached_dir_by_name() and cached_dir_lease_break()
We now funnel any cleanup to laundromat, so we can make the release
callback free only dirents, path, and cfid itself.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 78 +++++++++++---------------------------
fs/smb/client/cached_dir.h | 3 +-
fs/smb/client/inode.c | 2 +-
fs/smb/client/smb2inode.c | 7 ++--
fs/smb/client/smb2misc.c | 2 +-
5 files changed, 29 insertions(+), 63 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 37a9bff26da7..84ea2653cdb9 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -476,22 +476,6 @@ static void smb2_close_cached_fid(struct kref *ref)
struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
struct cached_dirent *de, *q;
- /*
- * There's no way a valid cfid can reach here.
- *
- * This is because we hould our own ref, and whenever we put it, we invalidate the cfid.
- *
- * So even if an external caller puts the last ref, cfid will already have been invalidated
- * by then by one of the invalidations that can happen concurrently, e.g. lease break,
- * invalidate_all_cached_dirs().
- *
- * So this check is mostly for precaution, but since we can still take the correct action
- * (just list_del()) if it's the case, do so.
- */
- if (WARN_ON(cfid_is_valid(cfid)))
- /* remaining invalidation done by drop_cfid() below */
- list_del(&cfid->entry);
-
drop_cfid(cfid);
/* Delete all cached dirent names */
@@ -506,23 +490,36 @@ static void smb2_close_cached_fid(struct kref *ref)
kfree(cfid);
}
-void drop_cached_dir_by_name(struct cached_fids *cfids, const char *name)
+bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode)
{
struct cached_fid *cfid;
- if (!cfids)
- return;
+ if (!cfids || !key)
+ return false;
- cfid = find_cached_dir(cfids, name, CFID_LOOKUP_PATH);
- if (!cfid) {
- cifs_dbg(FYI, "no cached dir found for rmdir(%s)\n", name);
- return;
- }
+ /*
+ * Raw lookup here as we _must_ find any matching cfid, no matter its state.
+ * Also, we might be racing with the SMB2 open in open_cached_dir(), so no need to wait
+ * for it to finish.
+ */
+ cfid = find_cfid(cfids, key, mode, false);
+ if (!cfid)
+ return false;
- drop_cfid(cfid);
+ if (mode != CFID_LOOKUP_LEASEKEY) {
+ drop_cfid(cfid);
+ } else {
+ /* we're locked in smb2_is_valid_lease_break(), so can't dput/close here */
+ spin_lock(&cfids->cfid_list_lock);
+ invalidate_cfid(cfid);
+ spin_unlock(&cfids->cfid_list_lock);
+ }
/* put lookup ref */
kref_put(&cfid->refcount, smb2_close_cached_fid);
+ mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
+
+ return true;
}
void close_cached_dir(struct cached_fid *cfid)
@@ -589,37 +586,6 @@ void invalidate_all_cached_dirs(struct cached_fids *cfids)
flush_delayed_work(&cfids->laundromat_work);
}
-bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
-{
- struct cached_fids *cfids = tcon->cfids;
- struct cached_fid *cfid;
-
- if (cfids == NULL)
- return false;
-
- /*
- * Raw lookup here as we _must_ find our lease, no matter cfid state.
- * Also, this lease break might be coming from the SMB2 open in open_cached_dir(), so no
- * need to wait for it to finish.
- */
- cfid = find_cfid(cfids, lease_key, CFID_LOOKUP_LEASEKEY, false);
- if (cfid) {
- /* found a lease, invalidate cfid and schedule immediate cleanup on laundromat */
- spin_lock(&cfids->cfid_list_lock);
- invalidate_cfid(cfid);
- cfid->has_lease = false;
- spin_unlock(&cfids->cfid_list_lock);
-
- /* put lookup ref */
- kref_put(&cfid->refcount, smb2_close_cached_fid);
- mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
-
- return true;
- }
-
- return false;
-}
-
static struct cached_fid *init_cached_dir(const char *path)
{
struct cached_fid *cfid;
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index afb9af227219..bed5ba68b07f 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -82,8 +82,7 @@ extern struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void
extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
struct cifs_sb_info *cifs_sb, struct cached_fid **cfid);
extern void close_cached_dir(struct cached_fid *cfid);
-extern void drop_cached_dir_by_name(struct cached_fids *cfids, const char *name);
+extern bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode);
extern void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
extern void invalidate_all_cached_dirs(struct cached_fids *cfids);
-extern bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]);
#endif /* _CACHED_DIR_H */
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index df236c844611..f2eff1138ed0 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2615,7 +2615,7 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
* ->i_nlink and then mark it as delete pending.
*/
if (S_ISDIR(inode->i_mode)) {
- drop_cached_dir_by_name(tcon->cfids, to_name);
+ drop_cached_dir(tcon->cfids, to_name, CFID_LOOKUP_PATH);
spin_lock(&inode->i_lock);
i_size_write(inode, 0);
clear_nlink(inode);
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index c76fe1dec390..62d6adf50ad1 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1160,7 +1160,7 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
{
struct cifs_open_parms oparms;
- drop_cached_dir_by_name(tcon->cfids, name);
+ drop_cached_dir(tcon->cfids, name, CFID_LOOKUP_PATH);
oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE,
FILE_OPEN, CREATE_NOT_FILE, ACL_NO_MODE);
return smb2_compound_op(xid, tcon, cifs_sb,
@@ -1238,7 +1238,7 @@ int smb2_rename_path(const unsigned int xid,
struct cifsFileInfo *cfile;
__u32 co = file_create_options(source_dentry);
- drop_cached_dir_by_name(tcon->cfids, from_name);
+ drop_cached_dir(tcon->cfids, from_name, CFID_LOOKUP_PATH);
cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile);
int rc = smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb,
@@ -1520,7 +1520,8 @@ int smb2_rename_pending_delete(const char *full_path,
goto out;
}
- drop_cached_dir_by_name(tcon->cfids, full_path);
+ drop_cached_dir(tcon->cfids, full_path, CFID_LOOKUP_PATH);
+
oparms = CIFS_OPARMS(cifs_sb, tcon, full_path,
DELETE | FILE_WRITE_ATTRIBUTES,
FILE_OPEN, co, ACL_NO_MODE);
diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c
index 89d933b4a8bc..71d987f76a12 100644
--- a/fs/smb/client/smb2misc.c
+++ b/fs/smb/client/smb2misc.c
@@ -660,7 +660,7 @@ smb2_is_valid_lease_break(char *buffer, struct TCP_Server_Info *server)
}
spin_unlock(&tcon->open_file_lock);
- if (cached_dir_lease_break(tcon, rsp->LeaseKey)) {
+ if (drop_cached_dir(tcon->cfids, rsp->LeaseKey, CFID_LOOKUP_LEASEKEY)) {
spin_unlock(&cifs_tcp_ses_lock);
return true;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 11/20] smb: client: simplify cached_fid state checking
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (9 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 10/20] smb: client: refactor dropping cached dirs Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children Enzo Matsumiya
` (9 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
cached_fid validity (usable by callers) is already based on ->time != 0,
having other flags/booleans to check the same thing can be confusing and
make things unnecessarily complex.
This patch removes cached_fid booleans ->has_lease, ->is_open,
->file_all_info_is_valid.
Replace their semantics with already existing, simpler checks:
- ->is_open is replaced by checking if persistent_fid != 0
- ->has_lease is currently used as a "is valid" check, but we already
validate it based on ->time anyway, so drop it
- ->file_all_info becomes a pointer and its presence becomes its
validity
This patch also concretly defines the "is opening" semantic; it's
based on ->time == 0, which is used as "creation time", so the only
time it's 0 is between allocation and valid (opened/cached) or invalid,
and it also never transitions back to 0 again.
(->last_access_time follows the same, but it's already used for
expiration checks)
Other:
- add CFID_INVALID_TIME (value 1); this allows us to differentiate
between opening (0), valid (jiffies), or invalid (1)
- rename time/last_access_time to ctime/atime to follow other common
usage of such fields
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 72 +++++++++++++++++++-------------------
fs/smb/client/cached_dir.h | 14 +++-----
fs/smb/client/cifs_debug.c | 2 +-
fs/smb/client/smb2inode.c | 17 +++++----
4 files changed, 52 insertions(+), 53 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 84ea2653cdb9..ff71f2c06b72 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -12,6 +12,9 @@
#include "smb2proto.h"
#include "cached_dir.h"
+/* since jiffies can never be 1 here, use it as an invalid value to add meaning for our purposes */
+#define CFID_INVALID_TIME 1
+
static struct cached_fid *init_cached_dir(const char *path);
static void smb2_close_cached_fid(struct kref *ref);
@@ -24,29 +27,30 @@ static inline void invalidate_cfid(struct cached_fid *cfid)
cfid->cfids->num_entries--;
/* do not change other fields here! */
- cfid->time = 0;
- cfid->last_access_time = 1;
+ cfid->ctime = CFID_INVALID_TIME;
+ cfid->atime = CFID_INVALID_TIME;
}
static inline void drop_cfid(struct cached_fid *cfid)
{
struct dentry *dentry = NULL;
+ u64 pfid = 0, vfid = 0;
spin_lock(&cfid->cfids->cfid_list_lock);
invalidate_cfid(cfid);
spin_lock(&cfid->fid_lock);
swap(cfid->dentry, dentry);
+ swap(cfid->fid.persistent_fid, pfid);
+ swap(cfid->fid.volatile_fid, vfid);
spin_unlock(&cfid->fid_lock);
spin_unlock(&cfid->cfids->cfid_list_lock);
dput(dentry);
- if (cfid->is_open) {
- int rc;
-
- cfid->is_open = false;
- rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid, cfid->fid.volatile_fid);
+ if (pfid) {
+ /* cfid->tcon is never set to NULL, so no need to check/swap it */
+ int rc = SMB2_close(0, cfid->tcon, pfid, vfid);
/* SMB2_close should handle -EBUSY or -EAGAIN */
if (rc)
@@ -56,7 +60,7 @@ static inline void drop_cfid(struct cached_fid *cfid)
/*
* Find a cached dir based on @key and @mode (raw lookup).
- * The only validation done here is if cfid is not going down (last_access_time != 1).
+ * The only validation done here is if cfid is going down (->ctime == CFID_INVALID_TIME).
*
* If @wait_open is true, keep retrying until cfid transitions from 'opening' to valid/invalid.
*
@@ -78,7 +82,7 @@ static struct cached_fid *find_cfid(struct cached_fids *cfids, const void *key,
spin_lock(&cfids->cfid_list_lock);
list_for_each_entry(cfid, &cfids->entries, entry) {
/* don't even bother checking if it's going away */
- if (cfid->last_access_time == 1)
+ if (cfid->ctime == CFID_INVALID_TIME)
continue;
if (mode == CFID_LOOKUP_PATH)
@@ -103,7 +107,7 @@ static struct cached_fid *find_cfid(struct cached_fids *cfids, const void *key,
if (wait_open && found) {
/* cfid is being opened in open_cached_dir(), retry lookup */
- if (found->has_lease && !found->time && !found->last_access_time)
+ if (!found->ctime)
goto retry_find;
/* we didn't get a ref above, so get one now */
@@ -185,7 +189,7 @@ struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void *key, i
cfid = find_cfid(cfids, key, mode, true);
if (cfid) {
if (cfid_is_valid(cfid)) {
- cfid->last_access_time = jiffies;
+ cfid->atime = jiffies;
} else {
kref_put(&cfid->refcount, smb2_close_cached_fid);
cfid = NULL;
@@ -207,6 +211,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];
@@ -241,6 +246,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
dentry = NULL;
cfid = NULL;
*ret_cfid = NULL;
+ memset(&info, 0, sizeof(info));
server = cifs_pick_channel(ses);
if (!server->ops->new_lease_key)
@@ -250,7 +256,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
if (!utf16_path)
return -ENOMEM;
- /* find_cached_dir() already checks has_lease and time, so no need to check here */
+ /* find_cached_dir() already validates cfid if found, so no need to check here again */
cfid = find_cached_dir(cfids, path, CFID_LOOKUP_PATH);
if (cfid) {
rc = 0;
@@ -393,7 +399,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
}
goto oshr_free;
}
- cfid->is_open = true;
spin_lock(&cfids->cfid_list_lock);
@@ -430,19 +435,21 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
spin_unlock(&cfids->cfid_list_lock);
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;
+ 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)) {
+ cfid->file_all_info = kmemdup(&info, sizeof(info), GFP_ATOMIC);
+ if (!cfid->file_all_info) {
+ rc = -ENOMEM;
+ goto out;
+ }
+ }
+
+ cfid->ctime = jiffies;
+ cfid->atime = jiffies;
spin_unlock(&cfids->cfid_list_lock);
/* At this point the directory handle is fully cached */
rc = 0;
-
oshr_free:
SMB2_open_free(&rqst[0]);
SMB2_query_info_free(&rqst[1]);
@@ -485,6 +492,8 @@ static void smb2_close_cached_fid(struct kref *ref)
kfree(de);
}
+ kfree(cfid->file_all_info);
+ cfid->file_all_info = NULL;
kfree(cfid->path);
cfid->path = NULL;
kfree(cfid);
@@ -538,9 +547,10 @@ 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);
- cfid->has_lease = false;
- if (closed)
- cfid->is_open = false;
+ if (closed) {
+ cfid->fid.persistent_fid = 0;
+ cfid->fid.volatile_fid = 0;
+ }
}
spin_unlock(&cfids->cfid_list_lock);
@@ -611,16 +621,6 @@ 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;
-
return cfid;
}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index bed5ba68b07f..c45151446049 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -33,17 +33,14 @@ 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;
- unsigned long time; /* jiffies of when lease was taken */
- unsigned long last_access_time; /* jiffies of when last accessed */
+ unsigned long ctime; /* (jiffies) creation time, when cfid was created (cached) */
+ unsigned long atime; /* (jiffies) access time, when it was last used */
struct kref refcount;
struct cifs_fid 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 *file_all_info;
struct cached_dirents dirents;
};
@@ -68,13 +65,12 @@ enum {
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));
+ return (cfid->atime && time_is_before_jiffies(cfid->atime + HZ * dir_cache_timeout));
}
static inline bool cfid_is_valid(const struct cached_fid *cfid)
{
- return (cfid->has_lease && cfid->time && !cfid_expired(cfid));
+ return (cfid->fid.persistent_fid && cfid->ctime && !cfid_expired(cfid));
}
extern struct cached_fids *init_cached_dirs(void);
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 2337cf795db3..bb27b9c97724 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->file_all_info)
seq_printf(m, "\tvalid file info");
if (cfid->dirents.is_valid)
seq_printf(m, ", valid dirents");
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 62d6adf50ad1..8ccdd1a3ba2c 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -970,14 +970,17 @@ 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->file_all_info) {
+ memcpy(&data->fi, cfid->file_all_info, sizeof(data->fi));
} else {
- rc = SMB2_query_info(xid, tcon,
- cfid->fid.persistent_fid,
- cfid->fid.volatile_fid,
- &data->fi);
+ rc = SMB2_query_info(xid, tcon, cfid->fid.persistent_fid,
+ cfid->fid.volatile_fid, &data->fi);
+ if (!rc) {
+ cfid->file_all_info = kmemdup(&data->fi, sizeof(data->fi),
+ GFP_KERNEL);
+ if (!cfid->file_all_info)
+ rc = -ENOMEM;
+ }
}
close_cached_dir(cfid);
return rc;
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (10 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 11/20] smb: client: simplify cached_fid state checking Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 14:23 ` Steve French
2025-09-29 17:17 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 13/20] smb: client: actually use cached dirs on readdir Enzo Matsumiya
` (8 subsequent siblings)
20 siblings, 2 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In SMB2_open_init(), when opening (not creating/deleting) a path, lookup
for a cached parent and set ParentLeaseKey in lease context if found.
Other:
- set oparms->cifs_sb in open_cached_dir() as we need it in
add_parent_lease_key(); use CIFS_OPARMS() too
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 42 ++++---------------
fs/smb/client/dir.c | 26 +++---------
fs/smb/client/smb2inode.c | 2 +
fs/smb/client/smb2pdu.c | 86 ++++++++++++++++++++++++++++++++------
4 files changed, 88 insertions(+), 68 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index ff71f2c06b72..9dd74268b2d8 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -226,7 +226,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
struct cached_fids *cfids;
const char *npath;
int retries = 0, cur_sleep = 1;
- __le32 lease_flags = 0;
if (cifs_sb->root == NULL)
return -ENOENT;
@@ -236,9 +235,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
ses = tcon->ses;
cfids = tcon->cfids;
-
if (!cfids)
return -EOPNOTSUPP;
+
replay_again:
/* reinitialize for possible replay */
flags = 0;
@@ -306,24 +305,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
rc = -ENOENT;
goto out;
}
- if (dentry->d_parent && server->dialect >= SMB30_PROT_ID) {
- struct cached_fid *parent_cfid;
-
- spin_lock(&cfids->cfid_list_lock);
- list_for_each_entry(parent_cfid, &cfids->entries, entry) {
- if (parent_cfid->dentry == dentry->d_parent) {
- if (!cfid_is_valid(parent_cfid))
- break;
-
- cifs_dbg(FYI, "found a parent cached file handle\n");
- lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
- memcpy(pfid->parent_lease_key, parent_cfid->fid.lease_key,
- SMB2_LEASE_KEY_SIZE);
- break;
- }
- }
- spin_unlock(&cfids->cfid_list_lock);
- }
}
cfid->dentry = dentry;
cfid->tcon = tcon;
@@ -350,20 +331,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
rqst[0].rq_iov = open_iov;
rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
- oparms = (struct cifs_open_parms) {
- .tcon = tcon,
- .path = path,
- .create_options = cifs_create_options(cifs_sb, CREATE_NOT_FILE),
- .desired_access = FILE_READ_DATA | FILE_READ_ATTRIBUTES |
- FILE_READ_EA,
- .disposition = FILE_OPEN,
- .fid = pfid,
- .lease_flags = lease_flags,
- .replay = !!(retries),
- };
-
- rc = SMB2_open_init(tcon, server,
- &rqst[0], &oplock, &oparms, utf16_path);
+ oparms = CIFS_OPARMS(cifs_sb, tcon, path,
+ FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA, FILE_OPEN,
+ cifs_create_options(cifs_sb, CREATE_NOT_FILE), 0);
+ oparms.fid = pfid;
+ oparms.replay = !!retries;
+
+ rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
if (rc)
goto oshr_free;
smb2_set_next_command(tcon, &rqst[0]);
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index e5372c2c799d..b60af27668bb 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -189,10 +189,9 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
struct inode *newinode = NULL;
int disposition;
struct TCP_Server_Info *server = tcon->ses->server;
+ struct cached_fid *parent_cfid;
struct cifs_open_parms oparms;
- struct cached_fid *parent_cfid = NULL;
int rdwr_for_fscache = 0;
- __le32 lease_flags = 0;
*oplock = 0;
if (tcon->ses->server->oplocks)
@@ -314,25 +313,11 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
if (!tcon->unix_ext && (mode & S_IWUGO) == 0)
create_options |= CREATE_OPTION_READONLY;
-
retry_open:
- if (tcon->cfids && direntry->d_parent && server->dialect >= SMB30_PROT_ID) {
- parent_cfid = NULL;
- spin_lock(&tcon->cfids->cfid_list_lock);
- list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
- if (parent_cfid->dentry == direntry->d_parent) {
- if (!cfid_is_valid(parent_cfid))
- break;
-
- cifs_dbg(FYI, "found a parent cached file handle\n");
- lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
- memcpy(fid->parent_lease_key, parent_cfid->fid.lease_key,
- SMB2_LEASE_KEY_SIZE);
- parent_cfid->dirents.is_valid = false;
- break;
- }
- }
- spin_unlock(&tcon->cfids->cfid_list_lock);
+ parent_cfid = find_cached_dir(tcon->cfids, direntry->d_parent, CFID_LOOKUP_DENTRY);
+ if (parent_cfid) {
+ parent_cfid->dirents.is_valid = false;
+ close_cached_dir(parent_cfid);
}
oparms = (struct cifs_open_parms) {
@@ -343,7 +328,6 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
.disposition = disposition,
.path = full_path,
.fid = fid,
- .lease_flags = lease_flags,
.mode = mode,
};
rc = server->ops->open(xid, &oparms, oplock, buf);
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 8ccdd1a3ba2c..6d643b8b9547 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1120,6 +1120,8 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode,
{
struct cifs_open_parms oparms;
+ drop_cached_dir(tcon->cfids, name, CFID_LOOKUP_PATH);
+
oparms = CIFS_OPARMS(cifs_sb, tcon, name, FILE_WRITE_ATTRIBUTES,
FILE_CREATE, CREATE_NOT_FILE, mode);
return smb2_compound_op(xid, tcon, cifs_sb,
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 07ba61583114..2474ac18b85e 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2419,7 +2419,8 @@ add_lease_context(struct TCP_Server_Info *server,
if (iov[num].iov_base == NULL)
return -ENOMEM;
iov[num].iov_len = server->vals->create_lease_size;
- req->RequestedOplockLevel = SMB2_OPLOCK_LEVEL_LEASE;
+ /* keep the requested oplock level in case of just setting ParentLeaseKey */
+ req->RequestedOplockLevel = *oplock;
*num_iovec = num + 1;
return 0;
}
@@ -3001,6 +3002,50 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
return rc;
}
+/*
+ * When opening a path, set ParentLeaseKey in @oparms if its parent is cached.
+ * We only have RH caching for dirs, so skip this on mkdir, unlink, rmdir.
+ *
+ * Ref: MS-SMB2 3.3.5.9 and MS-FSA 2.1.5.1
+ *
+ * Return: 0 if ParentLeaseKey was set in @oparms, -errno otherwise.
+ */
+static int check_cached_parent(struct cached_fids *cfids, struct cifs_open_parms *oparms)
+{
+ struct cached_fid *cfid;
+ const char *parent_path, *path;
+
+ if (!cfids || !oparms || !oparms->cifs_sb || !*oparms->path)
+ return -EINVAL;
+
+ if ((oparms->disposition == FILE_CREATE && oparms->create_options == CREATE_NOT_FILE) ||
+ oparms->desired_access == DELETE)
+ return -EOPNOTSUPP;
+
+ path = oparms->path;
+ parent_path = strrchr(path, CIFS_DIR_SEP(oparms->cifs_sb));
+ if (!parent_path)
+ return -ENOENT;
+
+ parent_path = kstrndup(path, parent_path - path, GFP_KERNEL);
+ if (!parent_path)
+ return -ENOMEM;
+
+ cfid = find_cached_dir(cfids, parent_path, CFID_LOOKUP_PATH);
+ kfree(parent_path);
+
+ if (!cfid)
+ return -ENOENT;
+
+ cifs_dbg(FYI, "%s: found cached parent for path: %s\n", __func__, oparms->path);
+
+ memcpy(oparms->fid->parent_lease_key, cfid->fid.lease_key, SMB2_LEASE_KEY_SIZE);
+ oparms->lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
+ close_cached_dir(cfid);
+
+ return 0;
+}
+
int
SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
struct smb_rqst *rqst, __u8 *oplock,
@@ -3077,20 +3122,35 @@ SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
iov[1].iov_len = uni_path_len;
iov[1].iov_base = path;
- if ((!server->oplocks) || (tcon->no_lease))
+ if (!server->oplocks || tcon->no_lease)
*oplock = SMB2_OPLOCK_LEVEL_NONE;
- if (!(server->capabilities & SMB2_GLOBAL_CAP_LEASING) ||
- *oplock == SMB2_OPLOCK_LEVEL_NONE)
- req->RequestedOplockLevel = *oplock;
- else if (!(server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING) &&
- (oparms->create_options & CREATE_NOT_FILE))
- req->RequestedOplockLevel = *oplock; /* no srv lease support */
- else {
- rc = add_lease_context(server, req, iov, &n_iov,
- oparms->fid->lease_key, oplock,
- oparms->fid->parent_lease_key,
- oparms->lease_flags);
+ req->RequestedOplockLevel = *oplock;
+
+ /*
+ * MS-SMB2 "Product Behavior" says Windows only checks/sets ParentLeaseKey when a lease is
+ * requested for the child/target.
+ * Practically speaking, adding the lease context with ParentLeaseKey set, even with oplock
+ * none, works fine.
+ * As a precaution, however, only set it for oplocks != none.
+ */
+ if ((server->capabilities & SMB2_GLOBAL_CAP_LEASING) &&
+ *oplock != SMB2_OPLOCK_LEVEL_NONE) {
+ rc = -EOPNOTSUPP;
+ if (server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING)
+ rc = check_cached_parent(tcon->cfids, oparms);
+
+ /*
+ * -ENOENT just means we couldn't find a cached parent, but we do have dir leasing,
+ * so try requesting a level II oplock for the child path.
+ */
+ if ((!rc || rc == -ENOENT) && *oplock == SMB2_OPLOCK_LEVEL_NONE)
+ *oplock = SMB2_OPLOCK_LEVEL_II;
+
+ if (*oplock != SMB2_OPLOCK_LEVEL_NONE)
+ rc = add_lease_context(server, req, iov, &n_iov, oparms->fid->lease_key,
+ oplock, oparms->fid->parent_lease_key,
+ oparms->lease_flags);
if (rc)
return rc;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children
2025-09-29 13:27 ` [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children Enzo Matsumiya
@ 2025-09-29 14:23 ` Steve French
2025-09-29 17:17 ` Enzo Matsumiya
1 sibling, 0 replies; 31+ messages in thread
From: Steve French @ 2025-09-29 14:23 UTC (permalink / raw)
To: Enzo Matsumiya
Cc: linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
This looks potentially very useful but would help to have some simple
repro example
On Mon, Sep 29, 2025 at 8:29 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> In SMB2_open_init(), when opening (not creating/deleting) a path, lookup
> for a cached parent and set ParentLeaseKey in lease context if found.
>
> Other:
> - set oparms->cifs_sb in open_cached_dir() as we need it in
> add_parent_lease_key(); use CIFS_OPARMS() too
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/smb/client/cached_dir.c | 42 ++++---------------
> fs/smb/client/dir.c | 26 +++---------
> fs/smb/client/smb2inode.c | 2 +
> fs/smb/client/smb2pdu.c | 86 ++++++++++++++++++++++++++++++++------
> 4 files changed, 88 insertions(+), 68 deletions(-)
>
> diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
> index ff71f2c06b72..9dd74268b2d8 100644
> --- a/fs/smb/client/cached_dir.c
> +++ b/fs/smb/client/cached_dir.c
> @@ -226,7 +226,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
> struct cached_fids *cfids;
> const char *npath;
> int retries = 0, cur_sleep = 1;
> - __le32 lease_flags = 0;
>
> if (cifs_sb->root == NULL)
> return -ENOENT;
> @@ -236,9 +235,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
>
> ses = tcon->ses;
> cfids = tcon->cfids;
> -
> if (!cfids)
> return -EOPNOTSUPP;
> +
> replay_again:
> /* reinitialize for possible replay */
> flags = 0;
> @@ -306,24 +305,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
> rc = -ENOENT;
> goto out;
> }
> - if (dentry->d_parent && server->dialect >= SMB30_PROT_ID) {
> - struct cached_fid *parent_cfid;
> -
> - spin_lock(&cfids->cfid_list_lock);
> - list_for_each_entry(parent_cfid, &cfids->entries, entry) {
> - if (parent_cfid->dentry == dentry->d_parent) {
> - if (!cfid_is_valid(parent_cfid))
> - break;
> -
> - cifs_dbg(FYI, "found a parent cached file handle\n");
> - lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
> - memcpy(pfid->parent_lease_key, parent_cfid->fid.lease_key,
> - SMB2_LEASE_KEY_SIZE);
> - break;
> - }
> - }
> - spin_unlock(&cfids->cfid_list_lock);
> - }
> }
> cfid->dentry = dentry;
> cfid->tcon = tcon;
> @@ -350,20 +331,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
> rqst[0].rq_iov = open_iov;
> rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
>
> - oparms = (struct cifs_open_parms) {
> - .tcon = tcon,
> - .path = path,
> - .create_options = cifs_create_options(cifs_sb, CREATE_NOT_FILE),
> - .desired_access = FILE_READ_DATA | FILE_READ_ATTRIBUTES |
> - FILE_READ_EA,
> - .disposition = FILE_OPEN,
> - .fid = pfid,
> - .lease_flags = lease_flags,
> - .replay = !!(retries),
> - };
> -
> - rc = SMB2_open_init(tcon, server,
> - &rqst[0], &oplock, &oparms, utf16_path);
> + oparms = CIFS_OPARMS(cifs_sb, tcon, path,
> + FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA, FILE_OPEN,
> + cifs_create_options(cifs_sb, CREATE_NOT_FILE), 0);
> + oparms.fid = pfid;
> + oparms.replay = !!retries;
> +
> + rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
> if (rc)
> goto oshr_free;
> smb2_set_next_command(tcon, &rqst[0]);
> diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> index e5372c2c799d..b60af27668bb 100644
> --- a/fs/smb/client/dir.c
> +++ b/fs/smb/client/dir.c
> @@ -189,10 +189,9 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> struct inode *newinode = NULL;
> int disposition;
> struct TCP_Server_Info *server = tcon->ses->server;
> + struct cached_fid *parent_cfid;
> struct cifs_open_parms oparms;
> - struct cached_fid *parent_cfid = NULL;
> int rdwr_for_fscache = 0;
> - __le32 lease_flags = 0;
>
> *oplock = 0;
> if (tcon->ses->server->oplocks)
> @@ -314,25 +313,11 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> if (!tcon->unix_ext && (mode & S_IWUGO) == 0)
> create_options |= CREATE_OPTION_READONLY;
>
> -
> retry_open:
> - if (tcon->cfids && direntry->d_parent && server->dialect >= SMB30_PROT_ID) {
> - parent_cfid = NULL;
> - spin_lock(&tcon->cfids->cfid_list_lock);
> - list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
> - if (parent_cfid->dentry == direntry->d_parent) {
> - if (!cfid_is_valid(parent_cfid))
> - break;
> -
> - cifs_dbg(FYI, "found a parent cached file handle\n");
> - lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
> - memcpy(fid->parent_lease_key, parent_cfid->fid.lease_key,
> - SMB2_LEASE_KEY_SIZE);
> - parent_cfid->dirents.is_valid = false;
> - break;
> - }
> - }
> - spin_unlock(&tcon->cfids->cfid_list_lock);
> + parent_cfid = find_cached_dir(tcon->cfids, direntry->d_parent, CFID_LOOKUP_DENTRY);
> + if (parent_cfid) {
> + parent_cfid->dirents.is_valid = false;
> + close_cached_dir(parent_cfid);
> }
>
> oparms = (struct cifs_open_parms) {
> @@ -343,7 +328,6 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> .disposition = disposition,
> .path = full_path,
> .fid = fid,
> - .lease_flags = lease_flags,
> .mode = mode,
> };
> rc = server->ops->open(xid, &oparms, oplock, buf);
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 8ccdd1a3ba2c..6d643b8b9547 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1120,6 +1120,8 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode,
> {
> struct cifs_open_parms oparms;
>
> + drop_cached_dir(tcon->cfids, name, CFID_LOOKUP_PATH);
> +
> oparms = CIFS_OPARMS(cifs_sb, tcon, name, FILE_WRITE_ATTRIBUTES,
> FILE_CREATE, CREATE_NOT_FILE, mode);
> return smb2_compound_op(xid, tcon, cifs_sb,
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index 07ba61583114..2474ac18b85e 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -2419,7 +2419,8 @@ add_lease_context(struct TCP_Server_Info *server,
> if (iov[num].iov_base == NULL)
> return -ENOMEM;
> iov[num].iov_len = server->vals->create_lease_size;
> - req->RequestedOplockLevel = SMB2_OPLOCK_LEVEL_LEASE;
> + /* keep the requested oplock level in case of just setting ParentLeaseKey */
> + req->RequestedOplockLevel = *oplock;
> *num_iovec = num + 1;
> return 0;
> }
> @@ -3001,6 +3002,50 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
> return rc;
> }
>
> +/*
> + * When opening a path, set ParentLeaseKey in @oparms if its parent is cached.
> + * We only have RH caching for dirs, so skip this on mkdir, unlink, rmdir.
> + *
> + * Ref: MS-SMB2 3.3.5.9 and MS-FSA 2.1.5.1
> + *
> + * Return: 0 if ParentLeaseKey was set in @oparms, -errno otherwise.
> + */
> +static int check_cached_parent(struct cached_fids *cfids, struct cifs_open_parms *oparms)
> +{
> + struct cached_fid *cfid;
> + const char *parent_path, *path;
> +
> + if (!cfids || !oparms || !oparms->cifs_sb || !*oparms->path)
> + return -EINVAL;
> +
> + if ((oparms->disposition == FILE_CREATE && oparms->create_options == CREATE_NOT_FILE) ||
> + oparms->desired_access == DELETE)
> + return -EOPNOTSUPP;
> +
> + path = oparms->path;
> + parent_path = strrchr(path, CIFS_DIR_SEP(oparms->cifs_sb));
> + if (!parent_path)
> + return -ENOENT;
> +
> + parent_path = kstrndup(path, parent_path - path, GFP_KERNEL);
> + if (!parent_path)
> + return -ENOMEM;
> +
> + cfid = find_cached_dir(cfids, parent_path, CFID_LOOKUP_PATH);
> + kfree(parent_path);
> +
> + if (!cfid)
> + return -ENOENT;
> +
> + cifs_dbg(FYI, "%s: found cached parent for path: %s\n", __func__, oparms->path);
> +
> + memcpy(oparms->fid->parent_lease_key, cfid->fid.lease_key, SMB2_LEASE_KEY_SIZE);
> + oparms->lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
> + close_cached_dir(cfid);
> +
> + return 0;
> +}
> +
> int
> SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> struct smb_rqst *rqst, __u8 *oplock,
> @@ -3077,20 +3122,35 @@ SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> iov[1].iov_len = uni_path_len;
> iov[1].iov_base = path;
>
> - if ((!server->oplocks) || (tcon->no_lease))
> + if (!server->oplocks || tcon->no_lease)
> *oplock = SMB2_OPLOCK_LEVEL_NONE;
>
> - if (!(server->capabilities & SMB2_GLOBAL_CAP_LEASING) ||
> - *oplock == SMB2_OPLOCK_LEVEL_NONE)
> - req->RequestedOplockLevel = *oplock;
> - else if (!(server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING) &&
> - (oparms->create_options & CREATE_NOT_FILE))
> - req->RequestedOplockLevel = *oplock; /* no srv lease support */
> - else {
> - rc = add_lease_context(server, req, iov, &n_iov,
> - oparms->fid->lease_key, oplock,
> - oparms->fid->parent_lease_key,
> - oparms->lease_flags);
> + req->RequestedOplockLevel = *oplock;
> +
> + /*
> + * MS-SMB2 "Product Behavior" says Windows only checks/sets ParentLeaseKey when a lease is
> + * requested for the child/target.
> + * Practically speaking, adding the lease context with ParentLeaseKey set, even with oplock
> + * none, works fine.
> + * As a precaution, however, only set it for oplocks != none.
> + */
> + if ((server->capabilities & SMB2_GLOBAL_CAP_LEASING) &&
> + *oplock != SMB2_OPLOCK_LEVEL_NONE) {
> + rc = -EOPNOTSUPP;
> + if (server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING)
> + rc = check_cached_parent(tcon->cfids, oparms);
> +
> + /*
> + * -ENOENT just means we couldn't find a cached parent, but we do have dir leasing,
> + * so try requesting a level II oplock for the child path.
> + */
> + if ((!rc || rc == -ENOENT) && *oplock == SMB2_OPLOCK_LEVEL_NONE)
> + *oplock = SMB2_OPLOCK_LEVEL_II;
> +
> + if (*oplock != SMB2_OPLOCK_LEVEL_NONE)
> + rc = add_lease_context(server, req, iov, &n_iov, oparms->fid->lease_key,
> + oplock, oparms->fid->parent_lease_key,
> + oparms->lease_flags);
> if (rc)
> return rc;
> }
> --
> 2.49.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children
2025-09-29 13:27 ` [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children Enzo Matsumiya
2025-09-29 14:23 ` Steve French
@ 2025-09-29 17:17 ` Enzo Matsumiya
1 sibling, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 17:17 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
On 09/29, Enzo Matsumiya wrote:
>In SMB2_open_init(), when opening (not creating/deleting) a path, lookup
>for a cached parent and set ParentLeaseKey in lease context if found.
>
>Other:
>- set oparms->cifs_sb in open_cached_dir() as we need it in
> add_parent_lease_key(); use CIFS_OPARMS() too
This patch is from a rebase gone wrong.
- the function above is now called check_cached_parent() instead of
add_parent_lease_key()
- the conditions to add ParentLeaseKey needs to be refreshed because of
the above
- it should've already include Bharath's fix in cifs_do_create()
(setting dirents.is_failed to true)
I'll fix these in V2.
Enzo
>Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
>---
> fs/smb/client/cached_dir.c | 42 ++++---------------
> fs/smb/client/dir.c | 26 +++---------
> fs/smb/client/smb2inode.c | 2 +
> fs/smb/client/smb2pdu.c | 86 ++++++++++++++++++++++++++++++++------
> 4 files changed, 88 insertions(+), 68 deletions(-)
>
>diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
>index ff71f2c06b72..9dd74268b2d8 100644
>--- a/fs/smb/client/cached_dir.c
>+++ b/fs/smb/client/cached_dir.c
>@@ -226,7 +226,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
> struct cached_fids *cfids;
> const char *npath;
> int retries = 0, cur_sleep = 1;
>- __le32 lease_flags = 0;
>
> if (cifs_sb->root == NULL)
> return -ENOENT;
>@@ -236,9 +235,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
>
> ses = tcon->ses;
> cfids = tcon->cfids;
>-
> if (!cfids)
> return -EOPNOTSUPP;
>+
> replay_again:
> /* reinitialize for possible replay */
> flags = 0;
>@@ -306,24 +305,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
> rc = -ENOENT;
> goto out;
> }
>- if (dentry->d_parent && server->dialect >= SMB30_PROT_ID) {
>- struct cached_fid *parent_cfid;
>-
>- spin_lock(&cfids->cfid_list_lock);
>- list_for_each_entry(parent_cfid, &cfids->entries, entry) {
>- if (parent_cfid->dentry == dentry->d_parent) {
>- if (!cfid_is_valid(parent_cfid))
>- break;
>-
>- cifs_dbg(FYI, "found a parent cached file handle\n");
>- lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
>- memcpy(pfid->parent_lease_key, parent_cfid->fid.lease_key,
>- SMB2_LEASE_KEY_SIZE);
>- break;
>- }
>- }
>- spin_unlock(&cfids->cfid_list_lock);
>- }
> }
> cfid->dentry = dentry;
> cfid->tcon = tcon;
>@@ -350,20 +331,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
> rqst[0].rq_iov = open_iov;
> rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
>
>- oparms = (struct cifs_open_parms) {
>- .tcon = tcon,
>- .path = path,
>- .create_options = cifs_create_options(cifs_sb, CREATE_NOT_FILE),
>- .desired_access = FILE_READ_DATA | FILE_READ_ATTRIBUTES |
>- FILE_READ_EA,
>- .disposition = FILE_OPEN,
>- .fid = pfid,
>- .lease_flags = lease_flags,
>- .replay = !!(retries),
>- };
>-
>- rc = SMB2_open_init(tcon, server,
>- &rqst[0], &oplock, &oparms, utf16_path);
>+ oparms = CIFS_OPARMS(cifs_sb, tcon, path,
>+ FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA, FILE_OPEN,
>+ cifs_create_options(cifs_sb, CREATE_NOT_FILE), 0);
>+ oparms.fid = pfid;
>+ oparms.replay = !!retries;
>+
>+ rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
> if (rc)
> goto oshr_free;
> smb2_set_next_command(tcon, &rqst[0]);
>diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
>index e5372c2c799d..b60af27668bb 100644
>--- a/fs/smb/client/dir.c
>+++ b/fs/smb/client/dir.c
>@@ -189,10 +189,9 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> struct inode *newinode = NULL;
> int disposition;
> struct TCP_Server_Info *server = tcon->ses->server;
>+ struct cached_fid *parent_cfid;
> struct cifs_open_parms oparms;
>- struct cached_fid *parent_cfid = NULL;
> int rdwr_for_fscache = 0;
>- __le32 lease_flags = 0;
>
> *oplock = 0;
> if (tcon->ses->server->oplocks)
>@@ -314,25 +313,11 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> if (!tcon->unix_ext && (mode & S_IWUGO) == 0)
> create_options |= CREATE_OPTION_READONLY;
>
>-
> retry_open:
>- if (tcon->cfids && direntry->d_parent && server->dialect >= SMB30_PROT_ID) {
>- parent_cfid = NULL;
>- spin_lock(&tcon->cfids->cfid_list_lock);
>- list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
>- if (parent_cfid->dentry == direntry->d_parent) {
>- if (!cfid_is_valid(parent_cfid))
>- break;
>-
>- cifs_dbg(FYI, "found a parent cached file handle\n");
>- lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
>- memcpy(fid->parent_lease_key, parent_cfid->fid.lease_key,
>- SMB2_LEASE_KEY_SIZE);
>- parent_cfid->dirents.is_valid = false;
>- break;
>- }
>- }
>- spin_unlock(&tcon->cfids->cfid_list_lock);
>+ parent_cfid = find_cached_dir(tcon->cfids, direntry->d_parent, CFID_LOOKUP_DENTRY);
>+ if (parent_cfid) {
>+ parent_cfid->dirents.is_valid = false;
>+ close_cached_dir(parent_cfid);
> }
>
> oparms = (struct cifs_open_parms) {
>@@ -343,7 +328,6 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> .disposition = disposition,
> .path = full_path,
> .fid = fid,
>- .lease_flags = lease_flags,
> .mode = mode,
> };
> rc = server->ops->open(xid, &oparms, oplock, buf);
>diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
>index 8ccdd1a3ba2c..6d643b8b9547 100644
>--- a/fs/smb/client/smb2inode.c
>+++ b/fs/smb/client/smb2inode.c
>@@ -1120,6 +1120,8 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode,
> {
> struct cifs_open_parms oparms;
>
>+ drop_cached_dir(tcon->cfids, name, CFID_LOOKUP_PATH);
>+
> oparms = CIFS_OPARMS(cifs_sb, tcon, name, FILE_WRITE_ATTRIBUTES,
> FILE_CREATE, CREATE_NOT_FILE, mode);
> return smb2_compound_op(xid, tcon, cifs_sb,
>diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
>index 07ba61583114..2474ac18b85e 100644
>--- a/fs/smb/client/smb2pdu.c
>+++ b/fs/smb/client/smb2pdu.c
>@@ -2419,7 +2419,8 @@ add_lease_context(struct TCP_Server_Info *server,
> if (iov[num].iov_base == NULL)
> return -ENOMEM;
> iov[num].iov_len = server->vals->create_lease_size;
>- req->RequestedOplockLevel = SMB2_OPLOCK_LEVEL_LEASE;
>+ /* keep the requested oplock level in case of just setting ParentLeaseKey */
>+ req->RequestedOplockLevel = *oplock;
> *num_iovec = num + 1;
> return 0;
> }
>@@ -3001,6 +3002,50 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
> return rc;
> }
>
>+/*
>+ * When opening a path, set ParentLeaseKey in @oparms if its parent is cached.
>+ * We only have RH caching for dirs, so skip this on mkdir, unlink, rmdir.
>+ *
>+ * Ref: MS-SMB2 3.3.5.9 and MS-FSA 2.1.5.1
>+ *
>+ * Return: 0 if ParentLeaseKey was set in @oparms, -errno otherwise.
>+ */
>+static int check_cached_parent(struct cached_fids *cfids, struct cifs_open_parms *oparms)
>+{
>+ struct cached_fid *cfid;
>+ const char *parent_path, *path;
>+
>+ if (!cfids || !oparms || !oparms->cifs_sb || !*oparms->path)
>+ return -EINVAL;
>+
>+ if ((oparms->disposition == FILE_CREATE && oparms->create_options == CREATE_NOT_FILE) ||
>+ oparms->desired_access == DELETE)
>+ return -EOPNOTSUPP;
>+
>+ path = oparms->path;
>+ parent_path = strrchr(path, CIFS_DIR_SEP(oparms->cifs_sb));
>+ if (!parent_path)
>+ return -ENOENT;
>+
>+ parent_path = kstrndup(path, parent_path - path, GFP_KERNEL);
>+ if (!parent_path)
>+ return -ENOMEM;
>+
>+ cfid = find_cached_dir(cfids, parent_path, CFID_LOOKUP_PATH);
>+ kfree(parent_path);
>+
>+ if (!cfid)
>+ return -ENOENT;
>+
>+ cifs_dbg(FYI, "%s: found cached parent for path: %s\n", __func__, oparms->path);
>+
>+ memcpy(oparms->fid->parent_lease_key, cfid->fid.lease_key, SMB2_LEASE_KEY_SIZE);
>+ oparms->lease_flags |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
>+ close_cached_dir(cfid);
>+
>+ return 0;
>+}
>+
> int
> SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> struct smb_rqst *rqst, __u8 *oplock,
>@@ -3077,20 +3122,35 @@ SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
> iov[1].iov_len = uni_path_len;
> iov[1].iov_base = path;
>
>- if ((!server->oplocks) || (tcon->no_lease))
>+ if (!server->oplocks || tcon->no_lease)
> *oplock = SMB2_OPLOCK_LEVEL_NONE;
>
>- if (!(server->capabilities & SMB2_GLOBAL_CAP_LEASING) ||
>- *oplock == SMB2_OPLOCK_LEVEL_NONE)
>- req->RequestedOplockLevel = *oplock;
>- else if (!(server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING) &&
>- (oparms->create_options & CREATE_NOT_FILE))
>- req->RequestedOplockLevel = *oplock; /* no srv lease support */
>- else {
>- rc = add_lease_context(server, req, iov, &n_iov,
>- oparms->fid->lease_key, oplock,
>- oparms->fid->parent_lease_key,
>- oparms->lease_flags);
>+ req->RequestedOplockLevel = *oplock;
>+
>+ /*
>+ * MS-SMB2 "Product Behavior" says Windows only checks/sets ParentLeaseKey when a lease is
>+ * requested for the child/target.
>+ * Practically speaking, adding the lease context with ParentLeaseKey set, even with oplock
>+ * none, works fine.
>+ * As a precaution, however, only set it for oplocks != none.
>+ */
>+ if ((server->capabilities & SMB2_GLOBAL_CAP_LEASING) &&
>+ *oplock != SMB2_OPLOCK_LEVEL_NONE) {
>+ rc = -EOPNOTSUPP;
>+ if (server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING)
>+ rc = check_cached_parent(tcon->cfids, oparms);
>+
>+ /*
>+ * -ENOENT just means we couldn't find a cached parent, but we do have dir leasing,
>+ * so try requesting a level II oplock for the child path.
>+ */
>+ if ((!rc || rc == -ENOENT) && *oplock == SMB2_OPLOCK_LEVEL_NONE)
>+ *oplock = SMB2_OPLOCK_LEVEL_II;
>+
>+ if (*oplock != SMB2_OPLOCK_LEVEL_NONE)
>+ rc = add_lease_context(server, req, iov, &n_iov, oparms->fid->lease_key,
>+ oplock, oparms->fid->parent_lease_key,
>+ oparms->lease_flags);
> if (rc)
> return rc;
> }
>--
>2.49.0
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 13/20] smb: client: actually use cached dirs on readdir
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (11 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 12/20] smb: client: prevent lease breaks of cached parents when opening children Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:27 ` [PATCH 14/20] smb: client: wait for concurrent caching of dirents in cifs_readdir() Enzo Matsumiya
` (7 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Currently, even when we have a valid cached dir, cifs_readdir will not
make use of it for the Find request, and will reopen a separate handle
in query_dir_first.
Fix this by setting cifsFile->fid to cfid->fid and resetting search info
parameters. Also add cifs_search_info->reset_scan to indicate
SMB2_query_directory_init to include the SMB2_RESTART_SCANS flag.
With this, we use query_dir_next directly instead of query_dir_first.
This patch also keeps the cfid reference all through cifs_readdir().
To prevent bogus/invalid usage of it, check if cfid is still valid after
each possible network call (i.e. where possible reconnects may have
happened).
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cifsglob.h | 1 +
fs/smb/client/readdir.c | 71 +++++++++++++++++++++++----------------
fs/smb/client/smb2ops.c | 2 +-
fs/smb/client/smb2pdu.c | 11 +++---
fs/smb/client/smb2proto.h | 2 +-
5 files changed, 51 insertions(+), 36 deletions(-)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 0fae95cf81c4..9a86efddc3c4 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1434,6 +1434,7 @@ struct cifs_search_info {
bool emptyDir:1;
bool unicode:1;
bool smallBuf:1; /* so we know which buf_release function to call */
+ bool restart_scan;
};
#define ACL_NO_MODE ((umode_t)(-1))
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index cc6762d950d2..0b2efd680fe6 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -344,7 +344,7 @@ cifs_std_info_to_fattr(struct cifs_fattr *fattr, FIND_FILE_STANDARD_INFO *info,
static int
_initiate_cifs_search(const unsigned int xid, struct file *file,
- const char *full_path)
+ const char *full_path, struct cached_fid *cfid)
{
__u16 search_flags;
int rc = 0;
@@ -374,7 +374,6 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
}
server = tcon->ses->server;
-
if (!server->ops->query_dir_first) {
rc = -ENOSYS;
goto error_exit;
@@ -382,6 +381,13 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
cifsFile->invalidHandle = true;
cifsFile->srch_inf.endOfSearch = false;
+ if (cfid) {
+ cifsFile->srch_inf.restart_scan = true;
+ cifsFile->srch_inf.entries_in_buffer = 0;
+ cifsFile->srch_inf.index_of_last_entry = 2;
+ cifsFile->fid.persistent_fid = cfid->fid.persistent_fid;
+ cifsFile->fid.volatile_fid = cfid->fid.volatile_fid;
+ }
cifs_dbg(FYI, "Full path: %s start at: %lld\n", full_path, file->f_pos);
@@ -406,12 +412,16 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
if (backup_cred(cifs_sb))
search_flags |= CIFS_SEARCH_BACKUP_SEARCH;
- rc = server->ops->query_dir_first(xid, tcon, full_path, cifs_sb,
- &cifsFile->fid, search_flags,
- &cifsFile->srch_inf);
-
+ if (cfid)
+ rc = server->ops->query_dir_next(xid, tcon, &cifsFile->fid, 0, &cifsFile->srch_inf);
+ else
+ rc = server->ops->query_dir_first(xid, tcon, full_path, cifs_sb, &cifsFile->fid,
+ search_flags, &cifsFile->srch_inf);
if (rc == 0) {
- cifsFile->invalidHandle = false;
+ if (cfid)
+ cifsFile->srch_inf.restart_scan = false;
+ else
+ cifsFile->invalidHandle = false;
} else if ((rc == -EOPNOTSUPP) &&
(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
cifs_autodisable_serverino(cifs_sb);
@@ -424,12 +434,12 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
static int
initiate_cifs_search(const unsigned int xid, struct file *file,
- const char *full_path)
+ const char *full_path, struct cached_fid *cfid)
{
int rc, retry_count = 0;
do {
- rc = _initiate_cifs_search(xid, file, full_path);
+ rc = _initiate_cifs_search(xid, file, full_path, cfid);
/*
* If we don't have enough credits to start reading the
* directory just try again after short wait.
@@ -683,7 +693,7 @@ static int cifs_save_resume_key(const char *current_entry,
static int
find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos,
struct file *file, const char *full_path,
- char **current_entry, int *num_to_ret)
+ char **current_entry, int *num_to_ret, struct cached_fid *cfid)
{
__u16 search_flags;
int rc = 0;
@@ -739,7 +749,7 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos,
cfile->srch_inf.srch_entries_start = NULL;
cfile->srch_inf.last_entry = NULL;
}
- rc = initiate_cifs_search(xid, file, full_path);
+ rc = initiate_cifs_search(xid, file, full_path, cfid);
if (rc) {
cifs_dbg(FYI, "error %d reinitiating a search on rewind\n",
rc);
@@ -1028,7 +1038,6 @@ static int cifs_filldir(char *find_entry, struct file *file,
&fattr, cfid, file);
}
-
int cifs_readdir(struct file *file, struct dir_context *ctx)
{
int rc = 0;
@@ -1036,7 +1045,7 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
int i;
struct tcon_link *tlink = NULL;
struct cifs_tcon *tcon;
- struct cifsFileInfo *cifsFile;
+ struct cifsFileInfo *cifsFile = NULL;
char *current_entry;
int num_to_fill = 0;
char *tmp_buf = NULL;
@@ -1095,23 +1104,22 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
}
mutex_unlock(&cfid->dirents.de_mutex);
- /* Drop the cache while calling initiate_cifs_search and
- * find_cifs_entry in case there will be reconnects during
- * query_directory.
- */
- close_cached_dir(cfid);
- cfid = NULL;
-
- cache_not_found:
+ /* keep our cfid ref, but check if still valid after network calls */
+cache_not_found:
/*
* Ensure FindFirst doesn't fail before doing filldir() for '.' and
* '..'. Otherwise we won't be able to notify VFS in case of failure.
*/
if (file->private_data == NULL) {
- rc = initiate_cifs_search(xid, file, full_path);
+ rc = initiate_cifs_search(xid, file, full_path, cfid);
cifs_dbg(FYI, "initiate cifs search rc %d\n", rc);
if (rc)
goto rddir2_exit;
+
+ if (tcon->status != TID_GOOD || (cfid && !cfid_is_valid(cfid))) {
+ rc = -ENOENT;
+ goto rddir2_exit;
+ }
}
if (!dir_emit_dots(file, ctx))
@@ -1128,15 +1136,17 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
rc = 0;
goto rddir2_exit;
}
- } /* else {
- cifsFile->invalidHandle = true;
- 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);
+ ¤t_entry, &num_to_fill, cfid);
+
+ if (tcon->status != TID_GOOD || (cfid && !cfid_is_valid(cfid))) {
+ rc = -ENOENT;
+ goto rddir2_exit;
+ }
+
if (rc) {
cifs_dbg(FYI, "fce error %d\n", rc);
goto rddir2_exit;
@@ -1204,8 +1214,11 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
kfree(tmp_buf);
rddir2_exit:
- if (cfid)
+ if (cfid) {
+ if (cifsFile)
+ cifsFile->invalidHandle = true;
close_cached_dir(cfid);
+ }
free_dentry_path(page);
free_xid(xid);
return rc;
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 39e6dc13d2da..8842315d2526 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -2397,7 +2397,7 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
rc = SMB2_query_directory_init(xid, tcon, server,
&rqst[1],
COMPOUND_FID, COMPOUND_FID,
- 0, srch_inf->info_level);
+ 0, srch_inf);
if (rc)
goto qdf_free;
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 2474ac18b85e..cc7251f4e3a6 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -5418,7 +5418,7 @@ int SMB2_query_directory_init(const unsigned int xid,
struct TCP_Server_Info *server,
struct smb_rqst *rqst,
u64 persistent_fid, u64 volatile_fid,
- int index, int info_level)
+ int index, struct cifs_search_info *search)
{
struct smb2_query_directory_req *req;
unsigned char *bufptr;
@@ -5435,7 +5435,7 @@ int SMB2_query_directory_init(const unsigned int xid,
if (rc)
return rc;
- switch (info_level) {
+ switch (search->info_level) {
case SMB_FIND_FILE_DIRECTORY_INFO:
req->FileInformationClass = FILE_DIRECTORY_INFORMATION;
break;
@@ -5449,14 +5449,15 @@ int SMB2_query_directory_init(const unsigned int xid,
req->FileInformationClass = FILE_FULL_DIRECTORY_INFORMATION;
break;
default:
- cifs_tcon_dbg(VFS, "info level %u isn't supported\n",
- info_level);
+ cifs_tcon_dbg(VFS, "info level %u isn't supported\n", search->info_level);
return -EINVAL;
}
req->FileIndex = cpu_to_le32(index);
req->PersistentFileId = persistent_fid;
req->VolatileFileId = volatile_fid;
+ if (search->restart_scan)
+ req->Flags |= SMB2_RESTART_SCANS;
len = 0x2;
bufptr = req->Buffer;
@@ -5603,7 +5604,7 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
rc = SMB2_query_directory_init(xid, tcon, server,
&rqst, persistent_fid,
volatile_fid, index,
- srch_inf->info_level);
+ srch_inf);
if (rc)
goto qdir_exit;
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index b3f1398c9f79..ac6db1f18b5b 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -231,7 +231,7 @@ extern int SMB2_query_directory_init(unsigned int xid, struct cifs_tcon *tcon,
struct TCP_Server_Info *server,
struct smb_rqst *rqst,
u64 persistent_fid, u64 volatile_fid,
- int index, int info_level);
+ int index, struct cifs_search_info *search);
extern void SMB2_query_directory_free(struct smb_rqst *rqst);
extern int SMB2_set_eof(const unsigned int xid, struct cifs_tcon *tcon,
u64 persistent_fid, u64 volatile_fid, u32 pid,
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 14/20] smb: client: wait for concurrent caching of dirents in cifs_readdir()
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (12 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 13/20] smb: client: actually use cached dirs on readdir Enzo Matsumiya
@ 2025-09-29 13:27 ` Enzo Matsumiya
2025-09-29 13:28 ` [PATCH 15/20] smb: client: remove cached_dirent->fattr Enzo Matsumiya
` (6 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:27 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
The file struct passed down to cifs_readdir() is a stack variable, which
means it makes no sense to keep/track it across a cached dir lifetime.
Instead, use it to track concurrent accesses to the same cached path,
and wait for the previous one to finish filling/emitting.
Without this patch, virtually every 'ls' will issue a Find request,
even when we have both directory and dirents cached and valid.
With this patch, the chances of cache hits increases a lot, so on
highly concurrent scenarios, the amount of network calls are
drastically reduced.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 3 ++
fs/smb/client/readdir.c | 66 +++++++++++++++++++++-----------------
2 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 9dd74268b2d8..ad455c067cba 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -595,6 +595,9 @@ static struct cached_fid *init_cached_dir(const char *path)
/* this is caller/lease ref */
kref_get(&cfid->refcount);
+ /* initial cached dirents position */
+ cfid->dirents.pos = 2;
+
return cfid;
}
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 0b2efd680fe6..903919345df1 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -860,22 +860,16 @@ static bool emit_cached_dirents(struct cached_dirents *cde,
return true;
}
-static void update_cached_dirents_count(struct cached_dirents *cde,
- struct file *file)
+static void update_cached_dirents_count(struct cached_dirents *cde)
{
- if (cde->file != file)
- 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, struct file *file)
+static void finished_cached_dirents_count(struct cached_dirents *cde, struct dir_context *ctx)
{
- if (cde->file != file)
- return;
if (cde->is_valid || cde->is_failed)
return;
if (ctx->pos != cde->pos)
@@ -884,16 +878,11 @@ static void finished_cached_dirents_count(struct cached_dirents *cde,
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 file *file)
+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->file != file)
- return;
if (cde->is_valid || cde->is_failed)
return;
if (ctx->pos != cde->pos) {
@@ -934,8 +923,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, file);
+ add_cached_dirent(&cfid->dirents, ctx, name, namelen, fattr);
mutex_unlock(&cfid->dirents.de_mutex);
}
@@ -1076,19 +1064,13 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid);
cifs_put_tlink(tlink);
- if (rc)
+retry_cached:
+ if (!cfid || !cfid_is_valid(cfid)) {
+ rc = -ENOENT;
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.file == NULL) {
- cfid->dirents.file = file;
- cfid->dirents.pos = 2;
- }
/*
* If we already have the entire directory cached then
* we can just serve the cache.
@@ -1102,10 +1084,32 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
mutex_unlock(&cfid->dirents.de_mutex);
goto rddir2_exit;
}
+
+ if (cfid->dirents.is_failed) {
+ rc = -ENOENT;
+ mutex_unlock(&cfid->dirents.de_mutex);
+ goto cache_not_found;
+ }
+
+ if (!cfid->dirents.file)
+ cfid->dirents.file = file;
+ else if (cfid->dirents.file != file)
+ rc = -EAGAIN;
mutex_unlock(&cfid->dirents.de_mutex);
+ /* someone else is filling up the dirents for this cfid, wait for them to finish */
+ if (rc == -EAGAIN) {
+ rc = 0;
+ goto retry_cached;
+ }
+
/* keep our cfid ref, but check if still valid after network calls */
cache_not_found:
+ if (rc && cfid) {
+ close_cached_dir(cfid);
+ cfid = NULL;
+ }
+
/*
* Ensure FindFirst doesn't fail before doing filldir() for '.' and
* '..'. Otherwise we won't be able to notify VFS in case of failure.
@@ -1155,7 +1159,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, file);
+ finished_cached_dirents_count(&cfid->dirents, ctx);
mutex_unlock(&cfid->dirents.de_mutex);
}
cifs_dbg(FYI, "Could not find entry\n");
@@ -1196,7 +1200,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, file);
+ update_cached_dirents_count(&cfid->dirents);
mutex_unlock(&cfid->dirents.de_mutex);
}
@@ -1215,8 +1219,12 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
rddir2_exit:
if (cfid) {
+ if (rc || cfid->dirents.is_failed || !cifsFile || cifsFile->srch_inf.endOfSearch)
+ cfid->dirents.file = NULL;
+
if (cifsFile)
cifsFile->invalidHandle = true;
+
close_cached_dir(cfid);
}
free_dentry_path(page);
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 15/20] smb: client: remove cached_dirent->fattr
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (13 preceding siblings ...)
2025-09-29 13:27 ` [PATCH 14/20] smb: client: wait for concurrent caching of dirents in cifs_readdir() Enzo Matsumiya
@ 2025-09-29 13:28 ` Enzo Matsumiya
2025-09-29 13:28 ` [PATCH 16/20] smb: client: add is_dir argument to query_path_info Enzo Matsumiya
` (5 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:28 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Replace with ->unique_id and ->dtype -- the only fields used from
cifs_fattr.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.h | 6 ++++--
fs/smb/client/readdir.c | 9 ++++-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index c45151446049..d5ad10a35ed7 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -8,13 +8,15 @@
#ifndef _CACHED_DIR_H
#define _CACHED_DIR_H
-
struct cached_dirent {
struct list_head entry;
char *name;
int namelen;
loff_t pos;
- struct cifs_fattr fattr;
+
+ /* filled from cifs_fattr */
+ u64 unique_id;
+ unsigned int dtype;
};
struct cached_dirents {
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 903919345df1..4b6e2632e8ed 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -850,9 +850,7 @@ static bool emit_cached_dirents(struct cached_dirents *cde,
* initial scan.
*/
ctx->pos = dirent->pos;
- rc = dir_emit(ctx, dirent->name, dirent->namelen,
- dirent->fattr.cf_uniqueid,
- dirent->fattr.cf_dtype);
+ rc = dir_emit(ctx, dirent->name, dirent->namelen, dirent->unique_id, dirent->dtype);
if (!rc)
return rc;
ctx->pos++;
@@ -901,9 +899,10 @@ static void add_cached_dirent(struct cached_dirents *cde, struct dir_context *ct
cde->is_failed = 1;
return;
}
- de->pos = ctx->pos;
- memcpy(&de->fattr, fattr, sizeof(struct cifs_fattr));
+ de->pos = ctx->pos;
+ de->unique_id = fattr->cf_uniqueid;
+ de->dtype = fattr->cf_dtype;
list_add_tail(&de->entry, &cde->entries);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 16/20] smb: client: add is_dir argument to query_path_info
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (14 preceding siblings ...)
2025-09-29 13:28 ` [PATCH 15/20] smb: client: remove cached_dirent->fattr Enzo Matsumiya
@ 2025-09-29 13:28 ` Enzo Matsumiya
2025-09-29 13:28 ` [PATCH 17/20] smb: client: use cached dir on queryfs/smb2_compound_op Enzo Matsumiya
` (4 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:28 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
When we have an inode on upper levels, pass is_dir down to
smb2_query_path_info() so we can lookup for a cached dir there.
Since we now have a possible recursive lookup in open_cached_dir() e.g.:
cifs_readdir
open_cached_dir
lookup_noperm_positive_unlocked
...
cifs_d_revalidate
...
smb2_query_path_info
find_cached_dir
the cfid must be added to the entries list only after dentry lookup, so
we don't hang on an infinite recursion while waiting for itself to open.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 12 ++++++++----
fs/smb/client/cifsglob.h | 2 +-
fs/smb/client/inode.c | 9 +++++++--
fs/smb/client/smb1ops.c | 4 ++--
fs/smb/client/smb2inode.c | 12 ++++++++----
fs/smb/client/smb2proto.h | 2 +-
6 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index ad455c067cba..c9e3e71e3f1f 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -277,12 +277,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
}
cfid->cfids = cfids;
- cfids->num_entries++;
- list_add(&cfid->entry, &cfids->entries);
+ cfid->tcon = tcon;
spin_unlock(&cfids->cfid_list_lock);
- pfid = &cfid->fid;
-
/*
* Skip any prefix paths in @path as lookup_noperm_positive_unlocked() ends up
* calling ->lookup() which already adds those through
@@ -310,6 +307,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
cfid->tcon = tcon;
dentry = NULL;
+ spin_lock(&cfids->cfid_list_lock);
+ cfids->num_entries++;
+ list_add(&cfid->entry, &cfids->entries);
+ spin_unlock(&cfids->cfid_list_lock);
+
+ pfid = &cfid->fid;
+
/*
* We do not hold the lock for the open because in case
* SMB2_open needs to reconnect.
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 9a86efddc3c4..08c8131c8018 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -401,7 +401,7 @@ struct smb_version_operations {
struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb,
const char *full_path,
- struct cifs_open_info_data *data);
+ struct cifs_open_info_data *data, bool is_dir);
/* query file data from the server */
int (*query_file_info)(const unsigned int xid, struct cifs_tcon *tcon,
struct cifsFileInfo *cfile, struct cifs_open_info_data *data);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index f2eff1138ed0..35c5557d5ea1 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1283,8 +1283,13 @@ static int cifs_get_fattr(struct cifs_open_info_data *data,
*/
if (!data) {
+ bool is_dir = false;
+
+ if (inode && *inode)
+ is_dir = S_ISDIR((*inode)->i_mode);
+
rc = server->ops->query_path_info(xid, tcon, cifs_sb,
- full_path, &tmp_data);
+ full_path, &tmp_data, is_dir);
data = &tmp_data;
}
@@ -1470,7 +1475,7 @@ static int smb311_posix_get_fattr(struct cifs_open_info_data *data,
*/
if (!data) {
rc = server->ops->query_path_info(xid, tcon, cifs_sb,
- full_path, &tmp_data);
+ full_path, &tmp_data, false);
data = &tmp_data;
}
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index a02d41d1ce4a..d964bc9c2823 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -543,7 +543,7 @@ static int cifs_query_path_info(const unsigned int xid,
struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb,
const char *full_path,
- struct cifs_open_info_data *data)
+ struct cifs_open_info_data *data, bool is_dir)
{
int rc = -EOPNOTSUPP;
FILE_ALL_INFO fi = {};
@@ -934,7 +934,7 @@ smb_set_file_info(struct inode *inode, const char *full_path,
if (!(tcon->ses->capabilities & CAP_NT_SMBS) &&
(!buf->CreationTime || !buf->LastAccessTime ||
!buf->LastWriteTime || !buf->ChangeTime)) {
- rc = cifs_query_path_info(xid, tcon, cifs_sb, full_path, &query_data);
+ rc = cifs_query_path_info(xid, tcon, cifs_sb, full_path, &query_data, false);
if (rc) {
if (open_file) {
cifsFileInfo_put(open_file);
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 6d643b8b9547..280aa033b06a 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -940,10 +940,9 @@ int smb2_query_path_info(const unsigned int xid,
struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb,
const char *full_path,
- struct cifs_open_info_data *data)
+ struct cifs_open_info_data *data, bool is_dir)
{
struct kvec in_iov[3], out_iov[5] = {};
- struct cached_fid *cfid = NULL;
struct cifs_open_parms oparms;
struct cifsFileInfo *cfile;
__u32 create_options = 0;
@@ -964,12 +963,17 @@ int smb2_query_path_info(const unsigned int xid,
* is fast enough (always using the compounded version).
*/
if (!tcon->posix_extensions) {
+ struct cached_fid *cfid = NULL;
+
rc = -ENOENT;
if (!*full_path)
rc = open_cached_dir(xid, tcon, full_path, cifs_sb, &cfid);
+ else if (is_dir)
+ cfid = find_cached_dir(tcon->cfids, full_path, CFID_LOOKUP_PATH);
+
+ if (cfid) {
+ rc = 0;
- /* If it is a root and its handle is cached then use it */
- if (!rc) {
if (cfid->file_all_info) {
memcpy(&data->fi, cfid->file_all_info, sizeof(data->fi));
} else {
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index ac6db1f18b5b..3e3faa7cf633 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -72,7 +72,7 @@ int smb2_query_path_info(const unsigned int xid,
struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb,
const char *full_path,
- struct cifs_open_info_data *data);
+ struct cifs_open_info_data *data, bool is_dir);
extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon,
const char *full_path, __u64 size,
struct cifs_sb_info *cifs_sb, bool set_alloc,
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 17/20] smb: client: use cached dir on queryfs/smb2_compound_op
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (15 preceding siblings ...)
2025-09-29 13:28 ` [PATCH 16/20] smb: client: add is_dir argument to query_path_info Enzo Matsumiya
@ 2025-09-29 13:28 ` Enzo Matsumiya
2025-09-29 14:26 ` Steve French
2025-09-29 13:28 ` [PATCH 18/20] smb: client: fix dentry revalidation of cached root Enzo Matsumiya
` (3 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:28 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
A dentry is passed to cifs_statfs(), so pass down d_is_dir() to
smb2_queryfs() so we can cache/reuse this dir.
Other:
- make smb2_compound_op a static function, as it's not used anywhere
else
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cifsfs.c | 2 +-
fs/smb/client/cifsglob.h | 2 +-
fs/smb/client/smb1ops.c | 2 +-
fs/smb/client/smb2ops.c | 32 +++++++++++++++++---------------
fs/smb/client/smb2proto.h | 6 ------
5 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index e1848276bab4..a2ecc5649860 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -339,7 +339,7 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
buf->f_ffree = 0; /* unlimited */
if (server->ops->queryfs)
- rc = server->ops->queryfs(xid, tcon, full_path, cifs_sb, buf);
+ rc = server->ops->queryfs(xid, tcon, full_path, cifs_sb, buf, d_is_dir(dentry));
statfs_out:
free_dentry_path(page);
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 08c8131c8018..dddac55abd6f 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -519,7 +519,7 @@ struct smb_version_operations {
__u16 net_fid, struct cifsInodeInfo *cifs_inode);
/* query remote filesystem */
int (*queryfs)(const unsigned int, struct cifs_tcon *,
- const char *, struct cifs_sb_info *, struct kstatfs *);
+ const char *, struct cifs_sb_info *, struct kstatfs *, bool);
/* send mandatory brlock to the server */
int (*mand_lock)(const unsigned int, struct cifsFileInfo *, __u64,
__u64, __u32, int, int, bool);
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index d964bc9c2823..9fa1ff9ea70d 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1105,7 +1105,7 @@ cifs_oplock_response(struct cifs_tcon *tcon, __u64 persistent_fid,
static int
cifs_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
- const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
+ const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf, bool is_dir)
{
int rc = -EOPNOTSUPP;
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 8842315d2526..f691271463b5 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1110,6 +1110,11 @@ move_smb2_ea_to_cifs(char *dst, size_t dst_size,
return (ssize_t)rc;
}
+static int smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
+ const char *path, u32 desired_access, u32 class, u32 type,
+ u32 output_len, struct kvec *rsp, int *buftype,
+ struct cifs_sb_info *cifs_sb, bool is_dir);
+
static ssize_t
smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
const unsigned char *path, const unsigned char *ea_name,
@@ -1129,7 +1134,7 @@ smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
CIFSMaxBufSize -
MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE,
- &rsp_iov, &buftype, cifs_sb);
+ &rsp_iov, &buftype, cifs_sb, false);
if (rc) {
/*
* If ea_name is NULL (listxattr) and there are no EAs,
@@ -1231,7 +1236,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
CIFSMaxBufSize -
MAX_SMB2_CREATE_RESPONSE_SIZE -
MAX_SMB2_CLOSE_RESPONSE_SIZE,
- &rsp_iov[1], &resp_buftype[1], cifs_sb);
+ &rsp_iov[1], &resp_buftype[1], cifs_sb, false);
if (rc == 0) {
rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
used_len = le32_to_cpu(rsp->OutputBufferLength);
@@ -2694,12 +2699,10 @@ bool smb2_should_replay(struct cifs_tcon *tcon,
* Passes the query info response back to the caller on success.
* Caller need to free this with free_rsp_buf().
*/
-int
-smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
- const char *path, u32 desired_access,
- u32 class, u32 type, u32 output_len,
- struct kvec *rsp, int *buftype,
- struct cifs_sb_info *cifs_sb)
+static int smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
+ const char *path, u32 desired_access, u32 class, u32 type,
+ u32 output_len, struct kvec *rsp, int *buftype,
+ struct cifs_sb_info *cifs_sb, bool is_dir)
{
struct smb2_compound_vars *vars;
struct cifs_ses *ses = tcon->ses;
@@ -2741,9 +2744,9 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
rsp_iov = vars->rsp_iov;
/*
- * We can only call this for things we know are directories.
+ * We can only open + cache paths we know are directories.
*/
- if (!strcmp(path, ""))
+ if (is_dir)
/* cfid null if open dir failed */
open_cached_dir(xid, tcon, path, cifs_sb, &cfid);
@@ -2852,7 +2855,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
static int
smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
- const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
+ const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf, bool is_dir)
{
struct smb2_query_info_rsp *rsp;
struct smb2_fs_full_size_info *info = NULL;
@@ -2860,13 +2863,12 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
int buftype = CIFS_NO_BUFFER;
int rc;
-
rc = smb2_query_info_compound(xid, tcon, path,
FILE_READ_ATTRIBUTES,
FS_FULL_SIZE_INFORMATION,
SMB2_O_INFO_FILESYSTEM,
sizeof(struct smb2_fs_full_size_info),
- &rsp_iov, &buftype, cifs_sb);
+ &rsp_iov, &buftype, cifs_sb, is_dir);
if (rc)
goto qfs_exit;
@@ -2889,7 +2891,7 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
static int
smb311_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
- const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
+ const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf, bool is_dir)
{
int rc;
__le16 *utf16_path = NULL;
@@ -2898,7 +2900,7 @@ smb311_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
struct cifs_fid fid;
if (!tcon->posix_extensions)
- return smb2_queryfs(xid, tcon, path, cifs_sb, buf);
+ return smb2_queryfs(xid, tcon, path, cifs_sb, buf, is_dir);
oparms = (struct cifs_open_parms) {
.tcon = tcon,
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index 3e3faa7cf633..99326810a159 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -299,12 +299,6 @@ extern int smb311_crypto_shash_allocate(struct TCP_Server_Info *server);
extern int smb311_update_preauth_hash(struct cifs_ses *ses,
struct TCP_Server_Info *server,
struct kvec *iov, int nvec);
-extern int smb2_query_info_compound(const unsigned int xid,
- struct cifs_tcon *tcon,
- const char *path, u32 desired_access,
- u32 class, u32 type, u32 output_len,
- struct kvec *rsp, int *buftype,
- struct cifs_sb_info *cifs_sb);
/* query path info from the server using SMB311 POSIX extensions*/
int smb311_posix_query_path_info(const unsigned int xid,
struct cifs_tcon *tcon,
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 17/20] smb: client: use cached dir on queryfs/smb2_compound_op
2025-09-29 13:28 ` [PATCH 17/20] smb: client: use cached dir on queryfs/smb2_compound_op Enzo Matsumiya
@ 2025-09-29 14:26 ` Steve French
0 siblings, 0 replies; 31+ messages in thread
From: Steve French @ 2025-09-29 14:26 UTC (permalink / raw)
To: Enzo Matsumiya
Cc: linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Would be helpful to show simple command example where this reduces ops
On Mon, Sep 29, 2025 at 8:29 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> A dentry is passed to cifs_statfs(), so pass down d_is_dir() to
> smb2_queryfs() so we can cache/reuse this dir.
>
> Other:
> - make smb2_compound_op a static function, as it's not used anywhere
> else
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/smb/client/cifsfs.c | 2 +-
> fs/smb/client/cifsglob.h | 2 +-
> fs/smb/client/smb1ops.c | 2 +-
> fs/smb/client/smb2ops.c | 32 +++++++++++++++++---------------
> fs/smb/client/smb2proto.h | 6 ------
> 5 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> index e1848276bab4..a2ecc5649860 100644
> --- a/fs/smb/client/cifsfs.c
> +++ b/fs/smb/client/cifsfs.c
> @@ -339,7 +339,7 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
> buf->f_ffree = 0; /* unlimited */
>
> if (server->ops->queryfs)
> - rc = server->ops->queryfs(xid, tcon, full_path, cifs_sb, buf);
> + rc = server->ops->queryfs(xid, tcon, full_path, cifs_sb, buf, d_is_dir(dentry));
>
> statfs_out:
> free_dentry_path(page);
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 08c8131c8018..dddac55abd6f 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -519,7 +519,7 @@ struct smb_version_operations {
> __u16 net_fid, struct cifsInodeInfo *cifs_inode);
> /* query remote filesystem */
> int (*queryfs)(const unsigned int, struct cifs_tcon *,
> - const char *, struct cifs_sb_info *, struct kstatfs *);
> + const char *, struct cifs_sb_info *, struct kstatfs *, bool);
> /* send mandatory brlock to the server */
> int (*mand_lock)(const unsigned int, struct cifsFileInfo *, __u64,
> __u64, __u32, int, int, bool);
> diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> index d964bc9c2823..9fa1ff9ea70d 100644
> --- a/fs/smb/client/smb1ops.c
> +++ b/fs/smb/client/smb1ops.c
> @@ -1105,7 +1105,7 @@ cifs_oplock_response(struct cifs_tcon *tcon, __u64 persistent_fid,
>
> static int
> cifs_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> - const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> + const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf, bool is_dir)
> {
> int rc = -EOPNOTSUPP;
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 8842315d2526..f691271463b5 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -1110,6 +1110,11 @@ move_smb2_ea_to_cifs(char *dst, size_t dst_size,
> return (ssize_t)rc;
> }
>
> +static int smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> + const char *path, u32 desired_access, u32 class, u32 type,
> + u32 output_len, struct kvec *rsp, int *buftype,
> + struct cifs_sb_info *cifs_sb, bool is_dir);
> +
> static ssize_t
> smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
> const unsigned char *path, const unsigned char *ea_name,
> @@ -1129,7 +1134,7 @@ smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon,
> CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE,
> - &rsp_iov, &buftype, cifs_sb);
> + &rsp_iov, &buftype, cifs_sb, false);
> if (rc) {
> /*
> * If ea_name is NULL (listxattr) and there are no EAs,
> @@ -1231,7 +1236,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
> CIFSMaxBufSize -
> MAX_SMB2_CREATE_RESPONSE_SIZE -
> MAX_SMB2_CLOSE_RESPONSE_SIZE,
> - &rsp_iov[1], &resp_buftype[1], cifs_sb);
> + &rsp_iov[1], &resp_buftype[1], cifs_sb, false);
> if (rc == 0) {
> rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
> used_len = le32_to_cpu(rsp->OutputBufferLength);
> @@ -2694,12 +2699,10 @@ bool smb2_should_replay(struct cifs_tcon *tcon,
> * Passes the query info response back to the caller on success.
> * Caller need to free this with free_rsp_buf().
> */
> -int
> -smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> - const char *path, u32 desired_access,
> - u32 class, u32 type, u32 output_len,
> - struct kvec *rsp, int *buftype,
> - struct cifs_sb_info *cifs_sb)
> +static int smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> + const char *path, u32 desired_access, u32 class, u32 type,
> + u32 output_len, struct kvec *rsp, int *buftype,
> + struct cifs_sb_info *cifs_sb, bool is_dir)
> {
> struct smb2_compound_vars *vars;
> struct cifs_ses *ses = tcon->ses;
> @@ -2741,9 +2744,9 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> rsp_iov = vars->rsp_iov;
>
> /*
> - * We can only call this for things we know are directories.
> + * We can only open + cache paths we know are directories.
> */
> - if (!strcmp(path, ""))
> + if (is_dir)
> /* cfid null if open dir failed */
> open_cached_dir(xid, tcon, path, cifs_sb, &cfid);
>
> @@ -2852,7 +2855,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
>
> static int
> smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> - const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> + const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf, bool is_dir)
> {
> struct smb2_query_info_rsp *rsp;
> struct smb2_fs_full_size_info *info = NULL;
> @@ -2860,13 +2863,12 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> int buftype = CIFS_NO_BUFFER;
> int rc;
>
> -
> rc = smb2_query_info_compound(xid, tcon, path,
> FILE_READ_ATTRIBUTES,
> FS_FULL_SIZE_INFORMATION,
> SMB2_O_INFO_FILESYSTEM,
> sizeof(struct smb2_fs_full_size_info),
> - &rsp_iov, &buftype, cifs_sb);
> + &rsp_iov, &buftype, cifs_sb, is_dir);
> if (rc)
> goto qfs_exit;
>
> @@ -2889,7 +2891,7 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
>
> static int
> smb311_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> - const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> + const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf, bool is_dir)
> {
> int rc;
> __le16 *utf16_path = NULL;
> @@ -2898,7 +2900,7 @@ smb311_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> struct cifs_fid fid;
>
> if (!tcon->posix_extensions)
> - return smb2_queryfs(xid, tcon, path, cifs_sb, buf);
> + return smb2_queryfs(xid, tcon, path, cifs_sb, buf, is_dir);
>
> oparms = (struct cifs_open_parms) {
> .tcon = tcon,
> diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
> index 3e3faa7cf633..99326810a159 100644
> --- a/fs/smb/client/smb2proto.h
> +++ b/fs/smb/client/smb2proto.h
> @@ -299,12 +299,6 @@ extern int smb311_crypto_shash_allocate(struct TCP_Server_Info *server);
> extern int smb311_update_preauth_hash(struct cifs_ses *ses,
> struct TCP_Server_Info *server,
> struct kvec *iov, int nvec);
> -extern int smb2_query_info_compound(const unsigned int xid,
> - struct cifs_tcon *tcon,
> - const char *path, u32 desired_access,
> - u32 class, u32 type, u32 output_len,
> - struct kvec *rsp, int *buftype,
> - struct cifs_sb_info *cifs_sb);
> /* query path info from the server using SMB311 POSIX extensions*/
> int smb311_posix_query_path_info(const unsigned int xid,
> struct cifs_tcon *tcon,
> --
> 2.49.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 18/20] smb: client: fix dentry revalidation of cached root
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (16 preceding siblings ...)
2025-09-29 13:28 ` [PATCH 17/20] smb: client: use cached dir on queryfs/smb2_compound_op Enzo Matsumiya
@ 2025-09-29 13:28 ` Enzo Matsumiya
2025-09-29 13:28 ` [PATCH 19/20] smb: client: rework cached dirs synchronization Enzo Matsumiya
` (2 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:28 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Don't check root dir dentry in cifs_dentry_needs_reval() as its inode
was created before the cfid, so the time check will fail and trigger an
unnecessary revalidation.
Also account for dir_cache_timeout in time comparison, because if we
have a cached dir, we have a lease for it, and, thus, may assume its
children are (still) valid. To confirm that, let the ac*max checks
go through for granular results.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/inode.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 35c5557d5ea1..4b817fd528f7 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2694,10 +2694,28 @@ cifs_dentry_needs_reval(struct dentry *dentry)
if (!lookupCacheEnabled)
return true;
- cfid = find_cached_dir(tcon->cfids, dentry->d_parent, CFID_LOOKUP_DENTRY);
- if (cfid) {
- close_cached_dir(cfid);
- return false;
+ if (!IS_ROOT(dentry)) {
+ cfid = find_cached_dir(tcon->cfids, dentry->d_parent, CFID_LOOKUP_DENTRY);
+ if (cfid) {
+ /*
+ * We hold a lease for the cached parent.
+ * So as long as this child is within cached dir lifetime, we don't need to
+ * revalidate it.
+ *
+ * Since cfid expiration is based on access time, use it for comparison
+ * instead of creation time.
+ */
+ if (time_before(cifs_i->time, cfid->atime - dir_cache_timeout * HZ)) {
+ close_cached_dir(cfid);
+ return true;
+ }
+
+ /*
+ * From cached dir perspective, we're done -- attr caching (ac*max) may
+ * have different requirements, so let the checks go through.
+ */
+ close_cached_dir(cfid);
+ }
}
/*
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 19/20] smb: client: rework cached dirs synchronization
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (17 preceding siblings ...)
2025-09-29 13:28 ` [PATCH 18/20] smb: client: fix dentry revalidation of cached root Enzo Matsumiya
@ 2025-09-29 13:28 ` Enzo Matsumiya
2025-09-30 19:02 ` kernel test robot
2025-09-29 13:28 ` [PATCH 20/20] smb: client: cleanup open_cached_dir() Enzo Matsumiya
2025-09-29 14:05 ` [PATCH 00/20] smb: client: cached dir fixes and improvements Steve French
20 siblings, 1 reply; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:28 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
This patch adds usage of RCU and seqlocks for cached dir (list and
entries).
Traversing the list under RCU allows faster lookups (no locks) and also
guarantees that entries being read are not gone (i.e. prevents UAF).
seqlocks provides atomicity/consistency when reading entries, allowing
callers to re-check cfid in case of write-side invalidation.
Combined with refcounting, this new approach provides safety for
callers and flexibility for future enhancements.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 269 ++++++++++++++++++++++++-------------
fs/smb/client/cached_dir.h | 19 ++-
fs/smb/client/cifs_debug.c | 5 +-
3 files changed, 193 insertions(+), 100 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index c9e3e71e3f1f..6d9a06ede476 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -16,12 +16,12 @@
#define CFID_INVALID_TIME 1
static struct cached_fid *init_cached_dir(const char *path);
-static void smb2_close_cached_fid(struct kref *ref);
+static void cfid_release_ref(struct kref *ref);
static inline void invalidate_cfid(struct cached_fid *cfid)
{
/* callers must hold the list lock and do any list operations (del/move) themselves */
- lockdep_assert_held(&cfid->cfids->cfid_list_lock);
+ lockdep_assert_held(&cfid->cfids->entries_seqlock.lock);
if (cfid_is_valid(cfid))
cfid->cfids->num_entries--;
@@ -31,20 +31,19 @@ static inline void invalidate_cfid(struct cached_fid *cfid)
cfid->atime = CFID_INVALID_TIME;
}
-static inline void drop_cfid(struct cached_fid *cfid)
+static inline void __drop_cfid(struct cached_fid *cfid)
{
struct dentry *dentry = NULL;
u64 pfid = 0, vfid = 0;
- spin_lock(&cfid->cfids->cfid_list_lock);
+ write_seqlock(&cfid->cfids->entries_seqlock);
+ write_seqlock(&cfid->seqlock);
invalidate_cfid(cfid);
-
- spin_lock(&cfid->fid_lock);
swap(cfid->dentry, dentry);
swap(cfid->fid.persistent_fid, pfid);
swap(cfid->fid.volatile_fid, vfid);
- spin_unlock(&cfid->fid_lock);
- spin_unlock(&cfid->cfids->cfid_list_lock);
+ write_sequnlock(&cfid->seqlock);
+ write_sequnlock(&cfid->cfids->entries_seqlock);
dput(dentry);
@@ -58,11 +57,18 @@ static inline void drop_cfid(struct cached_fid *cfid)
}
}
+static inline void drop_cfid(struct cached_fid *cfid)
+{
+ __drop_cfid(cfid);
+ kref_put(&cfid->refcount, cfid_release_ref);
+}
+
/*
* Find a cached dir based on @key and @mode (raw lookup).
* The only validation done here is if cfid is going down (->ctime == CFID_INVALID_TIME).
*
* If @wait_open is true, keep retrying until cfid transitions from 'opening' to valid/invalid.
+ * Will also keep retrying on list seqcount invalidations.
*
* Callers must handle any other validation as needed.
* Returned cfid, if found, has a ref taken, regardless of state.
@@ -70,51 +76,94 @@ static inline void drop_cfid(struct cached_fid *cfid)
static struct cached_fid *find_cfid(struct cached_fids *cfids, const void *key, int mode,
bool wait_open)
{
- struct cached_fid *cfid, *found;
- bool match;
+ struct cached_fid *cfid;
+ unsigned int lseq = 0;
+ int ret;
if (!cfids || !key)
return NULL;
retry_find:
- found = NULL;
+ ret = -ENOENT;
- spin_lock(&cfids->cfid_list_lock);
- list_for_each_entry(cfid, &cfids->entries, entry) {
+ rcu_read_lock();
+ lseq = read_seqbegin(&cfids->entries_seqlock);
+ list_for_each_entry_rcu(cfid, &cfids->entries, entry) {
+ if (need_seqretry(&cfids->entries_seqlock, lseq)) {
+ ret = -ECHILD;
+ break;
+ }
+
+ ret = -ENOENT;
/* don't even bother checking if it's going away */
if (cfid->ctime == CFID_INVALID_TIME)
continue;
if (mode == CFID_LOOKUP_PATH)
- match = !strcmp(cfid->path, (char *)key);
+ ret = strcmp(cfid->path, (char *)key);
if (mode == CFID_LOOKUP_DENTRY)
- match = (cfid->dentry == key);
+ ret = (cfid->dentry != key);
if (mode == CFID_LOOKUP_LEASEKEY)
- match = !memcmp(cfid->fid.lease_key, (u8 *)key, SMB2_LEASE_KEY_SIZE);
+ ret = memcmp(cfid->fid.lease_key, (u8 *)key, SMB2_LEASE_KEY_SIZE);
- if (!match)
+ if (ret) {
+ ret = -ENOENT;
continue;
+ }
+
+ if (wait_open && !cfid->ctime) {
+ unsigned int cseq = read_seqbegin(&cfid->seqlock);
+
+ if (!cfid->ctime)
+ ret = -ECHILD;
+ else if (!cfid_is_valid(cfid))
+ ret = -EINVAL;
+
+ if (read_seqretry(&cfid->seqlock, cseq) && !ret)
+ ret = -ECHILD;
- /* only get a ref here if not waiting for open */
- if (!wait_open)
- kref_get(&cfid->refcount);
- found = cfid;
+ if (ret)
+ break;
+ }
+
+ kref_get(&cfid->refcount);
break;
}
- spin_unlock(&cfids->cfid_list_lock);
- if (wait_open && found) {
- /* cfid is being opened in open_cached_dir(), retry lookup */
- if (!found->ctime)
- goto retry_find;
+ if (read_seqretry(&cfids->entries_seqlock, lseq)) {
+ if (wait_open) {
+ if (ret == -ENOENT) {
+ ret = -ECHILD;
+
+ /*
+ * Not found but caller requested wait for open.
+ * The list seqcount invalidation might have been our open, retry
+ * only once more (in case it wasn't).
+ */
+ wait_open = false;
+ }
+ }
- /* we didn't get a ref above, so get one now */
- kref_get(&found->refcount);
+ if (!ret)
+ ret = -EUCLEAN;
}
+ rcu_read_unlock();
- return found;
+ if (!ret)
+ return cfid;
+
+ if (ret == -EUCLEAN) {
+ kref_put(&cfid->refcount, cfid_release_ref);
+ ret = -ECHILD;
+ }
+
+ if (ret == -ECHILD)
+ goto retry_find;
+
+ /* -EINVAL or -ENOENT */
+ return NULL;
}
static struct dentry *
@@ -191,7 +240,7 @@ struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void *key, i
if (cfid_is_valid(cfid)) {
cfid->atime = jiffies;
} else {
- kref_put(&cfid->refcount, smb2_close_cached_fid);
+ kref_put(&cfid->refcount, cfid_release_ref);
cfid = NULL;
}
}
@@ -222,7 +271,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
u8 oplock = SMB2_OPLOCK_LEVEL_II;
struct cifs_fid *pfid;
struct dentry *dentry;
- struct cached_fid *cfid;
+ struct cached_fid __rcu *cfid;
struct cached_fids *cfids;
const char *npath;
int retries = 0, cur_sleep = 1;
@@ -251,34 +300,35 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
if (!server->ops->new_lease_key)
return -EIO;
- utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
- if (!utf16_path)
- return -ENOMEM;
-
/* find_cached_dir() already validates cfid if found, so no need to check here again */
cfid = find_cached_dir(cfids, path, CFID_LOOKUP_PATH);
if (cfid) {
- rc = 0;
- goto out;
+ *ret_cfid = cfid;
+ return 0;
}
- spin_lock(&cfids->cfid_list_lock);
+ utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
+ if (!utf16_path)
+ return -ENOMEM;
+
+ read_seqlock_excl(&cfids->entries_seqlock);
if (cfids->num_entries >= tcon->max_cached_dirs) {
- spin_unlock(&cfids->cfid_list_lock);
+ read_sequnlock_excl(&cfids->entries_seqlock);
rc = -ENOENT;
goto out;
}
+ read_sequnlock_excl(&cfids->entries_seqlock);
+ /* no ned to lock cfid or entries yet */
cfid = init_cached_dir(path);
if (!cfid) {
- spin_unlock(&cfids->cfid_list_lock);
rc = -ENOMEM;
goto out;
}
cfid->cfids = cfids;
cfid->tcon = tcon;
- spin_unlock(&cfids->cfid_list_lock);
+ pfid = &cfid->fid;
/*
* Skip any prefix paths in @path as lookup_noperm_positive_unlocked() ends up
@@ -303,16 +353,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
goto out;
}
}
- cfid->dentry = dentry;
- cfid->tcon = tcon;
- dentry = NULL;
- spin_lock(&cfids->cfid_list_lock);
+ write_seqlock(&cfids->entries_seqlock);
cfids->num_entries++;
- list_add(&cfid->entry, &cfids->entries);
- spin_unlock(&cfids->cfid_list_lock);
-
- pfid = &cfid->fid;
+ list_add_rcu(&cfid->entry, &cfids->entries);
+ write_sequnlock(&cfids->entries_seqlock);
/*
* We do not hold the lock for the open because in case
@@ -378,8 +423,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
goto oshr_free;
}
- spin_lock(&cfids->cfid_list_lock);
-
o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
oparms.fid->persistent_fid = o_rsp->PersistentFileId;
oparms.fid->volatile_fid = o_rsp->VolatileFileId;
@@ -389,30 +432,23 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
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 *)&info)) {
@@ -423,10 +459,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
}
}
- cfid->ctime = jiffies;
- cfid->atime = jiffies;
- spin_unlock(&cfids->cfid_list_lock);
- /* At this point the directory handle is fully cached */
rc = 0;
oshr_free:
SMB2_open_free(&rqst[0]);
@@ -434,16 +466,23 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
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))
+ /* cfid only becomes fully valid below, so can't use cfid_is_valid() here */
+ if (!rc && cfid->ctime == CFID_INVALID_TIME)
rc = -ENOENT;
if (rc) {
- if (cfid) {
+ dput(dentry);
+
+ if (cfid)
drop_cfid(cfid);
- kref_put(&cfid->refcount, smb2_close_cached_fid);
- }
} else {
+ /* seqlocked-write will inform concurrent lookups of opening -> open transition */
+ write_seqlock(&cfid->seqlock);
+ cfid->dentry = dentry;
+ cfid->ctime = jiffies;
+ cfid->atime = jiffies;
+ write_sequnlock(&cfid->seqlock);
+
*ret_cfid = cfid;
atomic_inc(&tcon->num_remote_opens);
}
@@ -456,13 +495,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
return rc;
}
-static void smb2_close_cached_fid(struct kref *ref)
+static void cfid_rcu_free(struct rcu_head *rcu)
{
- struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+ struct cached_fid *cfid = container_of(rcu, struct cached_fid, rcu);
struct cached_dirent *de, *q;
- drop_cfid(cfid);
-
/* Delete all cached dirent names */
list_for_each_entry_safe(de, q, &cfid->dirents.entries, entry) {
list_del(&de->entry);
@@ -477,6 +514,14 @@ static void smb2_close_cached_fid(struct kref *ref)
kfree(cfid);
}
+static void cfid_release_ref(struct kref *ref)
+{
+ struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+
+ __drop_cfid(cfid);
+ call_rcu_hurry(&cfid->rcu, cfid_rcu_free);
+}
+
bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode)
{
struct cached_fid *cfid;
@@ -497,13 +542,16 @@ bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode)
drop_cfid(cfid);
} else {
/* we're locked in smb2_is_valid_lease_break(), so can't dput/close here */
- spin_lock(&cfids->cfid_list_lock);
+ write_seqlock(&cfids->entries_seqlock);
+ write_seqlock(&cfid->seqlock);
invalidate_cfid(cfid);
- spin_unlock(&cfids->cfid_list_lock);
+ write_sequnlock(&cfid->seqlock);
+ write_sequnlock(&cfids->entries_seqlock);
+
+ /* put lookup ref */
+ kref_put(&cfid->refcount, cfid_release_ref);
}
- /* put lookup ref */
- kref_put(&cfid->refcount, smb2_close_cached_fid);
mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
return true;
@@ -511,26 +559,28 @@ bool drop_cached_dir(struct cached_fids *cfids, const void *key, int mode)
void close_cached_dir(struct cached_fid *cfid)
{
- kref_put(&cfid->refcount, smb2_close_cached_fid);
+ kref_put(&cfid->refcount, cfid_release_ref);
}
static void invalidate_all_cfids(struct cached_fids *cfids, bool closed)
{
- 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) {
+ write_seqlock(&cfids->entries_seqlock);
+ list_for_each_entry(cfid, &cfids->entries, entry) {
+ write_seqlock(&cfid->seqlock);
invalidate_cfid(cfid);
if (closed) {
cfid->fid.persistent_fid = 0;
cfid->fid.volatile_fid = 0;
}
+ write_sequnlock(&cfid->seqlock);
}
- spin_unlock(&cfids->cfid_list_lock);
+ write_sequnlock(&cfids->entries_seqlock);
/* run laundromat unconditionally now as there might have been previously queued work */
mod_delayed_work(cfid_put_wq, &cfids->laundromat_work, 0);
@@ -570,6 +620,8 @@ void invalidate_all_cached_dirs(struct cached_fids *cfids)
if (!cfids)
return;
+ synchronize_rcu();
+
invalidate_all_cfids(cfids, true);
flush_delayed_work(&cfids->laundromat_work);
}
@@ -591,7 +643,7 @@ static struct cached_fid *init_cached_dir(const char *path)
INIT_LIST_HEAD(&cfid->entry);
INIT_LIST_HEAD(&cfid->dirents.entries);
mutex_init(&cfid->dirents.de_mutex);
- spin_lock_init(&cfid->fid_lock);
+ seqlock_init(&cfid->seqlock);
/* this is our ref */
kref_init(&cfid->refcount);
@@ -607,20 +659,51 @@ static struct cached_fid *init_cached_dir(const char *path)
static void cfids_laundromat_worker(struct work_struct *work)
{
- struct cached_fids *cfids;
+ unsigned int lseq, cseq, done = 1;
struct cached_fid *cfid, *q;
+ struct cached_fids *cfids;
LIST_HEAD(entry);
cfids = container_of(work, struct cached_fids, laundromat_work.work);
- spin_lock(&cfids->cfid_list_lock);
+ rcu_read_lock();
+ /* RCU-seqcounted read first so we avoid unnecessary seqcount invalidations */
+ lseq = read_seqbegin(&cfids->entries_seqlock);
+ list_for_each_entry_rcu(cfid, &cfids->entries, entry) {
+ if (read_seqretry(&cfids->entries_seqlock, lseq)) {
+ done = 0;
+ break;
+ }
+
+ cseq = read_seqbegin(&cfid->seqlock);
+ if (cfid_expired(cfid))
+ done = 0;
+
+ if (read_seqretry(&cfid->seqlock, cseq) || !done) {
+ done = 0;
+ break;
+ }
+ }
+
+ /* no list invalidations and no invalidated/expired entries, requeue */
+ if (!read_seqretry(&cfids->entries_seqlock, lseq) && done) {
+ rcu_read_unlock();
+ goto requeue;
+ }
+ rcu_read_unlock();
+
+ /* now we know something was invalidated in the above read, do a write-locked run */
+ write_seqlock(&cfids->entries_seqlock);
list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
+ write_seqlock(&cfid->seqlock);
if (cfid_expired(cfid)) {
+ //raw_write_seqcount_barrier(&cfids->entries_seqlock.seqcount);
invalidate_cfid(cfid);
list_move(&cfid->entry, &entry);
}
+ write_sequnlock(&cfid->seqlock);
}
- spin_unlock(&cfids->cfid_list_lock);
+ write_sequnlock(&cfids->entries_seqlock);
list_for_each_entry_safe(cfid, q, &entry, entry) {
list_del(&cfid->entry);
@@ -636,9 +719,8 @@ static void cfids_laundromat_worker(struct work_struct *work)
* No risk for a double list_del() here because cfid is only on this list now.
*/
drop_cfid(cfid);
- kref_put(&cfid->refcount, smb2_close_cached_fid);
}
-
+requeue:
queue_delayed_work(cfid_put_wq, &cfids->laundromat_work, dir_cache_timeout * HZ);
}
@@ -649,7 +731,8 @@ struct cached_fids *init_cached_dirs(void)
cfids = kzalloc(sizeof(*cfids), GFP_KERNEL);
if (!cfids)
return NULL;
- spin_lock_init(&cfids->cfid_list_lock);
+
+ seqlock_init(&cfids->entries_seqlock);
INIT_LIST_HEAD(&cfids->entries);
INIT_DELAYED_WORK(&cfids->laundromat_work, cfids_laundromat_worker);
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index d5ad10a35ed7..77c292399978 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -8,6 +8,8 @@
#ifndef _CACHED_DIR_H
#define _CACHED_DIR_H
+#include <linux/seqlock.h>
+
struct cached_dirent {
struct list_head entry;
char *name;
@@ -33,13 +35,19 @@ struct cached_dirents {
struct cached_fid {
struct list_head entry;
+ struct rcu_head rcu;
+ /*
+ * ->seqlock must be used:
+ * - write-locked when updating
+ * - rcu_read_lock() + seqcounted on reads
+ */
+ seqlock_t seqlock;
struct cached_fids *cfids;
const char *path;
unsigned long ctime; /* (jiffies) creation time, when cfid was created (cached) */
unsigned long atime; /* (jiffies) access time, when it was last used */
struct kref refcount;
struct cifs_fid fid;
- spinlock_t fid_lock;
struct cifs_tcon *tcon;
struct dentry *dentry;
struct smb2_file_all_info *file_all_info;
@@ -48,11 +56,12 @@ struct cached_fid {
/* default MAX_CACHED_FIDS is 16 */
struct cached_fids {
- /* Must be held when:
- * - accessing the cfids->entries list
- * - accessing cfids->num_entries
+ /*
+ * ->entries_seqlock must be used when accessing ->entries or ->num_entries:
+ * - write-locked when updating
+ * - rcu_read_lock() + seqcounted on reads
*/
- spinlock_t cfid_list_lock;
+ seqlock_t entries_seqlock;
int num_entries;
struct list_head entries;
struct delayed_work laundromat_work;
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index bb27b9c97724..72e63db75403 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -306,7 +306,8 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
cfids = tcon->cfids;
if (!cfids)
continue;
- spin_lock(&cfids->cfid_list_lock); /* check lock ordering */
+
+ read_seqlock_excl(&cfids->entries_seqlock);
seq_printf(m, "Num entries: %d\n", cfids->num_entries);
list_for_each_entry(cfid, &cfids->entries, entry) {
seq_printf(m, "0x%x 0x%llx 0x%llx %s",
@@ -320,7 +321,7 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
seq_printf(m, ", valid dirents");
seq_printf(m, "\n");
}
- spin_unlock(&cfids->cfid_list_lock);
+ read_sequnlock_excl(&cfids->entries_seqlock);
}
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 19/20] smb: client: rework cached dirs synchronization
2025-09-29 13:28 ` [PATCH 19/20] smb: client: rework cached dirs synchronization Enzo Matsumiya
@ 2025-09-30 19:02 ` kernel test robot
0 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2025-09-30 19:02 UTC (permalink / raw)
To: Enzo Matsumiya, linux-cifs
Cc: oe-kbuild-all, smfrench, pc, ronniesahlberg, sprasad, tom,
bharathsm, henrique.carvalho
Hi Enzo,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.17]
[also build test WARNING on linus/master]
[cannot apply to cifs/for-next next-20250929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Enzo-Matsumiya/smb-client-remove-cfids_invalidation_worker/20250929-213155
base: v6.17
patch link: https://lore.kernel.org/r/20250929132805.220558-20-ematsumiya%40suse.de
patch subject: [PATCH 19/20] smb: client: rework cached dirs synchronization
config: i386-randconfig-061-20250930 (https://download.01.org/0day-ci/archive/20251001/202510010208.20c25Gzw-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251001/202510010208.20c25Gzw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510010208.20c25Gzw-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> fs/smb/client/cached_dir.c:304:14: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cached_fid [noderef] __rcu *[assigned] cfid @@ got struct cached_fid * @@
fs/smb/client/cached_dir.c:304:14: sparse: expected struct cached_fid [noderef] __rcu *[assigned] cfid
fs/smb/client/cached_dir.c:304:14: sparse: got struct cached_fid *
>> fs/smb/client/cached_dir.c:306:27: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cached_fid * @@ got struct cached_fid [noderef] __rcu *[assigned] cfid @@
fs/smb/client/cached_dir.c:306:27: sparse: expected struct cached_fid *
fs/smb/client/cached_dir.c:306:27: sparse: got struct cached_fid [noderef] __rcu *[assigned] cfid
fs/smb/client/cached_dir.c:323:14: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cached_fid [noderef] __rcu *[assigned] cfid @@ got struct cached_fid * @@
fs/smb/client/cached_dir.c:323:14: sparse: expected struct cached_fid [noderef] __rcu *[assigned] cfid
fs/smb/client/cached_dir.c:323:14: sparse: got struct cached_fid *
>> fs/smb/client/cached_dir.c:331:14: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cifs_fid *pfid @@ got struct cifs_fid [noderef] __rcu * @@
fs/smb/client/cached_dir.c:331:14: sparse: expected struct cifs_fid *pfid
fs/smb/client/cached_dir.c:331:14: sparse: got struct cifs_fid [noderef] __rcu *
>> fs/smb/client/cached_dir.c:359:23: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct list_head *new @@ got struct list_head [noderef] __rcu * @@
fs/smb/client/cached_dir.c:359:23: sparse: expected struct list_head *new
fs/smb/client/cached_dir.c:359:23: sparse: got struct list_head [noderef] __rcu *
>> fs/smb/client/cached_dir.c:477:35: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cached_fid *cfid @@ got struct cached_fid [noderef] __rcu *[assigned] cfid @@
fs/smb/client/cached_dir.c:477:35: sparse: expected struct cached_fid *cfid
fs/smb/client/cached_dir.c:477:35: sparse: got struct cached_fid [noderef] __rcu *[assigned] cfid
>> fs/smb/client/cached_dir.c:480:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct seqlock_t [usertype] *sl @@ got struct seqlock_t [noderef] __rcu * @@
fs/smb/client/cached_dir.c:480:32: sparse: expected struct seqlock_t [usertype] *sl
fs/smb/client/cached_dir.c:480:32: sparse: got struct seqlock_t [noderef] __rcu *
fs/smb/client/cached_dir.c:484:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct seqlock_t [usertype] *sl @@ got struct seqlock_t [noderef] __rcu * @@
fs/smb/client/cached_dir.c:484:34: sparse: expected struct seqlock_t [usertype] *sl
fs/smb/client/cached_dir.c:484:34: sparse: got struct seqlock_t [noderef] __rcu *
fs/smb/client/cached_dir.c:486:27: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cached_fid * @@ got struct cached_fid [noderef] __rcu *[assigned] cfid @@
fs/smb/client/cached_dir.c:486:27: sparse: expected struct cached_fid *
fs/smb/client/cached_dir.c:486:27: sparse: got struct cached_fid [noderef] __rcu *[assigned] cfid
>> fs/smb/client/cached_dir.c:329:9: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:330:9: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:455:17: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:456:22: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:470:20: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:481:17: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:482:17: sparse: sparse: dereference of noderef expression
fs/smb/client/cached_dir.c:483:17: sparse: sparse: dereference of noderef expression
vim +304 fs/smb/client/cached_dir.c
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 250
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 251 /*
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 252 * Open the and cache a directory handle.
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 253 * If error then *cfid is not initialized.
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 254 */
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 255 int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 256 struct cifs_sb_info *cifs_sb, struct cached_fid **ret_cfid)
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 257 {
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 258 struct cifs_ses *ses;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 259 struct TCP_Server_Info *server;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 260 struct cifs_open_parms oparms;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 261 struct smb2_create_rsp *o_rsp = NULL;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 262 struct smb2_query_info_rsp *qi_rsp = NULL;
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 263 struct smb2_file_all_info info;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 264 int resp_buftype[2];
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 265 struct smb_rqst rqst[2];
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 266 struct kvec rsp_iov[2];
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 267 struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 268 struct kvec qi_iov[1];
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 269 int rc, flags = 0;
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 270 __le16 *utf16_path = NULL;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 271 u8 oplock = SMB2_OPLOCK_LEVEL_II;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 272 struct cifs_fid *pfid;
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 273 struct dentry *dentry;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 274 struct cached_fid __rcu *cfid;
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 275 struct cached_fids *cfids;
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 276 const char *npath;
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 277 int retries = 0, cur_sleep = 1;
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 278
f6e88838400d88 fs/smb/client/cached_dir.c Henrique Carvalho 2024-11-22 279 if (cifs_sb->root == NULL)
f6e88838400d88 fs/smb/client/cached_dir.c Henrique Carvalho 2024-11-22 280 return -ENOENT;
f6e88838400d88 fs/smb/client/cached_dir.c Henrique Carvalho 2024-11-22 281
f6e88838400d88 fs/smb/client/cached_dir.c Henrique Carvalho 2024-11-22 282 if (tcon == NULL)
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 283 return -EOPNOTSUPP;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 284
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 285 ses = tcon->ses;
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 286 cfids = tcon->cfids;
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 287 if (!cfids)
f6e88838400d88 fs/smb/client/cached_dir.c Henrique Carvalho 2024-11-22 288 return -EOPNOTSUPP;
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 289
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 290 replay_again:
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 291 /* reinitialize for possible replay */
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 292 flags = 0;
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 293 oplock = SMB2_OPLOCK_LEVEL_II;
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 294 dentry = NULL;
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 295 cfid = NULL;
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 296 *ret_cfid = NULL;
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 297 memset(&info, 0, sizeof(info));
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 298 server = cifs_pick_channel(ses);
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 299
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 300 if (!server->ops->new_lease_key)
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 301 return -EIO;
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 302
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 303 /* find_cached_dir() already validates cfid if found, so no need to check here again */
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @304 cfid = find_cached_dir(cfids, path, CFID_LOOKUP_PATH);
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 305 if (cfid) {
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @306 *ret_cfid = cfid;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 307 return 0;
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 308 }
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 309
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 310 utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 311 if (!utf16_path)
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 312 return -ENOMEM;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 313
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 314 read_seqlock_excl(&cfids->entries_seqlock);
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 315 if (cfids->num_entries >= tcon->max_cached_dirs) {
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 316 read_sequnlock_excl(&cfids->entries_seqlock);
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 317 rc = -ENOENT;
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 318 goto out;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 319 }
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 320 read_sequnlock_excl(&cfids->entries_seqlock);
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 321
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 322 /* no ned to lock cfid or entries yet */
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @323 cfid = init_cached_dir(path);
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 324 if (!cfid) {
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 325 rc = -ENOMEM;
7d24f0ff5dad1f fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 326 goto out;
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 327 }
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 328
1d4a92c061c478 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @329 cfid->cfids = cfids;
65e58ef1dafb0c fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 330 cfid->tcon = tcon;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @331 pfid = &cfid->fid;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 332
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 333 /*
fa6fe07d153636 fs/smb/client/cached_dir.c NeilBrown 2025-03-19 334 * Skip any prefix paths in @path as lookup_noperm_positive_unlocked() ends up
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 335 * calling ->lookup() which already adds those through
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 336 * build_path_from_dentry(). Also, do it earlier as we might reconnect
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 337 * below when trying to send compounded request and then potentially
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 338 * having a different prefix path (e.g. after DFS failover).
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 339 */
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 340 npath = path_no_prefix(cifs_sb, path);
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 341 if (IS_ERR(npath)) {
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 342 rc = PTR_ERR(npath);
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 343 goto out;
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 344 }
be4fde79812f02 fs/cifs/cached_dir.c Paulo Alcantara 2023-03-24 345
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 346 if (!npath[0]) {
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 347 dentry = dget(cifs_sb->root);
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 348 } else {
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 349 dentry = path_to_dentry(cifs_sb, npath);
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 350 if (IS_ERR(dentry)) {
a43689b1b41ccc fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 351 dentry = NULL;
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 352 rc = -ENOENT;
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 353 goto out;
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 354 }
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 355 }
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 356
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 357 write_seqlock(&cfids->entries_seqlock);
65e58ef1dafb0c fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 358 cfids->num_entries++;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @359 list_add_rcu(&cfid->entry, &cfids->entries);
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 360 write_sequnlock(&cfids->entries_seqlock);
65e58ef1dafb0c fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 361
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 362 /*
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 363 * We do not hold the lock for the open because in case
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 364 * SMB2_open needs to reconnect.
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 365 * This is safe because no other thread will be able to get a ref
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 366 * to the cfid until we have finished opening the file and (possibly)
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 367 * acquired a lease.
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 368 */
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 369 if (smb3_encryption_required(tcon))
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 370 flags |= CIFS_TRANSFORM_REQ;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 371
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 372 server->ops->new_lease_key(pfid);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 373
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 374 memset(rqst, 0, sizeof(rqst));
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 375 resp_buftype[0] = resp_buftype[1] = CIFS_NO_BUFFER;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 376 memset(rsp_iov, 0, sizeof(rsp_iov));
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 377
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 378 /* Open */
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 379 memset(&open_iov, 0, sizeof(open_iov));
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 380 rqst[0].rq_iov = open_iov;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 381 rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 382
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 383 oparms = CIFS_OPARMS(cifs_sb, tcon, path,
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 384 FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA, FILE_OPEN,
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 385 cifs_create_options(cifs_sb, CREATE_NOT_FILE), 0);
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 386 oparms.fid = pfid;
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 387 oparms.replay = !!retries;
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 388
e28638bb28ed69 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 389 rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 390 if (rc)
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 391 goto oshr_free;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 392 smb2_set_next_command(tcon, &rqst[0]);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 393
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 394 memset(&qi_iov, 0, sizeof(qi_iov));
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 395 rqst[1].rq_iov = qi_iov;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 396 rqst[1].rq_nvec = 1;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 397
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 398 rc = SMB2_query_info_init(tcon, server,
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 399 &rqst[1], COMPOUND_FID,
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 400 COMPOUND_FID, FILE_ALL_INFORMATION,
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 401 SMB2_O_INFO_FILE, 0,
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 402 sizeof(struct smb2_file_all_info) +
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 403 PATH_MAX * 2, 0, NULL);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 404 if (rc)
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 405 goto oshr_free;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 406
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 407 smb2_set_related(&rqst[1]);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 408
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 409 if (retries) {
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 410 smb2_set_replay(server, &rqst[0]);
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 411 smb2_set_replay(server, &rqst[1]);
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 412 }
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 413
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 414 rc = compound_send_recv(xid, ses, server,
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 415 flags, 2, rqst,
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 416 resp_buftype, rsp_iov);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 417 if (rc) {
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 418 if (rc == -EREMCHG) {
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 419 tcon->need_reconnect = true;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 420 pr_warn_once("server share %s deleted\n",
68e14569d7e5a1 fs/cifs/cached_dir.c Steve French 2022-09-21 421 tcon->tree_name);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 422 }
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 423 goto oshr_free;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 424 }
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 425
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 426 o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 427 oparms.fid->persistent_fid = o_rsp->PersistentFileId;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 428 oparms.fid->volatile_fid = o_rsp->VolatileFileId;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 429 #ifdef CONFIG_CIFS_DEBUG2
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 430 oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 431 #endif /* CIFS_DEBUG2 */
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 432
af1689a9b7701d fs/smb/client/cached_dir.c Paulo Alcantara 2023-12-11 433
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 434 if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
af1689a9b7701d fs/smb/client/cached_dir.c Paulo Alcantara 2023-12-11 435 rc = -EINVAL;
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 436 goto oshr_free;
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 437 }
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 438
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 439 rc = smb2_parse_contexts(server, rsp_iov, &oparms.fid->epoch, oparms.fid->lease_key,
af1689a9b7701d fs/smb/client/cached_dir.c Paulo Alcantara 2023-12-11 440 &oplock, NULL, NULL);
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 441 if (rc)
af1689a9b7701d fs/smb/client/cached_dir.c Paulo Alcantara 2023-12-11 442 goto oshr_free;
af1689a9b7701d fs/smb/client/cached_dir.c Paulo Alcantara 2023-12-11 443
af1689a9b7701d fs/smb/client/cached_dir.c Paulo Alcantara 2023-12-11 444 rc = -EINVAL;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 445 if (!(oplock & SMB2_LEASE_READ_CACHING_HE))
66d45ca1350a3b fs/cifs/cached_dir.c Ronnie Sahlberg 2023-02-17 446 goto oshr_free;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 447
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 448 qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 449 if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
ebe98f1447bbcc fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-06 450 goto oshr_free;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 451
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 452 if (!smb2_validate_and_copy_iov(le16_to_cpu(qi_rsp->OutputBufferOffset),
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 453 sizeof(struct smb2_file_all_info), &rsp_iov[1],
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 454 sizeof(struct smb2_file_all_info), (char *)&info)) {
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 455 cfid->file_all_info = kmemdup(&info, sizeof(info), GFP_ATOMIC);
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 456 if (!cfid->file_all_info) {
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 457 rc = -ENOMEM;
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 458 goto out;
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 459 }
ff1e8e71b1ac5d fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 460 }
e4029e072673d8 fs/cifs/cached_dir.c Ronnie Sahlberg 2022-10-12 461
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 462 rc = 0;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 463 oshr_free:
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 464 SMB2_open_free(&rqst[0]);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 465 SMB2_query_info_free(&rqst[1]);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 466 free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 467 free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
a9685b409a03b7 fs/smb/client/cached_dir.c Paul Aurich 2024-11-18 468 out:
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 469 /* cfid only becomes fully valid below, so can't use cfid_is_valid() here */
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 470 if (!rc && cfid->ctime == CFID_INVALID_TIME)
a43689b1b41ccc fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 471 rc = -ENOENT;
a9685b409a03b7 fs/smb/client/cached_dir.c Paul Aurich 2024-11-18 472
a43689b1b41ccc fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 473 if (rc) {
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 474 dput(dentry);
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 475
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 476 if (cfid)
a43689b1b41ccc fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @477 drop_cfid(cfid);
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 478 } else {
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 479 /* seqlocked-write will inform concurrent lookups of opening -> open transition */
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 @480 write_seqlock(&cfid->seqlock);
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 481 cfid->dentry = dentry;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 482 cfid->ctime = jiffies;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 483 cfid->atime = jiffies;
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 484 write_sequnlock(&cfid->seqlock);
9faca5c38f7815 fs/smb/client/cached_dir.c Enzo Matsumiya 2025-09-29 485
a63ec83c462b5b fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-11 486 *ret_cfid = cfid;
66d45ca1350a3b fs/cifs/cached_dir.c Ronnie Sahlberg 2023-02-17 487 atomic_inc(&tcon->num_remote_opens);
66d45ca1350a3b fs/cifs/cached_dir.c Ronnie Sahlberg 2023-02-17 488 }
5c86919455c1ed fs/smb/client/cached_dir.c Paulo Alcantara 2023-10-30 489 kfree(utf16_path);
64cc377b7628b8 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 490
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 491 if (is_replayable_error(rc) &&
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 492 smb2_should_replay(tcon, &retries, &cur_sleep))
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 493 goto replay_again;
4f1fffa2376922 fs/smb/client/cached_dir.c Shyam Prasad N 2024-01-21 494
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 495 return rc;
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 496 }
05b98fd2da6bdf fs/cifs/cached_dir.c Ronnie Sahlberg 2022-08-10 497
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 20/20] smb: client: cleanup open_cached_dir()
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (18 preceding siblings ...)
2025-09-29 13:28 ` [PATCH 19/20] smb: client: rework cached dirs synchronization Enzo Matsumiya
@ 2025-09-29 13:28 ` Enzo Matsumiya
2025-09-29 14:05 ` [PATCH 00/20] smb: client: cached dir fixes and improvements Steve French
20 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 13:28 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
A bit of refactoring, and merge path_no_prefix() into path_to_dentry().
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/cached_dir.c | 133 ++++++++++++++++---------------------
1 file changed, 56 insertions(+), 77 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 6d9a06ede476..45be551aad5d 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -166,13 +166,33 @@ static struct cached_fid *find_cfid(struct cached_fids *cfids, const void *key,
return NULL;
}
-static struct dentry *
-path_to_dentry(struct cifs_sb_info *cifs_sb, const char *path)
+/*
+ * Skip any prefix paths in @path as lookup_noperm_positive_unlocked() ends up calling ->lookup()
+ * which already adds those through build_path_from_dentry().
+ *
+ * Also, this should be called before sending a network request as we might reconnect and
+ * potentially end up having a different prefix path (e.g. after DFS failover).
+ *
+ * Callers must dput() returned dentry if !IS_ERR().
+ */
+static struct dentry *path_to_dentry(struct cifs_sb_info *cifs_sb, const char *path)
{
struct dentry *dentry;
const char *s, *p;
char sep;
+ if (!*path)
+ return dget(cifs_sb->root);
+
+ if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_USE_PREFIX_PATH) && cifs_sb->prepath) {
+ size_t len = strlen(cifs_sb->prepath) + 1;
+
+ if (unlikely(len > strlen(path)))
+ return ERR_PTR(-EINVAL);
+
+ path += len;
+ }
+
sep = CIFS_DIR_SEP(cifs_sb);
dentry = dget(cifs_sb->root);
s = path;
@@ -197,29 +217,12 @@ 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));
- return dentry;
-}
-
-static const char *path_no_prefix(struct cifs_sb_info *cifs_sb,
- const char *path)
-{
- size_t len = 0;
-
- if (!*path)
- return path;
- if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_USE_PREFIX_PATH) &&
- cifs_sb->prepath) {
- len = strlen(cifs_sb->prepath) + 1;
- if (unlikely(len > strlen(path)))
- return ERR_PTR(-EINVAL);
- }
- return path + len;
+ return dentry;
}
/*
@@ -255,31 +258,29 @@ struct cached_fid *find_cached_dir(struct cached_fids *cfids, const void *key, i
int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
struct cifs_sb_info *cifs_sb, struct cached_fid **ret_cfid)
{
- struct cifs_ses *ses;
+ int rc, flags = 0, retries = 0, cur_sleep = 1;
+ struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
+ struct smb2_query_info_rsp *qi_rsp = NULL;
struct TCP_Server_Info *server;
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 smb2_create_rsp *o_rsp = NULL;
+ struct cached_fid __rcu *cfid;
+ struct cached_fids *cfids;
struct smb_rqst rqst[2];
struct kvec rsp_iov[2];
- struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
+ struct cifs_fid *pfid;
+ struct dentry *dentry;
struct kvec qi_iov[1];
- int rc, flags = 0;
+ struct cifs_ses *ses;
+ int resp_buftype[2];
__le16 *utf16_path = NULL;
u8 oplock = SMB2_OPLOCK_LEVEL_II;
- struct cifs_fid *pfid;
- struct dentry *dentry;
- struct cached_fid __rcu *cfid;
- struct cached_fids *cfids;
- const char *npath;
- int retries = 0, cur_sleep = 1;
- if (cifs_sb->root == NULL)
+ if (!cifs_sb->root)
return -ENOENT;
- if (tcon == NULL)
+ if (!tcon)
return -EOPNOTSUPP;
ses = tcon->ses;
@@ -307,10 +308,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
return 0;
}
- utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
- if (!utf16_path)
- return -ENOMEM;
-
read_seqlock_excl(&cfids->entries_seqlock);
if (cfids->num_entries >= tcon->max_cached_dirs) {
read_sequnlock_excl(&cfids->entries_seqlock);
@@ -330,30 +327,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
cfid->tcon = tcon;
pfid = &cfid->fid;
- /*
- * Skip any prefix paths in @path as lookup_noperm_positive_unlocked() ends up
- * calling ->lookup() which already adds those through
- * build_path_from_dentry(). Also, do it earlier as we might reconnect
- * below when trying to send compounded request and then potentially
- * having a different prefix path (e.g. after DFS failover).
- */
- npath = path_no_prefix(cifs_sb, path);
- if (IS_ERR(npath)) {
- rc = PTR_ERR(npath);
+ dentry = path_to_dentry(cifs_sb, path);
+ if (IS_ERR(dentry)) {
+ rc = PTR_ERR(dentry);
+ dentry = NULL;
goto out;
}
- if (!npath[0]) {
- dentry = dget(cifs_sb->root);
- } else {
- dentry = path_to_dentry(cifs_sb, npath);
- if (IS_ERR(dentry)) {
- dentry = NULL;
- rc = -ENOENT;
- goto out;
- }
- }
-
write_seqlock(&cfids->entries_seqlock);
cfids->num_entries++;
list_add_rcu(&cfid->entry, &cfids->entries);
@@ -386,21 +366,26 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
oparms.fid = pfid;
oparms.replay = !!retries;
+ utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
+ if (!utf16_path) {
+ rc = -ENOMEM;
+ goto oshr_free;
+ }
+
rc = SMB2_open_init(tcon, server, &rqst[0], &oplock, &oparms, utf16_path);
+ kfree(utf16_path);
+
if (rc)
goto oshr_free;
- smb2_set_next_command(tcon, &rqst[0]);
+ smb2_set_next_command(tcon, &rqst[0]);
memset(&qi_iov, 0, sizeof(qi_iov));
rqst[1].rq_iov = qi_iov;
rqst[1].rq_nvec = 1;
- rc = SMB2_query_info_init(tcon, server,
- &rqst[1], COMPOUND_FID,
- COMPOUND_FID, FILE_ALL_INFORMATION,
- SMB2_O_INFO_FILE, 0,
- sizeof(struct smb2_file_all_info) +
- PATH_MAX * 2, 0, NULL);
+ rc = SMB2_query_info_init(tcon, server, &rqst[1], COMPOUND_FID, COMPOUND_FID,
+ FILE_ALL_INFORMATION, SMB2_O_INFO_FILE, 0,
+ sizeof(struct smb2_file_all_info) + PATH_MAX * 2, 0, NULL);
if (rc)
goto oshr_free;
@@ -411,14 +396,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
smb2_set_replay(server, &rqst[1]);
}
- rc = compound_send_recv(xid, ses, server,
- flags, 2, rqst,
- resp_buftype, rsp_iov);
+ rc = compound_send_recv(xid, ses, server, flags, 2, rqst, resp_buftype, rsp_iov);
if (rc) {
if (rc == -EREMCHG) {
tcon->need_reconnect = true;
- pr_warn_once("server share %s deleted\n",
- tcon->tree_name);
+ pr_warn_once("server share %s deleted\n", tcon->tree_name);
}
goto oshr_free;
}
@@ -430,7 +412,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
#endif /* CIFS_DEBUG2 */
-
if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
rc = -EINVAL;
goto oshr_free;
@@ -449,9 +430,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
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 *)&info)) {
+ if (!smb2_validate_and_copy_iov(le16_to_cpu(qi_rsp->OutputBufferOffset), sizeof(info),
+ &rsp_iov[1], sizeof(info), (char *)&info)) {
cfid->file_all_info = kmemdup(&info, sizeof(info), GFP_ATOMIC);
if (!cfid->file_all_info) {
rc = -ENOMEM;
@@ -486,7 +466,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,
*ret_cfid = cfid;
atomic_inc(&tcon->num_remote_opens);
}
- kfree(utf16_path);
if (is_replayable_error(rc) &&
smb2_should_replay(tcon, &retries, &cur_sleep))
--
2.49.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 00/20] smb: client: cached dir fixes and improvements
2025-09-29 13:27 [PATCH 00/20] smb: client: cached dir fixes and improvements Enzo Matsumiya
` (19 preceding siblings ...)
2025-09-29 13:28 ` [PATCH 20/20] smb: client: cleanup open_cached_dir() Enzo Matsumiya
@ 2025-09-29 14:05 ` Steve French
2025-09-29 14:44 ` Enzo Matsumiya
20 siblings, 1 reply; 31+ messages in thread
From: Steve French @ 2025-09-29 14:05 UTC (permalink / raw)
To: Enzo Matsumiya
Cc: linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
Looks promising (although wish could be done in smaller stages, easier
to test and review).
Do you have any reproducers for the problems it fixes? I have been
worried about tests like generic/011 and generic/013 that fail about
10-20% of the time (probably due to deferred close issues/races) - and
hoping that we can get more consistent tests ot repro deferred close
issues so we don't see them in the future.
On Mon, Sep 29, 2025 at 8:28 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> Hi,
>
> This patch series aims to refactor cached dir related code in order to
> improve performance, improve code maintenance/readability, and of course
> fix several, existing and potential, bugs.
>
> Please note that the below only makes sense to the whole series applied.
>
> Semantic fixes:
> - cfid->has_lease vs cfid->is_open: when opening a cached dir, we get a fid
> (is_open) and a lease (has_lease), however, has_lease is used differently
> throughout the code, meaning, most of the time, that the cfid is 'usable'
> (fix in patch 11)
> - refcounting also follows has_lease, up to a point, when we need to
> 'steal' the reference, then we might have a cfid with 2 refs but
> has_lease == false (fix in patches 1-5)
> - cfid lookup: currently done with open_cached_dir() with @lookup_only arg,
> but that is not visibly good-looking and also highly inflexible (because
> it only works for paths (char *).
>
>
> Technical fixes:
> - due to the many "Dentry still in use" bugs, cleaning up a cfid has become
> too complex -- there are 3 workers to do that asynchronously, and the
> release callback itself. Complexity aside, this still has bugs because
> open_cached_dir() design doesn't account for any concurrent invalidation,
> leading sometimes to double opens/closes, sometimes straight UAF/deadlock
> bugs (examples upon request).
> (fix in patches 1-11)
> - locking: the list lock is not used consistently; sometimes protecting only
> the list, sometimes protecting only a cfid, sometimes both.
> cfid->fid_lock only protects ->dentry, nothing else. This leads to
> inconsistent data being read when a concurrent invalidation occurs, e.g.
> cached_dir_lease_break() (sets ->time = 0) vs cifs_dentry_needs_reval()
> (reads ->time unlocked)
> * also, open_cached_dir() always assume it has >1 refs, but such
> assumption is proven wrong when SMB2_open_init() triggers
> smb2_reconnect(), and kref_put() is ran locked in the rc != 0 case,
> leading to a deadlock because the extra ref has been dropped async
> (both fixed in patch 19 and others)
>
> Improvements:
> Having all above fixes and changes allows a cleaner code with a simpler
> design:
> - code readability is improved (cf. whole series)
> - usage of cached dirs in places that weren't making use of it (cf. patches
> 12-18)
> - patch 19 (locking) not only fixes the synchronization problems, but RCU +
> seqcounting allows faster lookups (read-mostly) while also allowing
> consistent reads and stability for callers (prevents UAF)
> - because a directory is always a parent, bake-in support for when opening
> a path, ParentLeaseKey can be set for any target child (cf. patch 12)
>
>
> Cheers,
>
> Enzo Matsumiya (20):
> smb: client: remove cfids_invalidation_worker
> smb: client: remove cached_dir_offload_close/close_work
> smb: client: remove cached_dir_put_work/put_work
> smb: client: remove cached_fids->dying list
> smb: client: remove cached_fid->on_list
> smb: client: merge {close,invalidate}_all_cached_dirs()
> smb: client: merge free_cached_dir in release callback
> smb: client: split find_or_create_cached_dir()
> smb: client: enhance cached dir lookups
> smb: client: refactor dropping cached dirs
> smb: client: simplify cached_fid state checking
> smb: client: prevent lease breaks of cached parents when opening
> children
> smb: client: actually use cached dirs on readdir
> smb: client: wait for concurrent caching of dirents in cifs_readdir()
> smb: client: remove cached_dirent->fattr
> smb: client: add is_dir argument to query_path_info
> smb: client: use cached dir on queryfs/smb2_compound_op
> smb: client: fix dentry revalidation of cached root
> smb: client: rework cached dirs synchronization
> smb: client: cleanup open_cached_dir()
>
> fs/smb/client/cached_dir.c | 946 ++++++++++++++++---------------------
> fs/smb/client/cached_dir.h | 74 +--
> fs/smb/client/cifs_debug.c | 7 +-
> fs/smb/client/cifsfs.c | 2 +-
> fs/smb/client/cifsglob.h | 5 +-
> fs/smb/client/dir.c | 27 +-
> fs/smb/client/file.c | 2 +-
> fs/smb/client/inode.c | 38 +-
> fs/smb/client/misc.c | 9 +-
> fs/smb/client/readdir.c | 146 +++---
> fs/smb/client/smb1ops.c | 6 +-
> fs/smb/client/smb2inode.c | 48 +-
> fs/smb/client/smb2misc.c | 2 +-
> fs/smb/client/smb2ops.c | 49 +-
> fs/smb/client/smb2pdu.c | 99 +++-
> fs/smb/client/smb2proto.h | 10 +-
> 16 files changed, 733 insertions(+), 737 deletions(-)
>
> --
> 2.49.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/20] smb: client: cached dir fixes and improvements
2025-09-29 14:05 ` [PATCH 00/20] smb: client: cached dir fixes and improvements Steve French
@ 2025-09-29 14:44 ` Enzo Matsumiya
0 siblings, 0 replies; 31+ messages in thread
From: Enzo Matsumiya @ 2025-09-29 14:44 UTC (permalink / raw)
To: Steve French
Cc: linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
On 09/29, Steve French wrote:
>Looks promising (although wish could be done in smaller stages, easier
>to test and review).
The patch I sent you privately a while ago contained most of these in a
single patch, and it would be definitely not fun to review.
Also, I keep seeing reviews saying to "split patch into smaller bits for
easier review", so which one is it?
>Do you have any reproducers for the problems it fixes?
Since the issues covered are mostly caused by races, it's hard to come
up with a reproducer, let alone a reliable one.
I do however can say that we have customer bugs with the following
backtraces, that inspired all these changes.
Deadlock in open_cached_dir() when smb2_reconnect() is triggered:
> Kernel panic - not syncing: softlockup: hung tasks
> ...
> RIP: 0010:native_queued_spin_lock_slowpath+0x2d/0x2c0
> ...
> _raw_spin_lock+0x25/0x30
> smb2_close_cached_fid+0x17/0xc0
> open_cached_dir+0x8ff/0xb60
> ? smb2_query_info_compound+0x332/0x5c0
> smb2_query_info_compound+0x332/0x5c0
> ? step_into+0x10d/0x690
> ? path_lookupat+0x98/0x140
> ? filename_lookup+0x115/0x1c0
> smb2_queryfs+0x6a/0xf0
> cifs_statfs+0x9f/0x2b0
> statfs_by_dentry+0x67/0x90
> vfs_statfs+0x16/0xd0
> user_statfs+0x54/0xa0
> __do_sys_statfs+0x20/0x50
> do_syscall_64+0x5b/0x80
Refcount underflow (UAF) in smb2_query_info_compound():
> refcount_t: underflow; use-after-free.
> WARNING: CPU: 1 PID: 11224 at ../lib/refcount.c:28 refcount_warn_saturate+0x9c/0x110
> ...
> RIP: 0010:refcount_warn_saturate+0x9c/0x110
> ...
> Call Trace:
> <TASK>
> smb2_query_info_compound+0x29c/0x5c0
> ? step_into+0x10d/0x690
> ? __legitimize_path+0x28/0x60
> smb2_queryfs+0x6a/0xf0
> smb311_queryfs+0x12d/0x140
> ? kmem_cache_alloc+0x18a/0x340
> ? getname_flags+0x46/0x1e0
> cifs_statfs+0x9f/0x2b0
> statfs_by_dentry+0x67/0x90
> vfs_statfs+0x16/0xd0
> user_statfs+0x54/0xa0
> __do_sys_statfs+0x20/0x50
> do_syscall_64+0x58/0x80
While developing the patches, I used xfstests generic/241 (dbench 4
clients) which makes heavy usage of cached dirs.
If you printk cfid fields at the end of open_cached_dir() when
rc == 0, you should see that it sometimes has been invalidated, and a
now-closed fid is returned to the caller.
If you mix in random interruptions (ctrl-c or network drops) you should
end up with similar backtraces as above.
>I have been
>worried about tests like generic/011 and generic/013 that fail about
>10-20% of the time (probably due to deferred close issues/races) - and
>hoping that we can get more consistent tests ot repro deferred close
>issues so we don't see them in the future.
Yes, that problem also shows up in generic/241 -- even though running
dbench standalone is fine, the xfstest unit does a final cleanup of the
test dir and leads to ENOTEMPTY in the end.
I spent the last 1-2 months debugging this, trying to find a way to fix
it in this very series, but failed.
FWIW, improvement-wise, the pcap for that test, with the series
applied, shows ~5% less opens and ~14% less lease breaks.
Cheers,
Enzo
>On Mon, Sep 29, 2025 at 8:28 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> Hi,
>>
>> This patch series aims to refactor cached dir related code in order to
>> improve performance, improve code maintenance/readability, and of course
>> fix several, existing and potential, bugs.
>>
>> Please note that the below only makes sense to the whole series applied.
>>
>> Semantic fixes:
>> - cfid->has_lease vs cfid->is_open: when opening a cached dir, we get a fid
>> (is_open) and a lease (has_lease), however, has_lease is used differently
>> throughout the code, meaning, most of the time, that the cfid is 'usable'
>> (fix in patch 11)
>> - refcounting also follows has_lease, up to a point, when we need to
>> 'steal' the reference, then we might have a cfid with 2 refs but
>> has_lease == false (fix in patches 1-5)
>> - cfid lookup: currently done with open_cached_dir() with @lookup_only arg,
>> but that is not visibly good-looking and also highly inflexible (because
>> it only works for paths (char *).
>>
>>
>> Technical fixes:
>> - due to the many "Dentry still in use" bugs, cleaning up a cfid has become
>> too complex -- there are 3 workers to do that asynchronously, and the
>> release callback itself. Complexity aside, this still has bugs because
>> open_cached_dir() design doesn't account for any concurrent invalidation,
>> leading sometimes to double opens/closes, sometimes straight UAF/deadlock
>> bugs (examples upon request).
>> (fix in patches 1-11)
>> - locking: the list lock is not used consistently; sometimes protecting only
>> the list, sometimes protecting only a cfid, sometimes both.
>> cfid->fid_lock only protects ->dentry, nothing else. This leads to
>> inconsistent data being read when a concurrent invalidation occurs, e.g.
>> cached_dir_lease_break() (sets ->time = 0) vs cifs_dentry_needs_reval()
>> (reads ->time unlocked)
>> * also, open_cached_dir() always assume it has >1 refs, but such
>> assumption is proven wrong when SMB2_open_init() triggers
>> smb2_reconnect(), and kref_put() is ran locked in the rc != 0 case,
>> leading to a deadlock because the extra ref has been dropped async
>> (both fixed in patch 19 and others)
>>
>> Improvements:
>> Having all above fixes and changes allows a cleaner code with a simpler
>> design:
>> - code readability is improved (cf. whole series)
>> - usage of cached dirs in places that weren't making use of it (cf. patches
>> 12-18)
>> - patch 19 (locking) not only fixes the synchronization problems, but RCU +
>> seqcounting allows faster lookups (read-mostly) while also allowing
>> consistent reads and stability for callers (prevents UAF)
>> - because a directory is always a parent, bake-in support for when opening
>> a path, ParentLeaseKey can be set for any target child (cf. patch 12)
>>
>>
>> Cheers,
>>
>> Enzo Matsumiya (20):
>> smb: client: remove cfids_invalidation_worker
>> smb: client: remove cached_dir_offload_close/close_work
>> smb: client: remove cached_dir_put_work/put_work
>> smb: client: remove cached_fids->dying list
>> smb: client: remove cached_fid->on_list
>> smb: client: merge {close,invalidate}_all_cached_dirs()
>> smb: client: merge free_cached_dir in release callback
>> smb: client: split find_or_create_cached_dir()
>> smb: client: enhance cached dir lookups
>> smb: client: refactor dropping cached dirs
>> smb: client: simplify cached_fid state checking
>> smb: client: prevent lease breaks of cached parents when opening
>> children
>> smb: client: actually use cached dirs on readdir
>> smb: client: wait for concurrent caching of dirents in cifs_readdir()
>> smb: client: remove cached_dirent->fattr
>> smb: client: add is_dir argument to query_path_info
>> smb: client: use cached dir on queryfs/smb2_compound_op
>> smb: client: fix dentry revalidation of cached root
>> smb: client: rework cached dirs synchronization
>> smb: client: cleanup open_cached_dir()
>>
>> fs/smb/client/cached_dir.c | 946 ++++++++++++++++---------------------
>> fs/smb/client/cached_dir.h | 74 +--
>> fs/smb/client/cifs_debug.c | 7 +-
>> fs/smb/client/cifsfs.c | 2 +-
>> fs/smb/client/cifsglob.h | 5 +-
>> fs/smb/client/dir.c | 27 +-
>> fs/smb/client/file.c | 2 +-
>> fs/smb/client/inode.c | 38 +-
>> fs/smb/client/misc.c | 9 +-
>> fs/smb/client/readdir.c | 146 +++---
>> fs/smb/client/smb1ops.c | 6 +-
>> fs/smb/client/smb2inode.c | 48 +-
>> fs/smb/client/smb2misc.c | 2 +-
>> fs/smb/client/smb2ops.c | 49 +-
>> fs/smb/client/smb2pdu.c | 99 +++-
>> fs/smb/client/smb2proto.h | 10 +-
>> 16 files changed, 733 insertions(+), 737 deletions(-)
>>
>> --
>> 2.49.0
>>
>
>
>--
>Thanks,
>
>Steve
^ permalink raw reply [flat|nested] 31+ messages in thread