Received-SPF: pass (google.com: domain of linux-kernel+bounces-28467-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240116105134.2245640-1-colin.i.king@gmail.com> <CAH2r5mvf+ZMyqpnFQUaO=DWC8ixXspsjWKE7BxQ1wW4WuvTVcA@mail.gmail.com>
In-Reply-To: <CAH2r5mvf+ZMyqpnFQUaO=DWC8ixXspsjWKE7BxQ1wW4WuvTVcA@mail.gmail.com>
From: Shyam Prasad N <nspmangalore@gmail.com>
Date: Wed, 17 Jan 2024 08:07:17 +0530
Message-ID: <CANT5p=r=y4BN-1eeSur_zWCx=R1fn+8OU-3nv+AffraPjja+qQ@mail.gmail.com>
Subject: Re: [PATCH][next] cifs: remove redundant variable tcon_exist
To: Steve French <smfrench@gmail.com>
Cc: Colin Ian King <colin.i.king@gmail.com>, Steve French <sfrench@samba.org>, 
	Paulo Alcantara <pc@manguebit.com>, Shyam Prasad N <sprasad@microsoft.com>, Tom Talpey <tom@talpey.com>, 
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org, 
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 17, 2024 at 4:47=E2=80=AFAM Steve French <smfrench@gmail.com> w=
rote:
>
> 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_sessi=
on
>     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=E2=80=AFAM Colin Ian King <colin.i.king@gmai=
l.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 *wo=
rk)
> >         struct cifs_ses *ses, *ses2;
> >         struct cifs_tcon *tcon, *tcon2;
> >         struct list_head tmp_list, tmp_ses_list;
> > -       bool tcon_exist =3D false, ses_exist =3D false;
> > +       bool ses_exist =3D false;
> >         bool tcon_selected =3D false;
> >         int rc;
> >         bool resched =3D false;
> > @@ -3964,7 +3964,7 @@ void smb2_reconnect_server(struct work_struct *wo=
rk)
> >                         if (tcon->need_reconnect || tcon->need_reopen_f=
iles) {
> >                                 tcon->tc_count++;
> >                                 list_add_tail(&tcon->rlist, &tmp_list);
> > -                               tcon_selected =3D tcon_exist =3D true;
> > +                               tcon_selected =3D true;
> >                         }
> >                 }
> >                 /*
> > @@ -3973,7 +3973,7 @@ void smb2_reconnect_server(struct work_struct *wo=
rk)
> >                  */
> >                 if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
> >                         list_add_tail(&ses->tcon_ipc->rlist, &tmp_list)=
;
> > -                       tcon_selected =3D tcon_exist =3D true;
> > +                       tcon_selected =3D true;
> >                         cifs_smb_ses_inc_refcount(ses);
> >                 }
> >                 /*
> > --
> > 2.39.2
> >
> >
>
>
> --
> Thanks,
>
> Steve
>

The change looks good to me.

--=20
Regards,
Shyam