| Message ID | 20230329201423.32134-3-pc@manguebit.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath | expand |
reviewed-by me On Thu, 30 Mar 2023 at 06:20, Paulo Alcantara <pc@manguebit.com> wrote: > > Prevent multiple threads of doing negotiate, session setup and tree > connect by holding @ses->session_mutex in cifs_reconnect_tcon() while > reconnecting session and tcon. > > Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> > --- > fs/cifs/cifssmb.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 38a697eca305..c9d57ba84be4 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -71,7 +71,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > int rc; > struct cifs_ses *ses; > struct TCP_Server_Info *server; > - struct nls_table *nls_codepage; > + struct nls_table *nls_codepage = NULL; > > /* > * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for > @@ -99,6 +99,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > } > spin_unlock(&tcon->tc_lock); > > +again: > rc = cifs_wait_for_server_reconnect(server, tcon->retry); > if (rc) > return rc; > @@ -110,8 +111,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > } > spin_unlock(&ses->chan_lock); > > - nls_codepage = load_nls_default(); > - > + mutex_lock(&ses->session_mutex); > /* > * Recheck after acquire mutex. If another thread is negotiating > * and the server never sends an answer the socket will be closed > @@ -120,29 +120,38 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > spin_lock(&server->srv_lock); > if (server->tcpStatus == CifsNeedReconnect) { > spin_unlock(&server->srv_lock); > + mutex_lock(&ses->session_mutex); > + > + if (tcon->retry) > + goto again; > rc = -EHOSTDOWN; > goto out; > } > spin_unlock(&server->srv_lock); > > + nls_codepage = load_nls_default(); > + > /* > * need to prevent multiple threads trying to simultaneously > * reconnect the same SMB session > */ > + spin_lock(&ses->ses_lock); > spin_lock(&ses->chan_lock); > - if (!cifs_chan_needs_reconnect(ses, server)) { > + if (!cifs_chan_needs_reconnect(ses, server) && > + ses->ses_status == SES_GOOD) { > spin_unlock(&ses->chan_lock); > + spin_unlock(&ses->ses_lock); > > /* this means that we only need to tree connect */ > if (tcon->need_reconnect) > goto skip_sess_setup; > > - rc = -EHOSTDOWN; > + mutex_unlock(&ses->session_mutex); > goto out; > } > spin_unlock(&ses->chan_lock); > + spin_unlock(&ses->ses_lock); > > - mutex_lock(&ses->session_mutex); > rc = cifs_negotiate_protocol(0, ses, server); > if (!rc) > rc = cifs_setup_session(0, ses, server, nls_codepage); > -- > 2.40.0 >
added ronnie's RB and merged into cifs-2.6.git for-next (I will hold off on patches 1 and 2 since they are cleanup until 6.4-rc) On Wed, Mar 29, 2023 at 3:14 PM Paulo Alcantara <pc@manguebit.com> wrote: > > Prevent multiple threads of doing negotiate, session setup and tree > connect by holding @ses->session_mutex in cifs_reconnect_tcon() while > reconnecting session and tcon. > > Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> > --- > fs/cifs/cifssmb.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 38a697eca305..c9d57ba84be4 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -71,7 +71,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > int rc; > struct cifs_ses *ses; > struct TCP_Server_Info *server; > - struct nls_table *nls_codepage; > + struct nls_table *nls_codepage = NULL; > > /* > * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for > @@ -99,6 +99,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > } > spin_unlock(&tcon->tc_lock); > > +again: > rc = cifs_wait_for_server_reconnect(server, tcon->retry); > if (rc) > return rc; > @@ -110,8 +111,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > } > spin_unlock(&ses->chan_lock); > > - nls_codepage = load_nls_default(); > - > + mutex_lock(&ses->session_mutex); > /* > * Recheck after acquire mutex. If another thread is negotiating > * and the server never sends an answer the socket will be closed > @@ -120,29 +120,38 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > spin_lock(&server->srv_lock); > if (server->tcpStatus == CifsNeedReconnect) { > spin_unlock(&server->srv_lock); > + mutex_lock(&ses->session_mutex); > + > + if (tcon->retry) > + goto again; > rc = -EHOSTDOWN; > goto out; > } > spin_unlock(&server->srv_lock); > > + nls_codepage = load_nls_default(); > + > /* > * need to prevent multiple threads trying to simultaneously > * reconnect the same SMB session > */ > + spin_lock(&ses->ses_lock); > spin_lock(&ses->chan_lock); > - if (!cifs_chan_needs_reconnect(ses, server)) { > + if (!cifs_chan_needs_reconnect(ses, server) && > + ses->ses_status == SES_GOOD) { > spin_unlock(&ses->chan_lock); > + spin_unlock(&ses->ses_lock); > > /* this means that we only need to tree connect */ > if (tcon->need_reconnect) > goto skip_sess_setup; > > - rc = -EHOSTDOWN; > + mutex_unlock(&ses->session_mutex); > goto out; > } > spin_unlock(&ses->chan_lock); > + spin_unlock(&ses->ses_lock); > > - mutex_lock(&ses->session_mutex); > rc = cifs_negotiate_protocol(0, ses, server); > if (!rc) > rc = cifs_setup_session(0, ses, server, nls_codepage); > -- > 2.40.0 >
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 38a697eca305..c9d57ba84be4 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -71,7 +71,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) int rc; struct cifs_ses *ses; struct TCP_Server_Info *server; - struct nls_table *nls_codepage; + struct nls_table *nls_codepage = NULL; /* * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for @@ -99,6 +99,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) } spin_unlock(&tcon->tc_lock); +again: rc = cifs_wait_for_server_reconnect(server, tcon->retry); if (rc) return rc; @@ -110,8 +111,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) } spin_unlock(&ses->chan_lock); - nls_codepage = load_nls_default(); - + mutex_lock(&ses->session_mutex); /* * Recheck after acquire mutex. If another thread is negotiating * and the server never sends an answer the socket will be closed @@ -120,29 +120,38 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) spin_lock(&server->srv_lock); if (server->tcpStatus == CifsNeedReconnect) { spin_unlock(&server->srv_lock); + mutex_lock(&ses->session_mutex); + + if (tcon->retry) + goto again; rc = -EHOSTDOWN; goto out; } spin_unlock(&server->srv_lock); + nls_codepage = load_nls_default(); + /* * need to prevent multiple threads trying to simultaneously * reconnect the same SMB session */ + spin_lock(&ses->ses_lock); spin_lock(&ses->chan_lock); - if (!cifs_chan_needs_reconnect(ses, server)) { + if (!cifs_chan_needs_reconnect(ses, server) && + ses->ses_status == SES_GOOD) { spin_unlock(&ses->chan_lock); + spin_unlock(&ses->ses_lock); /* this means that we only need to tree connect */ if (tcon->need_reconnect) goto skip_sess_setup; - rc = -EHOSTDOWN; + mutex_unlock(&ses->session_mutex); goto out; } spin_unlock(&ses->chan_lock); + spin_unlock(&ses->ses_lock); - mutex_lock(&ses->session_mutex); rc = cifs_negotiate_protocol(0, ses, server); if (!rc) rc = cifs_setup_session(0, ses, server, nls_codepage);
Prevent multiple threads of doing negotiate, session setup and tree connect by holding @ses->session_mutex in cifs_reconnect_tcon() while reconnecting session and tcon. Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> --- fs/cifs/cifssmb.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)