Re: [PATCH 6/7] cifs: tc_count updates should be done with tc_lock

[Paul Aurich <paul@darkrain42.org>]

On 2025-06-04 15:48:15 +0530, nspmangalore@gmail.com wrote:
>From: Shyam Prasad N <sprasad@microsoft.com>
>
>We had problems with deadlocks using the cifs_tcp_ses_lock for
>protecting a lot of structures. So we broke it down into smaller
>spinlocks. cifs_tcon struct fields are protected by tc_lock now.
>Hence we should stick to using that.

Is it really safe to adjust this to tc_lock?

There are a number of other points in the smb client that (AFAICT) only hold 
cifs_tcp_ses_lock while adjusting the tc_count (which is why I used that 
here).  See smb2_handle_cancelled_close, smb2_get_dfs_refer, 
smb2_reconnect_server, smb2_find_smb_tcon.

~Paul

>Fixes: 3fa640d035e5 ("smb: During unmount, ensure all cached dir instances drop their dentry")
>Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
>---
> fs/smb/client/cached_dir.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
>index 2746d693d80a..4abf5bbd8baf 100644
>--- a/fs/smb/client/cached_dir.c
>+++ b/fs/smb/client/cached_dir.c
>@@ -650,10 +650,12 @@ int cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
> 			spin_unlock(&cfid->fid_lock);
> 			cfids->num_entries--;
>
>+			spin_lock(&cfid->tcon->tc_lock);
> 			++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(&cfid->tcon->tc_lock);
> 			spin_unlock(&cfids->cfid_list_lock);
> 			return true;
> 		}
>@@ -767,11 +769,11 @@ static void cfids_laundromat_worker(struct work_struct *work)
> 		dput(dentry);
>
> 		if (cfid->is_open) {
>-			spin_lock(&cifs_tcp_ses_lock);
>+			spin_lock(&cfid->tcon->tc_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);
>+			spin_unlock(&cfid->tcon->tc_lock);
> 			queue_work(serverclose_wq, &cfid->close_work);
> 		} else
> 			/*
>-- 
>2.43.0
>