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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Steve French <[email protected]>
On Tue, Jan 16, 2024 at 4:51 AM Colin Ian King <[email protected]> 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 <[email protected]>
> ---
> 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
On Wed, Jan 17, 2024 at 4:47 AM Steve French <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Steve French <[email protected]>
>
> On Tue, Jan 16, 2024 at 4:51 AM Colin Ian King <[email protected]> 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 <[email protected]>
> > ---
> > 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.
--
Regards,
Shyam