Skip to content

Commit 73f9bfb

Browse files
sprasad-microsoftsmfrench
authored andcommitted
cifs: maintain a state machine for tcp/smb/tcon sessions
If functions like cifs_negotiate_protocol, cifs_setup_session, cifs_tree_connect are called in parallel on different channels, each of these will be execute the requests. This maybe unnecessary in some cases, and only the first caller may need to do the work. This is achieved by having more states for the tcp/smb/tcon session status fields. And tracking the state of reconnection based on the state machine. For example: for tcp connections: CifsNew/CifsNeedReconnect -> CifsNeedNegotiate -> CifsInNegotiate -> CifsNeedSessSetup -> CifsInSessSetup -> CifsGood for smb sessions: CifsNew/CifsNeedReconnect -> CifsGood for tcon: CifsNew/CifsNeedReconnect -> CifsInFilesInvalidate -> CifsNeedTcon -> CifsInTcon -> CifsGood If any channel reconnect sees that it's in the middle of transition to CifsGood, then they can skip the function. Signed-off-by: Shyam Prasad N <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 1913e11 commit 73f9bfb

File tree

5 files changed

+82
-35
lines changed

5 files changed

+82
-35
lines changed

fs/cifs/cifsglob.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,13 @@ enum statusEnum {
113113
CifsGood,
114114
CifsExiting,
115115
CifsNeedReconnect,
116-
CifsNeedNegotiate
116+
CifsNeedNegotiate,
117+
CifsInNegotiate,
118+
CifsNeedSessSetup,
119+
CifsInSessSetup,
120+
CifsNeedTcon,
121+
CifsInTcon,
122+
CifsInFilesInvalidate
117123
};
118124

119125
enum securityEnum {

fs/cifs/cifssmb.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
7373
struct list_head *tmp;
7474
struct list_head *tmp1;
7575

76+
/* only send once per connect */
77+
spin_lock(&cifs_tcp_ses_lock);
78+
if (tcon->ses->status != CifsGood ||
79+
tcon->tidStatus != CifsNeedReconnect) {
80+
spin_unlock(&cifs_tcp_ses_lock);
81+
return;
82+
}
83+
tcon->tidStatus = CifsInFilesInvalidate;
84+
spin_unlock(&cifs_tcp_ses_lock);
85+
7686
/* list all files open on tree connection and mark them invalid */
7787
spin_lock(&tcon->open_file_lock);
7888
list_for_each_safe(tmp, tmp1, &tcon->openFileList) {
@@ -89,6 +99,11 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
8999
memset(tcon->crfid.fid, 0, sizeof(struct cifs_fid));
90100
mutex_unlock(&tcon->crfid.fid_mutex);
91101

102+
spin_lock(&cifs_tcp_ses_lock);
103+
if (tcon->tidStatus == CifsInFilesInvalidate)
104+
tcon->tidStatus = CifsNeedTcon;
105+
spin_unlock(&cifs_tcp_ses_lock);
106+
92107
/*
93108
* BB Add call to invalidate_inodes(sb) for all superblocks mounted
94109
* to this tcon.
@@ -182,12 +197,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
182197

183198
nls_codepage = load_nls_default();
184199

185-
/*
186-
* need to prevent multiple threads trying to simultaneously
187-
* reconnect the same SMB session
188-
*/
189-
mutex_lock(&ses->session_mutex);
190-
191200
/*
192201
* Recheck after acquire mutex. If another thread is negotiating
193202
* and the server never sends an answer the socket will be closed
@@ -197,7 +206,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
197206
if (server->tcpStatus == CifsNeedReconnect) {
198207
spin_unlock(&cifs_tcp_ses_lock);
199208
rc = -EHOSTDOWN;
200-
mutex_unlock(&ses->session_mutex);
201209
goto out;
202210
}
203211
spin_unlock(&cifs_tcp_ses_lock);
@@ -215,11 +223,11 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
215223
goto skip_sess_setup;
216224

217225
rc = -EHOSTDOWN;
218-
mutex_unlock(&ses->session_mutex);
219226
goto out;
220227
}
221228
spin_unlock(&ses->chan_lock);
222229

230+
mutex_lock(&ses->session_mutex);
223231
rc = cifs_negotiate_protocol(0, ses, server);
224232
if (!rc)
225233
rc = cifs_setup_session(0, ses, server, nls_codepage);

fs/cifs/connect.c

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
208208
if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses))
209209
goto next_session;
210210

211+
ses->status = CifsNeedReconnect;
211212
num_sessions++;
212213

213-
list_for_each_entry(tcon, &ses->tcon_list, tcon_list)
214+
list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
214215
tcon->need_reconnect = true;
216+
tcon->tidStatus = CifsNeedReconnect;
217+
}
215218
if (ses->tcon_ipc)
216219
ses->tcon_ipc->need_reconnect = true;
217220

@@ -2035,12 +2038,12 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
20352038
cifs_dbg(FYI, "Existing smb sess found (status=%d)\n",
20362039
ses->status);
20372040

2038-
mutex_lock(&ses->session_mutex);
20392041
spin_lock(&ses->chan_lock);
20402042
if (cifs_chan_needs_reconnect(ses, server)) {
20412043
spin_unlock(&ses->chan_lock);
20422044
cifs_dbg(FYI, "Session needs reconnect\n");
20432045

2046+
mutex_lock(&ses->session_mutex);
20442047
rc = cifs_negotiate_protocol(xid, ses, server);
20452048
if (rc) {
20462049
mutex_unlock(&ses->session_mutex);
@@ -2059,10 +2062,11 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
20592062
free_xid(xid);
20602063
return ERR_PTR(rc);
20612064
}
2065+
mutex_unlock(&ses->session_mutex);
2066+
20622067
spin_lock(&ses->chan_lock);
20632068
}
20642069
spin_unlock(&ses->chan_lock);
2065-
mutex_unlock(&ses->session_mutex);
20662070

20672071
/* existing SMB ses has a server reference already */
20682072
cifs_put_tcp_session(server, 0);
@@ -2112,7 +2116,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
21122116

21132117
ses->sectype = ctx->sectype;
21142118
ses->sign = ctx->sign;
2115-
mutex_lock(&ses->session_mutex);
21162119

21172120
/* add server as first channel */
21182121
spin_lock(&ses->chan_lock);
@@ -2122,15 +2125,16 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
21222125
ses->chans_need_reconnect = 1;
21232126
spin_unlock(&ses->chan_lock);
21242127

2128+
mutex_lock(&ses->session_mutex);
21252129
rc = cifs_negotiate_protocol(xid, ses, server);
21262130
if (!rc)
21272131
rc = cifs_setup_session(xid, ses, server, ctx->local_nls);
2132+
mutex_unlock(&ses->session_mutex);
21282133

21292134
/* each channel uses a different signing key */
21302135
memcpy(ses->chans[0].signkey, ses->smb3signingkey,
21312136
sizeof(ses->smb3signingkey));
21322137

2133-
mutex_unlock(&ses->session_mutex);
21342138
if (rc)
21352139
goto get_ses_fail;
21362140

@@ -2347,10 +2351,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
23472351
}
23482352
}
23492353

2350-
/*
2351-
* BB Do we need to wrap session_mutex around this TCon call and Unix
2352-
* SetFS as we do on SessSetup and reconnect?
2353-
*/
23542354
xid = get_xid();
23552355
rc = ses->server->ops->tree_connect(xid, ses, ctx->UNC, tcon,
23562356
ctx->local_nls);
@@ -3870,14 +3870,20 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
38703870
return -ENOSYS;
38713871

38723872
/* only send once per connect */
3873-
if (!server->ops->need_neg(server))
3873+
spin_lock(&cifs_tcp_ses_lock);
3874+
if (!server->ops->need_neg(server) ||
3875+
server->tcpStatus != CifsNeedNegotiate) {
3876+
spin_unlock(&cifs_tcp_ses_lock);
38743877
return 0;
3878+
}
3879+
server->tcpStatus = CifsInNegotiate;
3880+
spin_unlock(&cifs_tcp_ses_lock);
38753881

38763882
rc = server->ops->negotiate(xid, ses, server);
38773883
if (rc == 0) {
38783884
spin_lock(&cifs_tcp_ses_lock);
3879-
if (server->tcpStatus == CifsNeedNegotiate)
3880-
server->tcpStatus = CifsGood;
3885+
if (server->tcpStatus == CifsInNegotiate)
3886+
server->tcpStatus = CifsNeedSessSetup;
38813887
else
38823888
rc = -EHOSTDOWN;
38833889
spin_unlock(&cifs_tcp_ses_lock);
@@ -3894,6 +3900,15 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
38943900
int rc = -ENOSYS;
38953901
bool is_binding = false;
38963902

3903+
/* only send once per connect */
3904+
spin_lock(&cifs_tcp_ses_lock);
3905+
if (server->tcpStatus != CifsNeedSessSetup) {
3906+
spin_unlock(&cifs_tcp_ses_lock);
3907+
return 0;
3908+
}
3909+
ses->status = CifsInSessSetup;
3910+
spin_unlock(&cifs_tcp_ses_lock);
3911+
38973912
spin_lock(&ses->chan_lock);
38983913
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
38993914
spin_unlock(&ses->chan_lock);
@@ -4264,6 +4279,17 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
42644279
struct dfs_cache_tgt_iterator *tit;
42654280
bool target_match;
42664281

4282+
/* only send once per connect */
4283+
spin_lock(&cifs_tcp_ses_lock);
4284+
if (tcon->ses->status != CifsGood ||
4285+
(tcon->tidStatus != CifsNew &&
4286+
tcon->tidStatus != CifsNeedTcon)) {
4287+
spin_unlock(&cifs_tcp_ses_lock);
4288+
return 0;
4289+
}
4290+
tcon->tidStatus = CifsInTcon;
4291+
spin_unlock(&cifs_tcp_ses_lock);
4292+
42674293
extract_unc_hostname(server->hostname, &tcp_host, &tcp_host_len);
42684294

42694295
tit = dfs_cache_get_tgt_iterator(tl);
@@ -4422,6 +4448,17 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
44224448
{
44234449
const struct smb_version_operations *ops = tcon->ses->server->ops;
44244450

4451+
/* only send once per connect */
4452+
spin_lock(&cifs_tcp_ses_lock);
4453+
if (tcon->ses->status != CifsGood ||
4454+
(tcon->tidStatus != CifsNew &&
4455+
tcon->tidStatus != CifsNeedTcon)) {
4456+
spin_unlock(&cifs_tcp_ses_lock);
4457+
return 0;
4458+
}
4459+
tcon->tidStatus = CifsInTcon;
4460+
spin_unlock(&cifs_tcp_ses_lock);
4461+
44254462
return ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
44264463
}
44274464
#endif

fs/cifs/sess.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
308308

309309
chan_server = cifs_get_tcp_session(&ctx, ses->server);
310310

311-
mutex_lock(&ses->session_mutex);
312311
spin_lock(&ses->chan_lock);
313312
chan = &ses->chans[ses->chan_count];
314313
chan->server = chan_server;
@@ -326,6 +325,7 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
326325

327326
spin_unlock(&ses->chan_lock);
328327

328+
mutex_lock(&ses->session_mutex);
329329
/*
330330
* We need to allocate the server crypto now as we will need
331331
* to sign packets before we generate the channel signing key
@@ -334,13 +334,16 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
334334
rc = smb311_crypto_shash_allocate(chan->server);
335335
if (rc) {
336336
cifs_dbg(VFS, "%s: crypto alloc failed\n", __func__);
337+
mutex_unlock(&ses->session_mutex);
337338
goto out;
338339
}
339340

340341
rc = cifs_negotiate_protocol(xid, ses, chan->server);
341342
if (!rc)
342343
rc = cifs_setup_session(xid, ses, chan->server, cifs_sb->local_nls);
343344

345+
mutex_unlock(&ses->session_mutex);
346+
344347
out:
345348
if (rc && chan->server) {
346349
spin_lock(&ses->chan_lock);
@@ -355,8 +358,6 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
355358
spin_unlock(&ses->chan_lock);
356359
}
357360

358-
mutex_unlock(&ses->session_mutex);
359-
360361
if (rc && chan->server)
361362
cifs_put_tcp_session(chan->server, 0);
362363

@@ -1053,6 +1054,7 @@ sess_establish_session(struct sess_data *sess_data)
10531054

10541055
/* Even if one channel is active, session is in good state */
10551056
spin_lock(&cifs_tcp_ses_lock);
1057+
server->tcpStatus = CifsGood;
10561058
ses->status = CifsGood;
10571059
spin_unlock(&cifs_tcp_ses_lock);
10581060

fs/cifs/smb2pdu.c

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
251251

252252
nls_codepage = load_nls_default();
253253

254-
/*
255-
* need to prevent multiple threads trying to simultaneously reconnect
256-
* the same SMB session
257-
*/
258-
mutex_lock(&ses->session_mutex);
259-
260254
/*
261255
* Recheck after acquire mutex. If another thread is negotiating
262256
* and the server never sends an answer the socket will be closed
@@ -266,7 +260,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
266260
if (server->tcpStatus == CifsNeedReconnect) {
267261
spin_unlock(&cifs_tcp_ses_lock);
268262
rc = -EHOSTDOWN;
269-
mutex_unlock(&ses->session_mutex);
270263
goto out;
271264
}
272265
spin_unlock(&cifs_tcp_ses_lock);
@@ -284,23 +277,23 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
284277
goto skip_sess_setup;
285278

286279
rc = -EHOSTDOWN;
287-
mutex_unlock(&ses->session_mutex);
288280
goto out;
289281
}
290282
spin_unlock(&ses->chan_lock);
291283

284+
mutex_lock(&ses->session_mutex);
292285
rc = cifs_negotiate_protocol(0, ses, server);
293286
if (!rc) {
294287
rc = cifs_setup_session(0, ses, server, nls_codepage);
295288
if ((rc == -EACCES) && !tcon->retry) {
296-
rc = -EHOSTDOWN;
297289
mutex_unlock(&ses->session_mutex);
290+
rc = -EHOSTDOWN;
298291
goto failed;
299292
}
300293
}
301294

302295
if (rc || !tcon->need_reconnect) {
303-
mutex_unlock(&tcon->ses->session_mutex);
296+
mutex_unlock(&ses->session_mutex);
304297
goto out;
305298
}
306299

@@ -310,7 +303,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
310303
tcon->need_reopen_files = true;
311304

312305
rc = cifs_tree_connect(0, tcon, nls_codepage);
313-
mutex_unlock(&tcon->ses->session_mutex);
306+
mutex_unlock(&ses->session_mutex);
314307

315308
cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
316309
if (rc) {
@@ -1397,6 +1390,7 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data)
13971390

13981391
/* Even if one channel is active, session is in good state */
13991392
spin_lock(&cifs_tcp_ses_lock);
1393+
server->tcpStatus = CifsGood;
14001394
ses->status = CifsGood;
14011395
spin_unlock(&cifs_tcp_ses_lock);
14021396

0 commit comments

Comments
 (0)