Re: [PATCH][CIFS] cleanup and clarify status of tree connections

[Steve French <smfrench@gmail.com>]

On Mon, Mar 28, 2022 at 11:27 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> On Mon, Mar 28, 2022 at 7:17 AM Steve French <smfrench@gmail.com> wrote:
> >
> > Updated patch to fix one place I missed pointed out by the kernel test robot.
> >
> > See attached.
> >
> > On Sun, Mar 27, 2022 at 4:14 PM Steve French <smfrench@gmail.com> wrote:
> > >
> > > Currently the way the tid (tree connection) status is tracked
> > > is confusing.  The same enum is used for structs cifs_tcon
> > > and cifs_ses and TCP_Server_info, but each of these three has
> > > different states that they transition among.  The current
> > > code also unnecessarily uses camelCase.
> > >
> > > Convert from use of statusEnum to a new tid_status_enum for
> > > tree connections.  The valid states for a tid are:
> > >
> > >             TID_NEW = 0,
> > >             TID_GOOD,
> > >             TID_EXITING,
> > >             TID_NEED_RECON,
> > >             TID_NEED_TCON,
> > >             TID_IN_TCON,
> > >             TID_NEED_FILES_INVALIDATE, /* unused, considering removing
> > > in future */
> > >             TID_IN_FILES_INVALIDATE
> > >
> > > It also removes CifsNeedTcon CifsInTcon CifsNeedFilesInvalidate and
> > > CifsInFilesInvalidate from the statusEnum used for session and
> > > TCP_Server_Info since they are not relevant for those.
> > >
> > > A follow on patch will fix the places where we use the
> > > tcon->need_reconnect flag to be more consistent with the tid->status
> > >
> > > See attached.
> > >
> > > Also FYI - Shyam had a work in progress patch to fix and clarify the
> > > ses->status enum
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
> Please try to maintain the old values in the new enum.
> Rest looks good.

They are mostly the same:
- for the statusEnum, only the ones at the end were removed, and those
don't apply to session or tcp_server (socket)
- for the tid->status the values for the first 4 didn't change

I did check carefully everwhere tidStatus was set/checked.

It shouldn't change any behavior.

The key question for next patch is whether we should check
tcon->need_reconnect which is what is used in all but one place or
check tid->status for need recon (which is done in only one place,
added by patch "cifs: check reconnects for channels of active tcons
too")

Thanks,

Steve