From patchwork Sun Mar 27 21:14:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 12792920 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AD204C433F5 for ; Sun, 27 Mar 2022 21:14:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234976AbiC0VQE (ORCPT ); Sun, 27 Mar 2022 17:16:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56162 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229878AbiC0VQE (ORCPT ); Sun, 27 Mar 2022 17:16:04 -0400 Received: from mail-lf1-x130.google.com (mail-lf1-x130.google.com [IPv6:2a00:1450:4864:20::130]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00AF7FD39 for ; Sun, 27 Mar 2022 14:14:23 -0700 (PDT) Received: by mail-lf1-x130.google.com with SMTP id h7so21659254lfl.2 for ; Sun, 27 Mar 2022 14:14:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:from:date:message-id:subject:to:cc; bh=KUa5DYOJ4T6jluRjP1Nt1GL+UeXKM7tjDkylxBT9uoc=; b=NTQ002mkXUy7GXRDWYEa2ZRqr+gw2j/QkDI1AXhX3yX1+6E6lZqt6El/IHKH5KtcSD hV19dBnTLdW+F8c3aFPY3F1/9p8cFRz/0jJMyVIKp290MQ9fMKhrV6CGpTtRCai6jGOv SmUj8X6AIHyXJbIROL1zktsGSc9YuISLC371JXWK2fUj97boRyLVhFVJ9l2oUVjy9H8C ZttFrVGbFVZSelADZif9xBLT1F6scWWg+Hqu752yzTYvTetqhMxpW7vOpUqUwoNJ13hr g8e+EI27fDpIlgaEdY18tRjbNZ52JP4gg8Qa5ARPZjpgyEqVWqSIW/B9oiDTjlktGq/n KccA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=KUa5DYOJ4T6jluRjP1Nt1GL+UeXKM7tjDkylxBT9uoc=; b=3u0Rl/pc/981h7FqUolI3ylEg8FUTKWoFuRTx9HMIRk3rH45VdZeSg7fARs6BRFE9T gYhFcfrYfzl94dhJSoQA8p57kKeDZGqLGblOaSFBIly1UMXpb26QQiCMRdVN0yEUKTLC kyYVz4irZBTFdEgWNiHqM6I0aHSENRmx/98JPi+KLhIK/wlgd8dLYnjB7tipQkWRnjwt 6i3T9tojDCLoZJyjSg2pCcSIU22xEX2BWKfm2dkEiYdh7Rinul5RvGEP9J6/fWQhaWM0 DfMg2F5we8WnJjmR9TM4jVpUBLZ0JJOZ4AJgAVzQWNZfhZX4g7t4X8yzo2NvU9cRACtZ ozlQ== X-Gm-Message-State: AOAM532HRLugCWTiUR/6/PlPNe8jD62ZuSbUsLu99OjJXRyXW9D0Ldag 5HtVp2SSw2r/dYWrs4rWOhewrXN9rPpV+FtjrwRRfCV8PS8= X-Google-Smtp-Source: ABdhPJybe7gu0JGfSAw3kzxjUK/DdVAAWjn1c8y3B2fnKFcbA5HKPvNhvKymApP+w36LWUYB6RtC4japc7rTDQFE0i4= X-Received: by 2002:ac2:5444:0:b0:44a:846e:ad2b with SMTP id d4-20020ac25444000000b0044a846ead2bmr7138050lfn.545.1648415662027; Sun, 27 Mar 2022 14:14:22 -0700 (PDT) MIME-Version: 1.0 From: Steve French Date: Sun, 27 Mar 2022 16:14:11 -0500 Message-ID: Subject: [PATCH][CIFS] cleanup and clarify status of tree connections To: CIFS Cc: Shyam Prasad N Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org 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 From 7e5c8c02911ba8d7e61d4fbd130215318343cf60 Mon Sep 17 00:00:00 2001 From: Steve French 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 --- 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