[{"id":3683600,"web_url":"http://patchwork.ozlabs.org/comment/3683600/","msgid":"","list_archive_url":null,"date":"2026-04-28T17:18:02","subject":"Re: [PATCH v3 08/19] cifs: make cfid locks more granular","submitter":{"id":78375,"url":"http://patchwork.ozlabs.org/api/people/78375/","name":"Enzo Matsumiya","email":"ematsumiya@suse.de"},"content":"On 04/28, nspmangalore@gmail.com wrote:\n>From: Shyam Prasad N \n>\n>Today all synchronization of cfid related data structures are done\n>using cfid_list_lock. This can serialize caching of different dirs\n>unnecessarily.\n>\n>This change introduces two new locks to provide finer locking.\n>Every cfid will now have a cfid_lock. This is designed to protect\n>everything inside a cfid that is not related to list operations.\n>\n>Every cfid will now also have a cfid_open_mutex. This is designed to\n>protect parallel open calls to the same dir.\n>\n>Additionally, this change will now make accesses to cfid->dentries\n>more stringent using the de_mutex.\n\nSuch lock provides no synchronization, race prevention, or any real\nbenefit really, cf. 17ef15fa80cf (\"smb: client: remove unused fid_lock\")\n\ne.g. here list operations on cfid->entry are not protected by that lock,\nand the list moving/removal are one of the biggest problems (\"dentry\nstill in use\") with e.g. laundromat vs tcon reconnect scenarios.\n\nBy no means I'm suggesting that cfid->entry access should be locked --\nquite the contrary, actually; I can confirm it's fully possible to have\nthe whole cfid infrastructure lockless as I've done it myself based on\nmy previous series\nhttps://lore.kernel.org/linux-cifs/lh2xlppdsuytbidpbl4tnua3wy5islx3iqbosy65hud4jo65l3@ysnku7wwx32v/\n\n>Signed-off-by: Shyam Prasad N \n>---\n> fs/smb/client/cached_dir.c | 155 +++++++++++++++++++++++++------------\n> fs/smb/client/cached_dir.h | 13 +++-\n> fs/smb/client/cifs_debug.c | 7 +-\n> fs/smb/client/cifsglob.h | 2 +\n> fs/smb/client/dir.c | 34 ++++++--\n> 5 files changed, 149 insertions(+), 62 deletions(-)\n>\n>diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c\n>index a9bc0c81868c8..ad2439856a1fe 100644\n>--- a/fs/smb/client/cached_dir.c\n>+++ b/fs/smb/client/cached_dir.c\n>@@ -16,13 +16,29 @@ static struct cached_fid *init_cached_dir(const char *path);\n> static void free_cached_dir(struct cached_fid *cfid);\n> static void smb2_close_cached_fid(struct kref *ref);\n> static void cfids_laundromat_worker(struct work_struct *work);\n>-static void close_cached_dir_locked(struct cached_fid *cfid);\n>\n> struct cached_dir_dentry {\n> \tstruct list_head entry;\n> \tstruct dentry *dentry;\n> };\n>\n>+bool cached_dir_copy_lease_key(struct cached_fid *cfid,\n>+\t\t\t __u8 lease_key[SMB2_LEASE_KEY_SIZE])\n>+{\n>+\tbool valid;\n>+\n>+\tif (!cfid)\n>+\t\treturn false;\n>+\n>+\tspin_lock(&cfid->cfid_lock);\n>+\tvalid = is_valid_cached_dir(cfid);\n>+\tif (valid)\n>+\t\tmemcpy(lease_key, cfid->fid.lease_key, SMB2_LEASE_KEY_SIZE);\n>+\tspin_unlock(&cfid->cfid_lock);\n>+\n>+\treturn valid;\n>+}\n>+\n> static bool emit_cached_dirents(struct cached_dirents *cde,\n> \t\t\t\tstruct dir_context *ctx)\n> {\n>@@ -244,9 +260,13 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,\n> \t\t\t * fully cached or it may be in the process of\n> \t\t\t * being deleted due to a lease break.\n> \t\t\t */\n>-\t\t\tif (!is_valid_cached_dir(cfid))\n>+\t\t\tspin_lock(&cfid->cfid_lock);\n>+\t\t\tif (!is_valid_cached_dir(cfid)) {\n>+\t\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\t\treturn NULL;\n>+\t\t\t}\n> \t\t\tkref_get(&cfid->refcount);\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\treturn cfid;\n> \t\t}\n> \t}\n>@@ -273,7 +293,9 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,\n> \t * Concurrent processes won't be to use it yet due to @cfid->time being\n> \t * zero.\n> \t */\n>+\tspin_lock(&cfid->cfid_lock);\n> \tcfid->has_lease = true;\n>+\tspin_unlock(&cfid->cfid_lock);\n>\n> \treturn cfid;\n> }\n>@@ -396,19 +418,23 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n> \t\tkfree(utf16_path);\n> \t\treturn -ENOENT;\n> \t}\n>+\tspin_unlock(&cfids->cfid_list_lock);\n>+\n> \t/*\n> \t * Return cached fid if it is valid (has a lease and has a time).\n> \t * Otherwise, it is either a new entry or laundromat worker removed it\n> \t * from @cfids->entries. Caller will put last reference if the latter.\n> \t */\n>+\n>+\tspin_lock(&cfid->cfid_lock);\n> \tif (is_valid_cached_dir(cfid)) {\n> \t\tcfid->last_access_time = jiffies;\n>-\t\tspin_unlock(&cfids->cfid_list_lock);\n>+\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t*ret_cfid = cfid;\n> \t\tkfree(utf16_path);\n> \t\treturn 0;\n> \t}\n>-\tspin_unlock(&cfids->cfid_list_lock);\n>+\tspin_unlock(&cfid->cfid_lock);\n>\n> \tpfid = &cfid->fid;\n>\n>@@ -438,6 +464,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n>\n> \t\t\tspin_lock(&cfids->cfid_list_lock);\n> \t\t\tlist_for_each_entry(parent_cfid, &cfids->entries, entry) {\n>+\t\t\t\tspin_lock(&parent_cfid->cfid_lock);\n> \t\t\t\tif (parent_cfid->dentry == dentry->d_parent) {\n> \t\t\t\t\tcifs_dbg(FYI, \"found a parent cached file handle\\n\");\n> \t\t\t\t\tif (is_valid_cached_dir(parent_cfid)) {\n>@@ -447,8 +474,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n> \t\t\t\t\t\t parent_cfid->fid.lease_key,\n> \t\t\t\t\t\t SMB2_LEASE_KEY_SIZE);\n> \t\t\t\t\t}\n>+\t\t\t\t\tspin_unlock(&parent_cfid->cfid_lock);\n> \t\t\t\t\tbreak;\n> \t\t\t\t}\n>+\t\t\t\tspin_unlock(&parent_cfid->cfid_lock);\n> \t\t\t}\n> \t\t\tspin_unlock(&cfids->cfid_list_lock);\n> \t\t}\n>@@ -527,10 +556,13 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n> \t\tsmb2_set_replay(server, &rqst[1]);\n> \t}\n>\n>+\tmutex_lock(&cfid->cfid_open_mutex);\n>+\n\nThis will make concurrent threads find 'no cfid for path' and then\nthey'll all do the SMB2 open serialized, which will make no use at all\nof the first cached one.\n\n> \trc = compound_send_recv(xid, ses, server,\n> \t\t\t\tflags, 2, rqst,\n> \t\t\t\tresp_buftype, rsp_iov);\n> \tif (rc) {\n>+\t\tmutex_unlock(&cfid->cfid_open_mutex);\n> \t\tif (rc == -EREMCHG) {\n> \t\t\ttcon->need_reconnect = true;\n> \t\t\tpr_warn_once(\"server share %s deleted\\n\",\n>@@ -538,10 +570,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n> \t\t}\n> \t\tgoto oshr_free;\n> \t}\n>+\tspin_lock(&cfid->cfid_lock);\n> \tcfid->is_open = true;\n>\n>-\tspin_lock(&cfids->cfid_list_lock);\n>-\n> \to_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;\n> \toparms.fid->persistent_fid = o_rsp->PersistentFileId;\n> \toparms.fid->volatile_fid = o_rsp->VolatileFileId;\n>@@ -551,8 +582,9 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n>\n>\n> \tif (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {\n>-\t\tspin_unlock(&cfids->cfid_list_lock);\n> \t\trc = -EINVAL;\n>+\t\tspin_unlock(&cfid->cfid_lock);\n>+\t\tmutex_unlock(&cfid->cfid_open_mutex);\n> \t\tgoto oshr_free;\n> \t}\n>\n>@@ -561,18 +593,21 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n> \t\t\t\t oparms.fid->lease_key,\n> \t\t\t\t &oplock, NULL, NULL);\n> \tif (rc) {\n>-\t\tspin_unlock(&cfids->cfid_list_lock);\n>+\t\tspin_unlock(&cfid->cfid_lock);\n>+\t\tmutex_unlock(&cfid->cfid_open_mutex);\n> \t\tgoto oshr_free;\n> \t}\n>\n> \trc = -EINVAL;\n> \tif (!(oplock & SMB2_LEASE_READ_CACHING_HE)) {\n>-\t\tspin_unlock(&cfids->cfid_list_lock);\n>+\t\tspin_unlock(&cfid->cfid_lock);\n>+\t\tmutex_unlock(&cfid->cfid_open_mutex);\n> \t\tgoto oshr_free;\n> \t}\n> \tqi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;\n> \tif (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) {\n>-\t\tspin_unlock(&cfids->cfid_list_lock);\n>+\t\tspin_unlock(&cfid->cfid_lock);\n>+\t\tmutex_unlock(&cfid->cfid_open_mutex);\n> \t\tgoto oshr_free;\n> \t}\n> \tif (!smb2_validate_and_copy_iov(\n>@@ -584,7 +619,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n>\n> \tcfid->time = jiffies;\n> \tcfid->last_access_time = jiffies;\n>-\tspin_unlock(&cfids->cfid_list_lock);\n>+\tspin_unlock(&cfid->cfid_lock);\n>+\tmutex_unlock(&cfid->cfid_open_mutex);\n> \t/* At this point the directory handle is fully cached */\n> \trc = 0;\n>\n>@@ -595,23 +631,24 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,\n> \tfree_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);\n> out:\n> \tif (rc) {\n>+\t\tbool drop_lease_ref = false;\n>+\n> \t\tspin_lock(&cfids->cfid_list_lock);\n> \t\tif (cfid->on_list) {\n> \t\t\tlist_del(&cfid->entry);\n> \t\t\tcfid->on_list = false;\n> \t\t\tcfids->num_entries--;\n> \t\t}\n>+\t\tspin_lock(&cfid->cfid_lock);\n> \t\tif (cfid->has_lease) {\n>-\t\t\t/*\n>-\t\t\t * We are guaranteed to have two references at this\n>-\t\t\t * point. One for the caller and one for a potential\n>-\t\t\t * lease. Release one here, and the second below.\n>-\t\t\t */\n> \t\t\tcfid->has_lease = false;\n>-\t\t\tclose_cached_dir_locked(cfid);\n>+\t\t\tdrop_lease_ref = true;\n> \t\t}\n>+\t\tspin_unlock(&cfid->cfid_lock);\n> \t\tspin_unlock(&cfids->cfid_list_lock);\n>\n>+\t\tif (drop_lease_ref)\n>+\t\t\tclose_cached_dir(cfid);\n> \t\tclose_cached_dir(cfid);\n> \t} else {\n> \t\t*ret_cfid = cfid;\n>@@ -642,12 +679,16 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,\n> \tspin_lock(&cfids->cfid_list_lock);\n> \tlist_for_each_entry(cfid, &cfids->entries, entry) {\n> \t\tif (cfid->dentry == dentry) {\n>-\t\t\tif (!is_valid_cached_dir(cfid))\n>+\t\t\tspin_lock(&cfid->cfid_lock);\n>+\t\t\tif (!is_valid_cached_dir(cfid)) {\n>+\t\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\t\tbreak;\n>+\t\t\t}\n> \t\t\tcifs_dbg(FYI, \"found a cached file handle by dentry\\n\");\n> \t\t\tkref_get(&cfid->refcount);\n> \t\t\t*ret_cfid = cfid;\n> \t\t\tcfid->last_access_time = jiffies;\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\tspin_unlock(&cfids->cfid_list_lock);\n> \t\t\treturn 0;\n> \t\t}\n>@@ -662,6 +703,8 @@ __releases(&cfid->cfids->cfid_list_lock)\n> {\n> \tstruct cached_fid *cfid = container_of(ref, struct cached_fid,\n> \t\t\t\t\t refcount);\n>+\tu64 persistent_fid = 0, volatile_fid = 0;\n>+\tbool is_open;\n> \tint rc;\n>\n> \tlockdep_assert_held(&cfid->cfids->cfid_list_lock);\n>@@ -676,9 +719,17 @@ __releases(&cfid->cfids->cfid_list_lock)\n> \tdput(cfid->dentry);\n> \tcfid->dentry = NULL;\n>\n>-\tif (cfid->is_open) {\n>-\t\trc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,\n>-\t\t\t cfid->fid.volatile_fid);\n>+\tspin_lock(&cfid->cfid_lock);\n>+\tis_open = cfid->is_open;\n>+\tif (is_open) {\n>+\t\tpersistent_fid = cfid->fid.persistent_fid;\n>+\t\tvolatile_fid = cfid->fid.volatile_fid;\n>+\t\tcfid->is_open = false;\n>+\t}\n>+\tspin_unlock(&cfid->cfid_lock);\n>+\n>+\tif (is_open) {\n>+\t\trc = SMB2_close(0, cfid->tcon, persistent_fid, volatile_fid);\n> \t\tif (rc) /* should we retry on -EBUSY or -EAGAIN? */\n> \t\t\tcifs_dbg(VFS, \"close cached dir rc %d\\n\", rc);\n> \t}\n>@@ -691,6 +742,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,\n> {\n> \tstruct cached_fid *cfid = NULL;\n> \tint rc;\n>+\tbool drop_lease_ref = false;\n>\n> \trc = open_cached_dir(xid, tcon, name, cifs_sb, true, &cfid);\n> \tif (rc) {\n>@@ -698,11 +750,16 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,\n> \t\treturn;\n> \t}\n> \tspin_lock(&cfid->cfids->cfid_list_lock);\n>+\tspin_lock(&cfid->cfid_lock);\n> \tif (cfid->has_lease) {\n> \t\tcfid->has_lease = false;\n>-\t\tclose_cached_dir_locked(cfid);\n>+\t\tdrop_lease_ref = true;\n> \t}\n>+\tspin_unlock(&cfid->cfid_lock);\n> \tspin_unlock(&cfid->cfids->cfid_list_lock);\n>+\n>+\tif (drop_lease_ref)\n>+\t\tclose_cached_dir(cfid);\n> \tclose_cached_dir(cfid);\n> }\n>\n>@@ -711,8 +768,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,\n> *\n> * The release function will be called with cfid_list_lock held to remove the\n> * cached dirs from the list before any other thread can take another @cfid\n>- * ref. Must not be called with cfid_list_lock held; use\n>- * close_cached_dir_locked() called instead.\n>+ * ref. Must not be called with cfid_list_lock held.\n> *\n> * @cfid: cached dir\n> */\n>@@ -722,30 +778,6 @@ void close_cached_dir(struct cached_fid *cfid)\n> \tkref_put_lock(&cfid->refcount, smb2_close_cached_fid, &cfid->cfids->cfid_list_lock);\n> }\n>\n>-/**\n>- * close_cached_dir_locked - put a reference of a cached dir with\n>- * cfid_list_lock held\n>- *\n>- * Calling close_cached_dir() with cfid_list_lock held has the potential effect\n>- * of causing a deadlock if the invariant of refcount >= 2 is false.\n>- *\n>- * This function is used in paths that hold cfid_list_lock and expect at least\n>- * two references. If that invariant is violated, WARNs and returns without\n>- * dropping a reference; the final put must still go through\n>- * close_cached_dir().\n>- *\n>- * @cfid: cached dir\n>- */\n>-static void close_cached_dir_locked(struct cached_fid *cfid)\n>-{\n>-\tlockdep_assert_held(&cfid->cfids->cfid_list_lock);\n>-\n>-\tif (WARN_ON(kref_read(&cfid->refcount) < 2))\n>-\t\treturn;\n>-\n>-\tkref_put(&cfid->refcount, smb2_close_cached_fid);\n>-}\n>-\n> /*\n> * Called from cifs_kill_sb when we unmount a share\n> */\n>@@ -784,8 +816,10 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)\n> \t\t\t\tgoto done;\n> \t\t\t}\n>\n>+\t\t\tspin_lock(&cfid->cfid_lock);\n> \t\t\ttmp_list->dentry = cfid->dentry;\n> \t\t\tcfid->dentry = NULL;\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n>\n> \t\t\tlist_add_tail(&tmp_list->entry, &entry);\n> \t\t}\n>@@ -825,16 +859,20 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)\n> \tlist_for_each_entry_safe(cfid, q, &cfids->entries, entry) {\n> \t\tlist_move(&cfid->entry, &cfids->dying);\n> \t\tcfids->num_entries--;\n>+\t\tspin_lock(&cfid->cfid_lock);\n> \t\tcfid->is_open = false;\n>-\t\tcfid->on_list = false;\n> \t\tif (cfid->has_lease) {\n> \t\t\t/*\n> \t\t\t * The lease was never cancelled from the server,\n> \t\t\t * so steal that reference.\n> \t\t\t */\n> \t\t\tcfid->has_lease = false;\n>-\t\t} else\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n>+\t\t} else {\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\tkref_get(&cfid->refcount);\n>+\t\t}\n>+\t\tcfid->on_list = false;\n> \t}\n> \tspin_unlock(&cfids->cfid_list_lock);\n>\n>@@ -883,12 +921,14 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])\n>\n> \tspin_lock(&cfids->cfid_list_lock);\n> \tlist_for_each_entry(cfid, &cfids->entries, entry) {\n>+\t\tspin_lock(&cfid->cfid_lock);\n> \t\tif (cfid->has_lease &&\n> \t\t !memcmp(lease_key,\n> \t\t\t cfid->fid.lease_key,\n> \t\t\t SMB2_LEASE_KEY_SIZE)) {\n> \t\t\tcfid->has_lease = false;\n> \t\t\tcfid->time = 0;\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\t/*\n> \t\t\t * We found a lease remove it from the list\n> \t\t\t * so no threads can access it.\n>@@ -904,6 +944,7 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])\n> \t\t\tspin_unlock(&cfids->cfid_list_lock);\n> \t\t\treturn true;\n> \t\t}\n>+\t\tspin_unlock(&cfid->cfid_lock);\n> \t}\n> \tspin_unlock(&cfids->cfid_list_lock);\n> \treturn false;\n>@@ -927,6 +968,8 @@ static struct cached_fid *init_cached_dir(const char *path)\n> \tINIT_LIST_HEAD(&cfid->entry);\n> \tINIT_LIST_HEAD(&cfid->dirents.entries);\n> \tmutex_init(&cfid->dirents.de_mutex);\n>+\tmutex_init(&cfid->cfid_open_mutex);\n>+\tspin_lock_init(&cfid->cfid_lock);\n> \tkref_init(&cfid->refcount);\n> \treturn cfid;\n> }\n>@@ -983,6 +1026,7 @@ static void cfids_laundromat_worker(struct work_struct *work)\n> \tlist_cut_before(&entry, &cfids->dying, &cfids->dying);\n>\n> \tlist_for_each_entry_safe(cfid, q, &cfids->entries, entry) {\n>+\t\tspin_lock(&cfid->cfid_lock);\n> \t\tif (cfid->last_access_time &&\n> \t\t time_after(jiffies, cfid->last_access_time + HZ * dir_cache_timeout)) {\n> \t\t\tcfid->on_list = false;\n>@@ -994,8 +1038,13 @@ static void cfids_laundromat_worker(struct work_struct *work)\n> \t\t\t\t * server. Steal that reference.\n> \t\t\t\t */\n> \t\t\t\tcfid->has_lease = false;\n>-\t\t\t} else\n>+\t\t\t\tspin_unlock(&cfid->cfid_lock);\n>+\t\t\t} else {\n>+\t\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\t\tkref_get(&cfid->refcount);\n>+\t\t\t}\n>+\t\t} else {\n>+\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t}\n> \t}\n> \tspin_unlock(&cfids->cfid_list_lock);\n>@@ -1062,12 +1111,16 @@ void free_cached_dirs(struct cached_fids *cfids)\n> \tspin_lock(&cfids->cfid_list_lock);\n> \tlist_for_each_entry_safe(cfid, q, &cfids->entries, entry) {\n> \t\tcfid->on_list = false;\n>+\t\tspin_lock(&cfid->cfid_lock);\n> \t\tcfid->is_open = false;\n>+\t\tspin_unlock(&cfid->cfid_lock);\n> \t\tlist_move(&cfid->entry, &entry);\n> \t}\n> \tlist_for_each_entry_safe(cfid, q, &cfids->dying, entry) {\n> \t\tcfid->on_list = false;\n>+\t\tspin_lock(&cfid->cfid_lock);\n> \t\tcfid->is_open = false;\n>+\t\tspin_unlock(&cfid->cfid_lock);\n> \t\tlist_move(&cfid->entry, &entry);\n> \t}\n> \tspin_unlock(&cfids->cfid_list_lock);\n>diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h\n>index 09f1f488059c9..f82db6a7ca5b0 100644\n>--- a/fs/smb/client/cached_dir.h\n>+++ b/fs/smb/client/cached_dir.h\n>@@ -48,6 +48,8 @@ struct cached_fid {\n> \tstruct work_struct put_work;\n> \tstruct work_struct close_work;\n> \tstruct cached_dirents dirents;\n>+\tstruct mutex cfid_open_mutex; /* Serializes OPEN response processing and lease key population */\n>+\tspinlock_t cfid_lock; /* Protects: has_lease, time, is_open, file_all_info_is_valid, last_access_time, fid.lease_key reads */\n>\n> \t/* Must be last as it ends in a flexible-array member. */\n> \tstruct smb2_file_all_info file_all_info;\n>@@ -56,8 +58,12 @@ struct cached_fid {\n> /* default MAX_CACHED_FIDS is 16 */\n> struct cached_fids {\n> \t/* Must be held when:\n>-\t * - accessing the cfids->entries list\n>-\t * - accessing the cfids->dying list\n>+\t * - modifying cfids->entries list (add/remove entries)\n>+\t * - modifying cfids->dying list\n>+\t * - modifying cfid->on_list or cfids->num_entries\n>+\t *\n>+\t * Lock ordering: if you need both cfid_list_lock and cfid_lock,\n>+\t * acquire cfid_list_lock FIRST, then cfid_lock to avoid deadlock.\n> \t */\n> \tspinlock_t cfid_list_lock;\n> \tint num_entries;\n>@@ -78,6 +84,9 @@ is_valid_cached_dir(struct cached_fid *cfid)\n> \treturn cfid->time && cfid->has_lease;\n> }\n>\n>+bool cached_dir_copy_lease_key(struct cached_fid *cfid,\n>+\t\t\t __u8 lease_key[SMB2_LEASE_KEY_SIZE]);\n>+\n> struct cached_fids *init_cached_dirs(void);\n> void free_cached_dirs(struct cached_fids *cfids);\n> int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, const char *path,\n>diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c\n>index 217444e3e6d01..cc7d26a3917c5 100644\n>--- a/fs/smb/client/cifs_debug.c\n>+++ b/fs/smb/client/cifs_debug.c\n>@@ -327,6 +327,7 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)\n> \t\t\t\t\t\t(unsigned long)atomic_long_read(&cfids->total_dirents_entries),\n> \t\t\t\t\t\t(unsigned long long)atomic64_read(&cfids->total_dirents_bytes));\n> \t\t\t\tlist_for_each_entry(cfid, &cfids->entries, entry) {\n>+\t\t\t\t\tspin_lock(&cfid->cfid_lock);\n> \t\t\t\t\tseq_printf(m, \"0x%x 0x%llx 0x%llx \",\n> \t\t\t\t\t\ttcon->tid,\n> \t\t\t\t\t\tses->Suid,\n>@@ -338,11 +339,13 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)\n> \t\t\t\t\tseq_printf(m, \"%s\", cfid->path);\n> \t\t\t\t\tif (cfid->file_all_info_is_valid)\n> \t\t\t\t\t\tseq_printf(m, \"\\tvalid file info\");\n>+\t\t\t\t\tspin_unlock(&cfid->cfid_lock);\n> \t\t\t\t\tif (cfid->dirents.is_valid)\n> \t\t\t\t\t\tseq_printf(m, \", valid dirents\");\n>-\t\t\t\t\tif (!list_empty(&cfid->dirents.entries))\n>+\t\t\t\t\tif (READ_ONCE(cfid->dirents.entries_count))\n> \t\t\t\t\t\tseq_printf(m, \", dirents: %lu entries, %lu bytes\",\n>-\t\t\t\t\t\tcfid->dirents.entries_count, cfid->dirents.bytes_used);\n>+\t\t\t\t\t\tREAD_ONCE(cfid->dirents.entries_count),\n>+\t\t\t\t\t\tREAD_ONCE(cfid->dirents.bytes_used));\n> \t\t\t\t\tseq_printf(m, \"\\n\");\n> \t\t\t\t}\n> \t\t\t\tspin_unlock(&cfids->cfid_list_lock);\n>diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h\n>index 38d5600efe2c8..a15971ffeee58 100644\n>--- a/fs/smb/client/cifsglob.h\n>+++ b/fs/smb/client/cifsglob.h\n>@@ -2068,6 +2068,8 @@ require use of the stronger protocol */\n> *\t\t\t\t->can_cache_brlcks\n> * cifsInodeInfo->deferred_lock\tcifsInodeInfo->deferred_closes\tcifsInodeInfo_alloc\n> * cached_fids->cfid_list_lock\tcifs_tcon->cfids->entries\tinit_cached_dirs\n>+ * cached_fid->cfid_open_mutex\tcached_fid OPEN/lease serialization\talloc_cached_dir\n>+ * cached_fid->cfid_lock\tcached_fid state\t\talloc_cached_dir\n> * cached_fid->dirents.de_mutex\tcached_fid->dirents\t\talloc_cached_dir\n> * cifsFileInfo->fh_mutex\tcifsFileInfo\t\t\tcifs_new_fileinfo\n> * cifsFileInfo->file_info_lock\tcifsFileInfo->count\t\tcifs_new_fileinfo\n>diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c\n>index 6d2378eeb7f68..4e5c580e4de0a 100644\n>--- a/fs/smb/client/dir.c\n>+++ b/fs/smb/client/dir.c\n>@@ -194,6 +194,7 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned\n> \tstruct cached_fid *parent_cfid = NULL;\n> \tint rdwr_for_fscache = 0;\n> \t__le32 lease_flags = 0;\n>+\tbool found_parent_cfid;\n>\n> \t*oplock = 0;\n> \tif (tcon->ses->server->oplocks)\n>@@ -319,24 +320,33 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned\n>\n> retry_open:\n> \tif (tcon->cfids && direntry->d_parent && server->dialect >= SMB30_PROT_ID) {\n>+\t\tfound_parent_cfid = false;\n> \t\tparent_cfid = NULL;\n> \t\tspin_lock(&tcon->cfids->cfid_list_lock);\n> \t\tlist_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {\n>+\t\t\tspin_lock(&parent_cfid->cfid_lock);\n> \t\t\tif (parent_cfid->dentry == direntry->d_parent) {\n>+\t\t\t\tkref_get(&parent_cfid->refcount);\n>+\t\t\t\tspin_unlock(&parent_cfid->cfid_lock);\n>+\t\t\t\tspin_unlock(&tcon->cfids->cfid_list_lock);\n>+\t\t\t\tfound_parent_cfid = true;\n> \t\t\t\tcifs_dbg(FYI, \"found a parent cached file handle\\n\");\n>-\t\t\t\tif (is_valid_cached_dir(parent_cfid)) {\n>+\t\t\t\tif (cached_dir_copy_lease_key(parent_cfid,\n>+\t\t\t\t\t\t fid->parent_lease_key)) {\n> \t\t\t\t\tlease_flags\n> \t\t\t\t\t\t|= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;\n>-\t\t\t\t\tmemcpy(fid->parent_lease_key,\n>-\t\t\t\t\t parent_cfid->fid.lease_key,\n>-\t\t\t\t\t SMB2_LEASE_KEY_SIZE);\n>+\t\t\t\t\tmutex_lock(&parent_cfid->dirents.de_mutex);\n> \t\t\t\t\tparent_cfid->dirents.is_valid = false;\n> \t\t\t\t\tparent_cfid->dirents.is_failed = true;\n>+\t\t\t\t\tmutex_unlock(&parent_cfid->dirents.de_mutex);\n> \t\t\t\t}\n>+\t\t\t\tclose_cached_dir(parent_cfid);\n> \t\t\t\tbreak;\n> \t\t\t}\n>+\t\t\tspin_unlock(&parent_cfid->cfid_lock);\n> \t\t}\n>-\t\tspin_unlock(&tcon->cfids->cfid_list_lock);\n>+\t\tif (!found_parent_cfid)\n>+\t\t\tspin_unlock(&tcon->cfids->cfid_list_lock);\n> \t}\n>\n> \toparms = (struct cifs_open_parms) {\n>@@ -737,7 +747,12 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,\n> \t\t\t * dentry is negative and parent is fully cached:\n> \t\t\t * we can assume file does not exist\n> \t\t\t */\n>-\t\t\tif (cfid->dirents.is_valid) {\n>+\t\t\tbool dirents_valid;\n>+\n>+\t\t\tmutex_lock(&cfid->dirents.de_mutex);\n>+\t\t\tdirents_valid = cfid->dirents.is_valid;\n>+\t\t\tmutex_unlock(&cfid->dirents.de_mutex);\n>+\t\t\tif (dirents_valid) {\n> \t\t\t\tclose_cached_dir(cfid);\n> \t\t\t\tgoto out;\n> \t\t\t}\n>@@ -848,7 +863,12 @@ cifs_d_revalidate(struct inode *dir, const struct qstr *name,\n> \t\t\t * dentry is negative and parent is fully cached:\n> \t\t\t * we can assume file does not exist\n> \t\t\t */\n>-\t\t\tif (cfid->dirents.is_valid) {\n>+\t\t\tbool dirents_valid;\n>+\n>+\t\t\tmutex_lock(&cfid->dirents.de_mutex);\n>+\t\t\tdirents_valid = cfid->dirents.is_valid;\n>+\t\t\tmutex_unlock(&cfid->dirents.de_mutex);\n>+\t\t\tif (dirents_valid) {\n> \t\t\t\tclose_cached_dir(cfid);\n> \t\t\t\treturn 1;\n> \t\t\t}\n>-- \n>2.43.0\n>\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256\n header.s=susede2_rsa header.b=uX+bXiuO;\n\tdkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=7lMt1ESt;\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.a=rsa-sha256 header.s=susede2_rsa header.b=qNTTW/NJ;\n\tdkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=3cPkCaLW;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-cifs+bounces-11260-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"uX+bXiuO\";\n\tdkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"7lMt1ESt\";\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"qNTTW/NJ\";\n\tdkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"3cPkCaLW\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=195.135.223.131","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=suse.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=suse.de","smtp-out2.suse.de;\n\tdkim=pass header.d=suse.de header.s=susede2_rsa header.b=\"qNTTW/NJ\";\n\tdkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=3cPkCaLW"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g4nRD7286z1yHv\n\tfor ; Wed, 29 Apr 2026 03:26:32 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 770CB3161C2E\n\tfor ; Tue, 28 Apr 2026 17:18:21 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 840F53D9DDF;\n\tTue, 28 Apr 2026 17:18:12 +0000 (UTC)","from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 211D63B2FC8\n\tfor ; Tue, 28 Apr 2026 17:18:07 +0000 (UTC)","from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org\n [IPv6:2a07:de40:b281:104:10:150:64:97])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby smtp-out2.suse.de (Postfix) with ESMTPS id A7B065BD6C;\n\tTue, 28 Apr 2026 17:18:05 +0000 (UTC)","from imap1.dmz-prg2.suse.org (localhost [127.0.0.1])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 2C740593B0;\n\tTue, 28 Apr 2026 17:18:04 +0000 (UTC)","from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167])\n\tby imap1.dmz-prg2.suse.org with ESMTPSA\n\tid c8MuOMzr8GnOVAAAD6G6ig\n\t(envelope-from ); Tue, 28 Apr 2026 17:18:04 +0000"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777396692; cv=none;\n b=DQQC1umhtsP9LVefnqTWl7dIUK7EvUs2alf2WVbbFypYBXqFZXBKKdIJYqf0HLG1KGmO0khHpuNUeZTlF9bB53/YQmVKlrK0MdM/CQqUhA9xr5A1BD1yucEIh+QmG33SVKf8qdiDSW4Lu9RU+etK0Mxrz63LBrLNHD0+zUN4tzA=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777396692; c=relaxed/simple;\n\tbh=PDNyrVI3QnO7C9zalEACIjeR5WLuN3eBwkGRK08XPnY=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=f8zvJ/9ziw+Xe13vgdWPkDHQ1y29y4dUR0TK1CWoDg6178W7gW7xVg1ZHOAYoi1lUQHzi2mAhmsrGH0m5Y2XzP5aORFUnuXJsgMVC6ladoQZ+2WwZsHD7psr5p71xG3Bp3mrtwNuHrHUz2xB8TcS4HEWIuyyTkyXTM20P2sXZIA=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=suse.de;\n spf=pass smtp.mailfrom=suse.de;\n dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=uX+bXiuO;\n dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=7lMt1ESt;\n dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=qNTTW/NJ;\n dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=3cPkCaLW; arc=none smtp.client-ip=195.135.223.131","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n\tt=1777396686;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=0pc29CmDVcfYDjkA3mN+DyJ4AxP9qdp6P5KhKDihaIs=;\n\tb=uX+bXiuOoY7l7M9XbHTjfBbqLw5P3+L95sXKwbXIWLcYb/L1ShliUwoqnAPcYJ108K9+QV\n\tvU/jyCtBixyaQyR31Tq5yy89kAuoSKG3vAaog7zhTsB8YzISSxurvKXs/xPNLlvqrGsajy\n\tDIPHPSQgntoHRHGds2tUvQRGCJwU43Q=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n\ts=susede2_ed25519; t=1777396686;\n\th=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=0pc29CmDVcfYDjkA3mN+DyJ4AxP9qdp6P5KhKDihaIs=;\n\tb=7lMt1EStI5Vc+y06+IlrT1BzGf1dOrK4mfoXnMA4qymM5ieCMgmTdmwvDXpIGAWXmG8PJm\n\t1CvXAozqXJHnroAA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n\tt=1777396685;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=0pc29CmDVcfYDjkA3mN+DyJ4AxP9qdp6P5KhKDihaIs=;\n\tb=qNTTW/NJUDbPTc/H3qTzeJetaggFll4JwVlYrG/rVw2hIh2DWiOt62KGEcV9hK3MlQYdGR\n\tncSvTIvExxSV5oZV2hGiFGX5hUWFHXatB8nMO60+HN5WsI56lSupZALUvjygZHySh7cZDY\n\tGR8Ga9zmt//JN5DTbQ9d69Jin4JeHUA=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n\ts=susede2_ed25519; t=1777396685;\n\th=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=0pc29CmDVcfYDjkA3mN+DyJ4AxP9qdp6P5KhKDihaIs=;\n\tb=3cPkCaLWnB0l6kQAtZ+vHfjCpyzn4xb0MznFKSlakytq9ukaSgZNZn+Zfl9X1qJUFeORAS\n\tlZaUqwkYZKO7TWCQ=="],"Date":"Tue, 28 Apr 2026 14:18:02 -0300","From":"Enzo Matsumiya ","To":"nspmangalore@gmail.com","Cc":"linux-cifs@vger.kernel.org, smfrench@gmail.com, pc@manguebit.org,\n\tbharathsm@microsoft.com, dhowells@redhat.com, henrique.carvalho@suse.com,\n\tShyam Prasad N ","Subject":"Re: [PATCH v3 08/19] cifs: make cfid locks more granular","Message-ID":"","References":"<20260428160804.281745-1-sprasad@microsoft.com>\n <20260428160804.281745-8-sprasad@microsoft.com>","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii; format=flowed","Content-Disposition":"inline","In-Reply-To":"<20260428160804.281745-8-sprasad@microsoft.com>","X-Rspamd-Action":"no action","X-Rspamd-Server":"rspamd2.dmz-prg2.suse.org","X-Spamd-Result":"default: False [-4.51 / 50.00];\n\tBAYES_HAM(-3.00)[100.00%];\n\tNEURAL_HAM_LONG(-1.00)[-1.000];\n\tR_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519];\n\tNEURAL_HAM_SHORT(-0.20)[-1.000];\n\tMIME_GOOD(-0.10)[text/plain];\n\tMX_GOOD(-0.01)[];\n\tFREEMAIL_ENVRCPT(0.00)[gmail.com];\n\tTO_MATCH_ENVRCPT_ALL(0.00)[];\n\tDKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519];\n\tFREEMAIL_TO(0.00)[gmail.com];\n\tARC_NA(0.00)[];\n\tFUZZY_RATELIMITED(0.00)[rspamd.com];\n\tTO_DN_SOME(0.00)[];\n\tMIME_TRACE(0.00)[0:+];\n\tFREEMAIL_CC(0.00)[vger.kernel.org,gmail.com,manguebit.org,microsoft.com,redhat.com,suse.com];\n\tRCVD_TLS_ALL(0.00)[];\n\tDKIM_TRACE(0.00)[suse.de:+];\n\tRCVD_COUNT_TWO(0.00)[2];\n\tFROM_EQ_ENVFROM(0.00)[];\n\tFROM_HAS_DN(0.00)[];\n\tSPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from];\n\tDNSWL_BLOCKED(0.00)[2a07:de40:b281:104:10:150:64:97:from];\n\tRCPT_COUNT_SEVEN(0.00)[8];\n\tMID_RHS_MATCH_FROM(0.00)[];\n\tMISSING_XM_UA(0.00)[];\n\tRCVD_VIA_SMTP_AUTH(0.00)[];\n\tDBL_BLOCKED_OPENRESOLVER(0.00)[imap1.dmz-prg2.suse.org:helo,imap1.dmz-prg2.suse.org:rdns,suse.de:dkim,suse.de:mid]","X-Rspamd-Queue-Id":"A7B065BD6C","X-Spam-Flag":"NO","X-Spam-Score":"-4.51","X-Spam-Level":""}}]