Received-SPF: pass (google.com: domain of linux-kernel+bounces-28373-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240116105134.2245640-1-colin.i.king@gmail.com>
In-Reply-To: <20240116105134.2245640-1-colin.i.king@gmail.com>
From: Steve French <smfrench@gmail.com>
Date: Tue, 16 Jan 2024 17:17:26 -0600
Message-ID: <CAH2r5mvf+ZMyqpnFQUaO=DWC8ixXspsjWKE7BxQ1wW4WuvTVcA@mail.gmail.com>
Subject: Re: [PATCH][next] cifs: remove redundant variable tcon_exist
To: Colin Ian King <colin.i.king@gmail.com>
Cc: 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

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-f=
ixes)
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 no=
t.

    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@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 =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 *work=
)
>                         if (tcon->need_reconnect || tcon->need_reopen_fil=
es) {
>                                 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 *work=
)
>                  */
>                 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
>
>


--=20
Thanks,

Steve