Skip to content

Commit

Permalink
cifs: take cifs_tcp_ses_lock for status checks
Browse files Browse the repository at this point in the history
While checking/updating status for tcp ses, smb ses or tcon,
we take GlobalMid_Lock. This doesn't make any sense.
Replaced it with cifs_tcp_ses_lock.

Ideally, we should take a spin lock per struct.
But since tcp ses, smb ses and tcon objects won't add up to a lot,
I think there should not be too much contention.

Also, in few other places, these are checked without locking.
Added locking for these.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
  • Loading branch information
Shyam Prasad N authored and Steve French committed Jan 3, 2022
1 parent 183eea2 commit 220c5bc
Show file tree
Hide file tree
Showing 12 changed files with 164 additions and 43 deletions.
4 changes: 2 additions & 2 deletions fs/cifs/cifs_swn.c
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,10 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
goto unlock;
}

spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
if (tcon->ses->server->tcpStatus != CifsExiting)
tcon->ses->server->tcpStatus = CifsNeedReconnect;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);

unlock:
mutex_unlock(&tcon->ses->server->srv_mutex);
Expand Down
6 changes: 5 additions & 1 deletion fs/cifs/cifsencrypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,13 @@ int cifs_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server,
if ((cifs_pdu == NULL) || (server == NULL))
return -EINVAL;

spin_lock(&cifs_tcp_ses_lock);
if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) ||
server->tcpStatus == CifsNeedNegotiate)
server->tcpStatus == CifsNeedNegotiate) {
spin_unlock(&cifs_tcp_ses_lock);
return rc;
}
spin_unlock(&cifs_tcp_ses_lock);

if (!server->session_estab) {
memcpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8);
Expand Down
4 changes: 2 additions & 2 deletions fs/cifs/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ struct TCP_Server_Info {
char server_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
struct smb_version_operations *ops;
struct smb_version_values *vals;
/* updates to tcpStatus protected by GlobalMid_Lock */
/* updates to tcpStatus protected by cifs_tcp_ses_lock */
enum statusEnum tcpStatus; /* what we think the status is */
char *hostname; /* hostname portion of UNC string */
struct socket *ssocket;
Expand Down Expand Up @@ -924,7 +924,7 @@ struct cifs_ses {
struct mutex session_mutex;
struct TCP_Server_Info *server; /* pointer to server info */
int ses_count; /* reference counter */
enum statusEnum status; /* updates protected by GlobalMid_Lock */
enum statusEnum status; /* updates protected by cifs_tcp_ses_lock */
unsigned overrideSecFlg; /* if non-zero override global sec flags */
char *serverOS; /* name of operating system underlying server */
char *serverNOS; /* name of network operating system of server */
Expand Down
12 changes: 11 additions & 1 deletion fs/cifs/cifssmb.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,18 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
* only tree disconnect, open, and write, (and ulogoff which does not
* have tcon) are allowed as we start force umount
*/
spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsExiting) {
if (smb_command != SMB_COM_WRITE_ANDX &&
smb_command != SMB_COM_OPEN_ANDX &&
smb_command != SMB_COM_TREE_DISCONNECT) {
spin_unlock(&cifs_tcp_ses_lock);
cifs_dbg(FYI, "can not send cmd %d while umounting\n",
smb_command);
return -ENODEV;
}
}
spin_unlock(&cifs_tcp_ses_lock);

retries = server->nr_targets;

Expand All @@ -148,8 +151,12 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
}

/* are we still trying to reconnect? */
if (server->tcpStatus != CifsNeedReconnect)
spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus != CifsNeedReconnect) {
spin_unlock(&cifs_tcp_ses_lock);
break;
}
spin_unlock(&cifs_tcp_ses_lock);

if (retries && --retries)
continue;
Expand Down Expand Up @@ -186,11 +193,14 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
* and the server never sends an answer the socket will be closed
* and tcpStatus set to reconnect.
*/
spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus == CifsNeedReconnect) {
spin_unlock(&cifs_tcp_ses_lock);
rc = -EHOSTDOWN;
mutex_unlock(&ses->session_mutex);
goto out;
}
spin_unlock(&cifs_tcp_ses_lock);

/*
* need to prevent multiple threads trying to simultaneously
Expand Down
36 changes: 26 additions & 10 deletions fs/cifs/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,12 @@ reconnect_dfs_server(struct TCP_Server_Info *server,
dfs_cache_free_tgts(&tl);

/* Need to set up echo worker again once connection has been established */
spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus == CifsNeedNegotiate)
mod_delayed_work(cifsiod_wq, &server->echo, 0);

spin_unlock(&cifs_tcp_ses_lock);

wake_up(&server->response_q);
return rc;
}
Expand Down Expand Up @@ -571,15 +574,18 @@ server_unresponsive(struct TCP_Server_Info *server)
* 65s kernel_recvmsg times out, and we see that we haven't gotten
* a response in >60s.
*/
spin_lock(&cifs_tcp_ses_lock);
if ((server->tcpStatus == CifsGood ||
server->tcpStatus == CifsNeedNegotiate) &&
(!server->ops->can_echo || server->ops->can_echo(server)) &&
time_after(jiffies, server->lstrp + 3 * server->echo_interval)) {
spin_unlock(&cifs_tcp_ses_lock);
cifs_server_dbg(VFS, "has not responded in %lu seconds. Reconnecting...\n",
(3 * server->echo_interval) / HZ);
cifs_reconnect(server, false);
return true;
}
spin_unlock(&cifs_tcp_ses_lock);

return false;
}
Expand Down Expand Up @@ -624,13 +630,18 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
else
length = sock_recvmsg(server->ssocket, smb_msg, 0);

if (server->tcpStatus == CifsExiting)
spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus == CifsExiting) {
spin_unlock(&cifs_tcp_ses_lock);
return -ESHUTDOWN;
}

if (server->tcpStatus == CifsNeedReconnect) {
spin_unlock(&cifs_tcp_ses_lock);
cifs_reconnect(server, false);
return -ECONNABORTED;
}
spin_unlock(&cifs_tcp_ses_lock);

if (length == -ERESTARTSYS ||
length == -EAGAIN ||
Expand Down Expand Up @@ -808,9 +819,9 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
cancel_delayed_work_sync(&server->echo);
cancel_delayed_work_sync(&server->resolve);

spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
server->tcpStatus = CifsExiting;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);
wake_up_all(&server->response_q);

/* check if we have blocked requests that need to free */
Expand Down Expand Up @@ -1427,9 +1438,9 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
else
cancel_delayed_work_sync(&server->reconnect);

spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
server->tcpStatus = CifsExiting;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);

cifs_crypto_secmech_release(server);

Expand Down Expand Up @@ -1582,7 +1593,9 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
* to the struct since the kernel thread not created yet
* no need to spinlock this update of tcpStatus
*/
spin_lock(&cifs_tcp_ses_lock);
tcp_ses->tcpStatus = CifsNeedNegotiate;
spin_unlock(&cifs_tcp_ses_lock);

if ((ctx->max_credits < 20) || (ctx->max_credits > 60000))
tcp_ses->max_credits = SMB2_MAX_CREDITS_AVAILABLE;
Expand Down Expand Up @@ -1799,15 +1812,13 @@ void cifs_put_smb_ses(struct cifs_ses *ses)
spin_unlock(&cifs_tcp_ses_lock);
return;
}
spin_unlock(&cifs_tcp_ses_lock);

/* ses_count can never go negative */
WARN_ON(ses->ses_count < 0);

spin_lock(&GlobalMid_Lock);
if (ses->status == CifsGood)
ses->status = CifsExiting;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);

cifs_free_ipc(ses);

Expand Down Expand Up @@ -3075,12 +3086,15 @@ static int mount_get_conns(struct mount_ctx *mnt_ctx)
* for just this mount.
*/
reset_cifs_unix_caps(xid, tcon, cifs_sb, ctx);
spin_lock(&cifs_tcp_ses_lock);
if ((tcon->ses->server->tcpStatus == CifsNeedReconnect) &&
(le64_to_cpu(tcon->fsUnixInfo.Capability) &
CIFS_UNIX_TRANSPORT_ENCRYPTION_MANDATORY_CAP)) {
spin_unlock(&cifs_tcp_ses_lock);
rc = -EACCES;
goto out;
}
spin_unlock(&cifs_tcp_ses_lock);
} else
tcon->unix_ext = 0; /* server does not support them */

Expand Down Expand Up @@ -3755,7 +3769,9 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
if (rc == 0) {
bool is_unicode;

spin_lock(&cifs_tcp_ses_lock);
tcon->tidStatus = CifsGood;
spin_unlock(&cifs_tcp_ses_lock);
tcon->need_reconnect = false;
tcon->tid = smb_buffer_response->Tid;
bcc_ptr = pByteArea(smb_buffer_response);
Expand Down Expand Up @@ -3859,12 +3875,12 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,

rc = server->ops->negotiate(xid, ses, server);
if (rc == 0) {
spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus == CifsNeedNegotiate)
server->tcpStatus = CifsGood;
else
rc = -EHOSTDOWN;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);
}

return rc;
Expand Down
4 changes: 2 additions & 2 deletions fs/cifs/netmisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -896,10 +896,10 @@ map_and_check_smb_error(struct mid_q_entry *mid, bool logErr)
if (class == ERRSRV && code == ERRbaduid) {
cifs_dbg(FYI, "Server returned 0x%x, reconnecting session...\n",
code);
spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
if (mid->server->tcpStatus != CifsExiting)
mid->server->tcpStatus = CifsNeedReconnect;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);
}
}

Expand Down
8 changes: 4 additions & 4 deletions fs/cifs/sess.c
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,10 @@ void cifs_ses_mark_for_reconnect(struct cifs_ses *ses)
int i;

for (i = 0; i < ses->chan_count; i++) {
spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
if (ses->chans[i].server->tcpStatus != CifsExiting)
ses->chans[i].server->tcpStatus = CifsNeedReconnect;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);
}
}

Expand Down Expand Up @@ -1052,9 +1052,9 @@ sess_establish_session(struct sess_data *sess_data)
spin_unlock(&ses->chan_lock);

/* Even if one channel is active, session is in good state */
spin_lock(&GlobalMid_Lock);
spin_lock(&cifs_tcp_ses_lock);
ses->status = CifsGood;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);

return 0;
}
Expand Down
11 changes: 9 additions & 2 deletions fs/cifs/smb1ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
{
__u64 mid = 0;
__u16 last_mid, cur_mid;
bool collision;
bool collision, reconnect;

spin_lock(&GlobalMid_Lock);

Expand Down Expand Up @@ -215,7 +215,7 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
* an eventual reconnect to clean out the pending_mid_q.
*/
if (num_mids > 32768)
server->tcpStatus = CifsNeedReconnect;
reconnect = true;

if (!collision) {
mid = (__u64)cur_mid;
Expand All @@ -225,6 +225,13 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
cur_mid++;
}
spin_unlock(&GlobalMid_Lock);

if (reconnect) {
spin_lock(&cifs_tcp_ses_lock);
server->tcpStatus = CifsNeedReconnect;
spin_unlock(&cifs_tcp_ses_lock);
}

return mid;
}

Expand Down
15 changes: 13 additions & 2 deletions fs/cifs/smb2ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,13 @@ smb2_add_credits(struct TCP_Server_Info *server,
optype, scredits, add);
}

spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus == CifsNeedReconnect
|| server->tcpStatus == CifsExiting)
|| server->tcpStatus == CifsExiting) {
spin_unlock(&cifs_tcp_ses_lock);
return;
}
spin_unlock(&cifs_tcp_ses_lock);

switch (rc) {
case -1:
Expand Down Expand Up @@ -208,11 +212,15 @@ smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
return rc;
spin_lock(&server->req_lock);
} else {
spin_unlock(&server->req_lock);
spin_lock(&cifs_tcp_ses_lock);
if (server->tcpStatus == CifsExiting) {
spin_unlock(&server->req_lock);
spin_unlock(&cifs_tcp_ses_lock);
return -ENOENT;
}
spin_unlock(&cifs_tcp_ses_lock);

spin_lock(&server->req_lock);
scredits = server->credits;
/* can deadlock with reopen */
if (scredits <= 8) {
Expand Down Expand Up @@ -4983,17 +4991,20 @@ static void smb2_decrypt_offload(struct work_struct *work)

mid->callback(mid);
} else {
spin_lock(&cifs_tcp_ses_lock);
spin_lock(&GlobalMid_Lock);
if (dw->server->tcpStatus == CifsNeedReconnect) {
mid->mid_state = MID_RETRY_NEEDED;
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);
mid->callback(mid);
} else {
mid->mid_state = MID_REQUEST_SUBMITTED;
mid->mid_flags &= ~(MID_DELETED);
list_add_tail(&mid->qhead,
&dw->server->pending_mid_q);
spin_unlock(&GlobalMid_Lock);
spin_unlock(&cifs_tcp_ses_lock);
}
}
cifs_mid_q_entry_release(mid);
Expand Down
Loading

0 comments on commit 220c5bc

Please sign in to comment.