| Message ID | 20220331180151.5301-2-pc@cjr.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] cifs: fix potential race with cifsd thread | expand |
On 03/31, Paulo Alcantara wrote: >Do not reuse existing sessions and tcons in DFS failover as it might >connect to different servers and shares. > >Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> >--- > fs/cifs/connect.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > >diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >index 3ca06bd88b6e..3956672a11ae 100644 >--- a/fs/cifs/connect.c >+++ b/fs/cifs/connect.c >@@ -536,8 +536,11 @@ int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session) > return __cifs_reconnect(server, mark_smb_session); > } > spin_unlock(&cifs_tcp_ses_lock); >- >- return reconnect_dfs_server(server, mark_smb_session); >+ /* >+ * Ignore @mark_smb_session and invalidate all sessions & tcons as we might be connecting to >+ * a different server or share during failover. >+ */ >+ return reconnect_dfs_server(server, true); If you're ignoring @mark_smb_session, why not just leave @server for reconnect_dfs_server()? Cheers, Enzo
On Fri, Apr 1, 2022 at 12:42 AM Paulo Alcantara <pc@cjr.nz> wrote: > > Do not reuse existing sessions and tcons in DFS failover as it might > connect to different servers and shares. > > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > --- > fs/cifs/connect.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 3ca06bd88b6e..3956672a11ae 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -536,8 +536,11 @@ int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session) > return __cifs_reconnect(server, mark_smb_session); > } > spin_unlock(&cifs_tcp_ses_lock); > - > - return reconnect_dfs_server(server, mark_smb_session); > + /* > + * Ignore @mark_smb_session and invalidate all sessions & tcons as we might be connecting to > + * a different server or share during failover. > + */ > + return reconnect_dfs_server(server, true); > } > #else > int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session) > -- > 2.35.1 > This makes sense. Wondering if you could check if the reconnect happens to a new server/share and then change mark_smb_session? Maybe that complicates the logic. Looks good to me. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Enzo Matsumiya <ematsumiya@suse.de> writes: > If you're ignoring @mark_smb_session, why not just leave @server for > reconnect_dfs_server()? Good point, yes. I'll send v2 with it removed.
Shyam Prasad N <nspmangalore@gmail.com> writes: > This makes sense. > Wondering if you could check if the reconnect happens to a new > server/share and then change mark_smb_session? Maybe that complicates > the logic. Yes, it would complicate alot. We expect failover to not occur quite often, so leaving it without such logic would be OK for now, IMO. I'm trying to get all reconnect issues fixed and then we can talk about possible improvements, obviously.
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 3ca06bd88b6e..3956672a11ae 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -536,8 +536,11 @@ int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session) return __cifs_reconnect(server, mark_smb_session); } spin_unlock(&cifs_tcp_ses_lock); - - return reconnect_dfs_server(server, mark_smb_session); + /* + * Ignore @mark_smb_session and invalidate all sessions & tcons as we might be connecting to + * a different server or share during failover. + */ + return reconnect_dfs_server(server, true); } #else int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session)
Do not reuse existing sessions and tcons in DFS failover as it might connect to different servers and shares. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/connect.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)