smb3: cleanup and clarify status of tree connections

Currently the way the tid (tree connection) status is tracked
is confusing.  The same enum is used for structs cifs_tcon
and cifs_ses and TCP_Server_info, but each of these three has
different states that they transition among.  The current
code also unnecessarily uses camelCase.

Convert from use of statusEnum to a new tid_status_enum for
tree connections.  The valid states for a tid are:

        TID_NEW = 0,
        TID_GOOD,
        TID_EXITING,
        TID_NEED_RECON,
        TID_NEED_TCON,
        TID_IN_TCON,
        TID_NEED_FILES_INVALIDATE, /* unused, considering removing in future */
        TID_IN_FILES_INVALIDATE

It also removes CifsNeedTcon, CifsInTcon, CifsNeedFilesInvalidate and
CifsInFilesInvalidate from the statusEnum used for session and
TCP_Server_Info since they are not relevant for those.

A follow on patch will fix the places where we use the
tcon->need_reconnect flag to be more consistent with the tid->status.

Also fixes a bug that was:
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
This commit is contained in:
Steve French 2022-03-27 16:07:30 -05:00
parent be13500043
commit fdf59eb548
7 changed files with 40 additions and 33 deletions

View File

@ -94,7 +94,7 @@ static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon)
le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics), le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics),
le32_to_cpu(tcon->fsAttrInfo.Attributes), le32_to_cpu(tcon->fsAttrInfo.Attributes),
le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength), le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength),
tcon->tidStatus); tcon->status);
if (dev_type == FILE_DEVICE_DISK) if (dev_type == FILE_DEVICE_DISK)
seq_puts(m, " type: DISK "); seq_puts(m, " type: DISK ");
else if (dev_type == FILE_DEVICE_CD_ROM) else if (dev_type == FILE_DEVICE_CD_ROM)

View File

@ -699,14 +699,14 @@ static void cifs_umount_begin(struct super_block *sb)
tcon = cifs_sb_master_tcon(cifs_sb); tcon = cifs_sb_master_tcon(cifs_sb);
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if ((tcon->tc_count > 1) || (tcon->tidStatus == CifsExiting)) { if ((tcon->tc_count > 1) || (tcon->status == TID_EXITING)) {
/* we have other mounts to same share or we have /* we have other mounts to same share or we have
already tried to force umount this and woken up already tried to force umount this and woken up
all waiting network requests, nothing to do */ all waiting network requests, nothing to do */
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
return; return;
} else if (tcon->tc_count == 1) } else if (tcon->tc_count == 1)
tcon->tidStatus = CifsExiting; tcon->status = TID_EXITING;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
/* cancel_brl_requests(tcon); */ /* BB mark all brl mids as exiting */ /* cancel_brl_requests(tcon); */ /* BB mark all brl mids as exiting */

View File

@ -115,10 +115,18 @@ enum statusEnum {
CifsInNegotiate, CifsInNegotiate,
CifsNeedSessSetup, CifsNeedSessSetup,
CifsInSessSetup, CifsInSessSetup,
CifsNeedTcon, };
CifsInTcon,
CifsNeedFilesInvalidate, /* associated with each tree connection to the server */
CifsInFilesInvalidate enum tid_status_enum {
TID_NEW = 0,
TID_GOOD,
TID_EXITING,
TID_NEED_RECON,
TID_NEED_TCON,
TID_IN_TCON,
TID_NEED_FILES_INVALIDATE, /* currently unused */
TID_IN_FILES_INVALIDATE
}; };
enum securityEnum { enum securityEnum {
@ -1032,7 +1040,7 @@ struct cifs_tcon {
char *password; /* for share-level security */ char *password; /* for share-level security */
__u32 tid; /* The 4 byte tree id */ __u32 tid; /* The 4 byte tree id */
__u16 Flags; /* optional support bits */ __u16 Flags; /* optional support bits */
enum statusEnum tidStatus; enum tid_status_enum status;
atomic_t num_smbs_sent; atomic_t num_smbs_sent;
union { union {
struct { struct {

View File

@ -75,12 +75,11 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
/* only send once per connect */ /* only send once per connect */
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->ses->status != CifsGood || if ((tcon->ses->status != CifsGood) || (tcon->status != TID_NEED_RECON)) {
tcon->tidStatus != CifsNeedReconnect) {
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
return; return;
} }
tcon->tidStatus = CifsInFilesInvalidate; tcon->status = TID_IN_FILES_INVALIDATE;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
/* list all files open on tree connection and mark them invalid */ /* list all files open on tree connection and mark them invalid */
@ -100,8 +99,8 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
mutex_unlock(&tcon->crfid.fid_mutex); mutex_unlock(&tcon->crfid.fid_mutex);
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsInFilesInvalidate) if (tcon->status == TID_IN_FILES_INVALIDATE)
tcon->tidStatus = CifsNeedTcon; tcon->status = TID_NEED_TCON;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
/* /*
@ -136,7 +135,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
* have tcon) are allowed as we start force umount * have tcon) are allowed as we start force umount
*/ */
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsExiting) { if (tcon->status == TID_EXITING) {
if (smb_command != SMB_COM_WRITE_ANDX && if (smb_command != SMB_COM_WRITE_ANDX &&
smb_command != SMB_COM_OPEN_ANDX && smb_command != SMB_COM_OPEN_ANDX &&
smb_command != SMB_COM_TREE_DISCONNECT) { smb_command != SMB_COM_TREE_DISCONNECT) {

View File

@ -245,7 +245,7 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
list_for_each_entry(tcon, &ses->tcon_list, tcon_list) { list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
tcon->need_reconnect = true; tcon->need_reconnect = true;
tcon->tidStatus = CifsNeedReconnect; tcon->status = TID_NEED_RECON;
} }
if (ses->tcon_ipc) if (ses->tcon_ipc)
ses->tcon_ipc->need_reconnect = true; ses->tcon_ipc->need_reconnect = true;
@ -2207,7 +2207,7 @@ get_ses_fail:
static int match_tcon(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) static int match_tcon(struct cifs_tcon *tcon, struct smb3_fs_context *ctx)
{ {
if (tcon->tidStatus == CifsExiting) if (tcon->status == TID_EXITING)
return 0; return 0;
if (strncmp(tcon->treeName, ctx->UNC, MAX_TREE_SIZE)) if (strncmp(tcon->treeName, ctx->UNC, MAX_TREE_SIZE))
return 0; return 0;
@ -4486,12 +4486,12 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
/* only send once per connect */ /* only send once per connect */
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->ses->status != CifsGood || if (tcon->ses->status != CifsGood ||
(tcon->tidStatus != CifsNew && (tcon->status != TID_NEW &&
tcon->tidStatus != CifsNeedTcon)) { tcon->status != TID_NEED_TCON)) {
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
return 0; return 0;
} }
tcon->tidStatus = CifsInTcon; tcon->status = TID_IN_TCON;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL); tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
@ -4532,13 +4532,13 @@ out:
if (rc) { if (rc) {
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsInTcon) if (tcon->status == TID_IN_TCON)
tcon->tidStatus = CifsNeedTcon; tcon->status = TID_NEED_TCON;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
} else { } else {
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsInTcon) if (tcon->status == TID_IN_TCON)
tcon->tidStatus = CifsGood; tcon->status = TID_GOOD;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
tcon->need_reconnect = false; tcon->need_reconnect = false;
} }
@ -4554,24 +4554,24 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
/* only send once per connect */ /* only send once per connect */
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->ses->status != CifsGood || if (tcon->ses->status != CifsGood ||
(tcon->tidStatus != CifsNew && (tcon->status != TID_NEW &&
tcon->tidStatus != CifsNeedTcon)) { tcon->status != TID_NEED_TCON)) {
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
return 0; return 0;
} }
tcon->tidStatus = CifsInTcon; tcon->status = TID_IN_TCON;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc); rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
if (rc) { if (rc) {
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsInTcon) if (tcon->status == TID_IN_TCON)
tcon->tidStatus = CifsNeedTcon; tcon->status = TID_NEED_TCON;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
} else { } else {
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsInTcon) if (tcon->status == TID_IN_TCON)
tcon->tidStatus = CifsGood; tcon->status = TID_GOOD;
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
tcon->need_reconnect = false; tcon->need_reconnect = false;
} }

View File

@ -116,7 +116,7 @@ tconInfoAlloc(void)
} }
atomic_inc(&tconInfoAllocCount); atomic_inc(&tconInfoAllocCount);
ret_buf->tidStatus = CifsNew; ret_buf->status = TID_NEW;
++ret_buf->tc_count; ++ret_buf->tc_count;
INIT_LIST_HEAD(&ret_buf->openFileList); INIT_LIST_HEAD(&ret_buf->openFileList);
INIT_LIST_HEAD(&ret_buf->tcon_list); INIT_LIST_HEAD(&ret_buf->tcon_list);

View File

@ -163,7 +163,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
return 0; return 0;
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
if (tcon->tidStatus == CifsExiting) { if (tcon->status == TID_EXITING) {
/* /*
* only tree disconnect, open, and write, * only tree disconnect, open, and write,
* (and ulogoff which does not have tcon) * (and ulogoff which does not have tcon)
@ -3860,7 +3860,7 @@ void smb2_reconnect_server(struct work_struct *work)
goto done; goto done;
} }
tcon->tidStatus = CifsGood; tcon->status = TID_GOOD;
tcon->retry = false; tcon->retry = false;
tcon->need_reconnect = false; tcon->need_reconnect = false;