Re: [PATCH][next] cifs: remove redundant variable tcon_exist

From: Steve French
Date: Tue Jan 16 2024 - 18:17:53 EST


Yes - it looks like Shyam's commit made that variable obsolete.

Shyam/Paulo,
Let me know if any objections. Will put into cifs-2.6.git for-next

commit 04909192ada3285070f8ced0af7f07735478b364 (tag: 6.7-rc4-smb3-client-fixes)
Author: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Date: Wed Dec 6 16:37:38 2023 +0000

cifs: reconnect worker should take reference on server struct
unconditionally

Reconnect worker currently assumes that the server struct
is alive and only takes reference on the server if it needs
to call smb2_reconnect.

With the new ability to disable channels based on whether the
server has multichannel disabled, this becomes a problem when
we need to disable established channels. While disabling the
channels and deallocating the server, there could be reconnect
work that could not be cancelled (because it started).

This change forces the reconnect worker to unconditionally
take a reference on the server when it runs.

Also, this change now allows smb2_reconnect to know if it was
called by the reconnect worker. Based on this, the cifs_put_tcp_session
can decide whether it can cancel the reconnect work synchronously or not.

Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>

On Tue, Jan 16, 2024 at 4:51 AM Colin Ian King <colin.i.king@xxxxxxxxx> wrote:
>
> The variable tcon_exist is being assigned however it is never read, the
> variable is redundant and can be removed.
>
> Cleans up clang scan build warning:
> warning: Although the value stored to 'tcon_exist' is used in
> the enclosing expression, the value is never actually readfrom
> 'tcon_exist' [deadcode.DeadStores]
>
> Signed-off-by: Colin Ian King <colin.i.king@xxxxxxxxx>
> ---
> fs/smb/client/smb2pdu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index bd25c34dc398..50f6bf16b624 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -3918,7 +3918,7 @@ void smb2_reconnect_server(struct work_struct *work)
> struct cifs_ses *ses, *ses2;
> struct cifs_tcon *tcon, *tcon2;
> struct list_head tmp_list, tmp_ses_list;
> - bool tcon_exist = false, ses_exist = false;
> + bool ses_exist = false;
> bool tcon_selected = false;
> int rc;
> bool resched = false;
> @@ -3964,7 +3964,7 @@ void smb2_reconnect_server(struct work_struct *work)
> if (tcon->need_reconnect || tcon->need_reopen_files) {
> tcon->tc_count++;
> list_add_tail(&tcon->rlist, &tmp_list);
> - tcon_selected = tcon_exist = true;
> + tcon_selected = true;
> }
> }
> /*
> @@ -3973,7 +3973,7 @@ void smb2_reconnect_server(struct work_struct *work)
> */
> if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
> list_add_tail(&ses->tcon_ipc->rlist, &tmp_list);
> - tcon_selected = tcon_exist = true;
> + tcon_selected = true;
> cifs_smb_ses_inc_refcount(ses);
> }
> /*
> --
> 2.39.2
>
>


--
Thanks,

Steve