diff mbox series

[next] cifs: remove redundant variable tcon_exist

Message ID 20240116105134.2245640-1-colin.i.king@gmail.com
State New
Headers show
Series [next] cifs: remove redundant variable tcon_exist | expand

Commit Message

Colin Ian King Jan. 16, 2024, 10:51 a.m. UTC
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@gmail.com>
---
 fs/smb/client/smb2pdu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Steve French Jan. 16, 2024, 11:17 p.m. UTC | #1
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@microsoft.com>
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@microsoft.com>
    Signed-off-by: Steve French <stfrench@microsoft.com>

On Tue, Jan 16, 2024 at 4:51 AM Colin Ian King <colin.i.king@gmail.com> 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@gmail.com>
> ---
>  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
>
>
Shyam Prasad N Jan. 17, 2024, 2:37 a.m. UTC | #2
On Wed, Jan 17, 2024 at 4:47 AM Steve French <smfrench@gmail.com> wrote:
>
> 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@microsoft.com>
> 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@microsoft.com>
>     Signed-off-by: Steve French <stfrench@microsoft.com>
>
> On Tue, Jan 16, 2024 at 4:51 AM Colin Ian King <colin.i.king@gmail.com> 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@gmail.com>
> > ---
> >  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
>

The change looks good to me.
diff mbox series

Patch

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);
 		}
 		/*