Both patches look good. Thanks!
Reviewed-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
--
Best regards,
Pavel Shilovsky
вс, 21 февр. 2021 г. в 22:44, Rohith <rohiths.msft@xxxxxxxxx>:
>
> Thanks Steve. Will make sure to run checkpatch for further patches.
>
>
>
> Regards,
>
> Rohith
>
>
>
> Sent from Mail for Windows 10
>
>
>
> From: Steve French
> Sent: Sunday, February 21, 2021 6:26 AM
> To: Pavel Shilovsky
> Cc: Rohith; linux-cifs@xxxxxxxxxxxxxxx; Shyam Prasad N; sribhat.msa@xxxxxxxxxxx
> Subject: Re: TCON reconnect during STATUS_NETWORK_NAME_DELETED
>
>
>
> The remaining suggestion of Pavel's is included in this trivial cleanup.
>
>
>
> Trivial change to clarify code in smb2_is_network_name_deleted
>
> Suggested-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
> ---
> fs/cifs/smb2ops.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index aee9da88a333..b2191babce26 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -2459,23 +2459,24 @@ smb2_is_network_name_deleted(char *buf, struct TCP_Server_Info *server)
> struct cifs_ses *ses;
> struct cifs_tcon *tcon;
>
> - if (shdr->Status == STATUS_NETWORK_NAME_DELETED) {
> - spin_lock(&cifs_tcp_ses_lock);
> - list_for_each(tmp, &server->smb_ses_list) {
> - ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
> - list_for_each(tmp1, &ses->tcon_list) {
> - tcon = list_entry(tmp1, struct cifs_tcon, tcon_list);
> - if (tcon->tid == shdr->TreeId) {
> - tcon->need_reconnect = true;
> - spin_unlock(&cifs_tcp_ses_lock);
> - pr_warn_once("Server share %s deleted.\n",
> - tcon->treeName);
> - return;
> - }
> + if (shdr->Status != STATUS_NETWORK_NAME_DELETED)
> + return;
> +
> + spin_lock(&cifs_tcp_ses_lock);
> + list_for_each(tmp, &server->smb_ses_list) {
> + ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
> + list_for_each(tmp1, &ses->tcon_list) {
> + tcon = list_entry(tmp1, struct cifs_tcon, tcon_list);
> + if (tcon->tid == shdr->TreeId) {
> + tcon->need_reconnect = true;
> + spin_unlock(&cifs_tcp_ses_lock);
> + pr_warn_once("Server share %s deleted.\n",
> + tcon->treeName);
> + return;
> }
> }
> - spin_unlock(&cifs_tcp_ses_lock);
> }
> + spin_unlock(&cifs_tcp_ses_lock);
> }
>
> static int
>
>
>
> On Sat, Feb 20, 2021 at 5:38 PM Steve French <smfrench@xxxxxxxxx> wrote:
>
> I corrected various checkpatch errors - including two of those Pavel noted, and tentatively merged into cifs-2.6.git for-next. See attached updated patch.
>
>
>
> Will fix the indentation next.
>
>
>
> On Sat, Feb 20, 2021 at 12:18 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> чт, 18 февр. 2021 г. в 08:10, Rohith <rohiths.msft@xxxxxxxxx>:
> >
> > Hi Pavel,
> >
> >
> >
> > Addressed review comments. Can you please take a look.
> >
> >
> >
> > >> Btw, I think is_status_io timeout should be called in this loop for
> >
> > >> every mid not outside the loop (sorry, missed that in the original
> >
> > >> review). Could you please fix that separately?
> >
> >
> >
> > Yes, will send out different patch for status_io_timeout.
>
> Thanks!
>
> The patch looks good. Please find my minor comments below:
>
> 1.
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 10fe6d6d2dee..b2f51546c2ef 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -993,6 +993,10 @@ cifs_demultiplex_thread(void *p)
> if (mids[i] != NULL) {
> mids[i]->resp_buf_size = server->pdu_size;
>
> + if (server->ops->is_network_name_deleted) {
> + server->ops->is_network_name_deleted(bufs[i],
> + server);
> + }
>
> ^^^
> remove extra {}
>
> 2.
> +static void
> +smb2_is_network_name_deleted(char *buf, struct TCP_Server_Info *server)
> +{
> + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf;
>
> + struct list_head *tmp, *tmp1;
> + struct cifs_ses *ses;
> + struct cifs_tcon *tcon;
> +
> + if (shdr->Status == STATUS_NETWORK_NAME_DELETED) {
>
> ^^^
> let's do the opposite check: if status is not
> STATUS_NETWORK_NAME_DELETED then return immediately from the function.
> This will remove overall indentation for the nested for loops.
>
> 3.
> @@ -4605,6 +4632,10 @@ static void smb2_decrypt_offload(struct
> work_struct *work)
> #ifdef CONFIG_CIFS_STATS2
> mid->when_received = jiffies;
> #endif
> + if (dw->server->ops->is_network_name_deleted) {
> + dw->server->ops->is_network_name_deleted(dw->buf,
> + dw->server);
> + }
>
> ^^^
> remove extra {}
>
> --
> Best regards,
> Pavel Shilovsky
>
>
>
>
> --
>
> Thanks,
>
> Steve
>
>
>
>
> --
>
> Thanks,
>
> Steve
>
>
