From: Steve French <smfrench@gmail.com>
To: CIFS <linux-cifs@vger.kernel.org>
Cc: Shyam Prasad N <nspmangalore@gmail.com>
Subject: [PATCH][CIFS] cleanup and clarify status of tree connections
Date: Sun, 27 Mar 2022 16:14:11 -0500 [thread overview]
Message-ID: <CAH2r5msLJ166+yuHWQnV7_10_SZ3R1RmMwgLyGMBbggcYks1hQ@mail.gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]
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
See attached.
Also FYI - Shyam had a work in progress patch to fix and clarify the
ses->status enum
--
Thanks,
Steve
[-- Attachment #2: 0001-smb3-cleanup-and-clarify-status-of-tree-connections.patch --]
[-- Type: text/x-patch, Size: 9315 bytes --]
From 7e5c8c02911ba8d7e61d4fbd130215318343cf60 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Sun, 27 Mar 2022 16:07:30 -0500
Subject: [PATCH] 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.
Signed-off-by: Steve French <stfrench@microsoft.com>
---
fs/cifs/cifs_debug.c | 2 +-
fs/cifs/cifsfs.c | 4 ++--
fs/cifs/cifsglob.h | 18 +++++++++++++-----
fs/cifs/cifssmb.c | 11 +++++------
fs/cifs/connect.c | 32 ++++++++++++++++----------------
fs/cifs/misc.c | 2 +-
fs/cifs/smb2pdu.c | 4 ++--
7 files changed, 40 insertions(+), 33 deletions(-)
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index ea00e1a91250..9d334816eac0 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -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->fsAttrInfo.Attributes),
le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength),
- tcon->tidStatus);
+ tcon->status);
if (dev_type == FILE_DEVICE_DISK)
seq_puts(m, " type: DISK ");
else if (dev_type == FILE_DEVICE_CD_ROM)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 6e5246122ee2..969ec2308775 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -699,14 +699,14 @@ static void cifs_umount_begin(struct super_block *sb)
tcon = cifs_sb_master_tcon(cifs_sb);
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
already tried to force umount this and woken up
all waiting network requests, nothing to do */
spin_unlock(&cifs_tcp_ses_lock);
return;
} else if (tcon->tc_count == 1)
- tcon->tidStatus = CifsExiting;
+ tcon->status = CifsExiting;
spin_unlock(&cifs_tcp_ses_lock);
/* cancel_brl_requests(tcon); */ /* BB mark all brl mids as exiting */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ad3cd6053f4e..cd9127510a55 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -115,10 +115,18 @@ enum statusEnum {
CifsInNegotiate,
CifsNeedSessSetup,
CifsInSessSetup,
- CifsNeedTcon,
- CifsInTcon,
- CifsNeedFilesInvalidate,
- CifsInFilesInvalidate
+};
+
+/* associated with each tree connection to the server */
+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 {
@@ -1032,7 +1040,7 @@ struct cifs_tcon {
char *password; /* for share-level security */
__u32 tid; /* The 4 byte tree id */
__u16 Flags; /* optional support bits */
- enum statusEnum tidStatus;
+ enum tid_status_enum status;
atomic_t num_smbs_sent;
union {
struct {
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 071e2f21a7db..aca9338b0877 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -75,12 +75,11 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
/* only send once per connect */
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->ses->status != CifsGood ||
- tcon->tidStatus != CifsNeedReconnect) {
+ if ((tcon->ses->status != CifsGood) || (tcon->status != TID_NEED_RECON)) {
spin_unlock(&cifs_tcp_ses_lock);
return;
}
- tcon->tidStatus = CifsInFilesInvalidate;
+ tcon->status = TID_IN_FILES_INVALIDATE;
spin_unlock(&cifs_tcp_ses_lock);
/* 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);
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsInFilesInvalidate)
- tcon->tidStatus = CifsNeedTcon;
+ if (tcon->status == TID_IN_FILES_INVALIDATE)
+ tcon->status = TID_NEED_TCON;
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
*/
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsExiting) {
+ if (tcon->status == TID_EXITING) {
if (smb_command != SMB_COM_WRITE_ANDX &&
smb_command != SMB_COM_OPEN_ANDX &&
smb_command != SMB_COM_TREE_DISCONNECT) {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index d6f8ccc7bfe2..ee3b7c15e884 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -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) {
tcon->need_reconnect = true;
- tcon->tidStatus = CifsNeedReconnect;
+ tcon->status = TID_NEED_RECON;
}
if (ses->tcon_ipc)
ses->tcon_ipc->need_reconnect = true;
@@ -2207,7 +2207,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, 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;
if (strncmp(tcon->treeName, ctx->UNC, MAX_TREE_SIZE))
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 */
spin_lock(&cifs_tcp_ses_lock);
if (tcon->ses->status != CifsGood ||
- (tcon->tidStatus != CifsNew &&
- tcon->tidStatus != CifsNeedTcon)) {
+ (tcon->status != TID_NEW &&
+ tcon->status != TID_NEED_TCON)) {
spin_unlock(&cifs_tcp_ses_lock);
return 0;
}
- tcon->tidStatus = CifsInTcon;
+ tcon->status = TID_IN_TCON;
spin_unlock(&cifs_tcp_ses_lock);
tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
@@ -4532,13 +4532,13 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
if (rc) {
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsInTcon)
- tcon->tidStatus = CifsNeedTcon;
+ if (tcon->status == TID_IN_TCON)
+ tcon->status = TID_NEED_TCON;
spin_unlock(&cifs_tcp_ses_lock);
} else {
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsInTcon)
- tcon->tidStatus = CifsGood;
+ if (tcon->status == TID_IN_TCON)
+ tcon->status = TID_GOOD;
spin_unlock(&cifs_tcp_ses_lock);
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 */
spin_lock(&cifs_tcp_ses_lock);
if (tcon->ses->status != CifsGood ||
- (tcon->tidStatus != CifsNew &&
- tcon->tidStatus != CifsNeedTcon)) {
+ (tcon->status != TID_NEW &&
+ tcon->status != TID_NEED_TCON)) {
spin_unlock(&cifs_tcp_ses_lock);
return 0;
}
- tcon->tidStatus = CifsInTcon;
+ tcon->status = TID_IN_TCON;
spin_unlock(&cifs_tcp_ses_lock);
rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
if (rc) {
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsInTcon)
- tcon->tidStatus = CifsNeedTcon;
+ if (tcon->status == TID_IN_TCON)
+ tcon->status = TID_NEED_TCON;
spin_unlock(&cifs_tcp_ses_lock);
} else {
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsInTcon)
- tcon->tidStatus = CifsGood;
+ if (tcon->status == TID_IN_TCON)
+ tcon->status = TID_GOOD;
spin_unlock(&cifs_tcp_ses_lock);
tcon->need_reconnect = false;
}
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 56598f7dbe00..afaf59c22193 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -116,7 +116,7 @@ tconInfoAlloc(void)
}
atomic_inc(&tconInfoAllocCount);
- ret_buf->tidStatus = CifsNew;
+ ret_buf->status = TID_NEW;
++ret_buf->tc_count;
INIT_LIST_HEAD(&ret_buf->openFileList);
INIT_LIST_HEAD(&ret_buf->tcon_list);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 54b554c7aee8..1b7ad0c09566 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -163,7 +163,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
return 0;
spin_lock(&cifs_tcp_ses_lock);
- if (tcon->tidStatus == CifsExiting) {
+ if (tcon->status == TID_EXITING) {
/*
* only tree disconnect, open, and write,
* (and ulogoff which does not have tcon)
@@ -3860,7 +3860,7 @@ void smb2_reconnect_server(struct work_struct *work)
goto done;
}
- tcon->tidStatus = CifsGood;
+ tcon->status = TID_GOOD;
tcon->retry = false;
tcon->need_reconnect = false;
--
2.32.0
next reply other threads:[~2022-03-27 21:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-27 21:14 Steve French [this message]
2022-03-28 1:47 ` [PATCH][CIFS] cleanup and clarify status of tree connections Steve French
2022-03-28 16:27 ` Shyam Prasad N
2022-03-28 17:13 ` Steve French
2022-03-28 22:03 ` ronnie sahlberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAH2r5msLJ166+yuHWQnV7_10_SZ3R1RmMwgLyGMBbggcYks1hQ@mail.gmail.com \
--to=smfrench@gmail.com \
--cc=linux-cifs@vger.kernel.org \
--cc=nspmangalore@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.