test run started with those 9 patches (8 multichannel, and Aurelien's small patch) http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/11/builds/68 On Sat, Jun 5, 2021 at 12:43 PM Steve French <smfrench@xxxxxxxxx> wrote: > > Had to fix two things in those recently updated patches to get them to > build. You may want to fold those in as well to make the patches > clearer to read and bisectable > > smfrench@smfrench-ThinkPad-P52:~/cifs-2.6/fs/cifs$ git diff -a > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 6e4934052159..f340b7d389ef 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -260,7 +260,7 @@ cifs_reconnect(struct TCP_Server_Info *server) > > /* If all channels need reconnect, then tcon needs reconnect */ > if (!CIFS_ALL_CHANS_NEED_RECONNECT(ses)) > - goto skip_tcon_reconnect; > + continue; > > list_for_each(tmp2, &ses->tcon_list) { > tcon = list_entry(tmp2, struct cifs_tcon, tcon_list); > @@ -268,8 +268,6 @@ cifs_reconnect(struct TCP_Server_Info *server) > } > if (ses->tcon_ipc) > ses->tcon_ipc->need_reconnect = true; > - > -skip_tcon_reconnect: > } > spin_unlock(&cifs_tcp_ses_lock); > mutex_unlock(&ses->session_mutex); > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > index cdbd256be4e4..202d98d06606 100644 > --- a/fs/cifs/sess.c > +++ b/fs/cifs/sess.c > @@ -92,10 +92,10 @@ unsigned long cifs_ses_get_chan_index(struct cifs_ses *ses, > void cifs_chan_set_need_reconnect(struct cifs_ses *ses, > struct TCP_Server_Info *server) > { > - size_t chan_index = cifs_ses_get_chan_index(ses, server); > + unsigned long chan_index = cifs_ses_get_chan_index(ses, server); > > set_bit(chan_index, &ses->chans_need_reconnect); > - cifs_dbg(FYI, "Set reconnect bitmask for chan %u; now 0x%lx\n", > + cifs_dbg(FYI, "Set reconnect bitmask for chan %lu; now 0x%lx\n", > chan_index, ses->chans_need_reconnect); > > On Sat, Jun 5, 2021 at 9:08 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: > > > > The buildbot testing once hit a deadlock when running with the above patches. > > I found one possibility during cifs_reconnect, where a deadlock can occur. > > > > I've fixed that and some warnings that kernel bots have identified in > > the below two patches: > > https://github.com/sprasad-microsoft/smb-kernel-client/pull/5/commits/f3e65f72b03b03bc4b301e8e04e9babb0e9582cf.patch > > https://github.com/sprasad-microsoft/smb-kernel-client/pull/5/commits/7b3e867e994a7cbc88efe85c95167ae49d4b7a9d.patch > > > > Regards, > > Shyam > > > > On Fri, Jun 4, 2021 at 8:55 PM Paulo Alcantara <pc@xxxxxx> wrote: > > > > > > Shyam Prasad N <nspmangalore@xxxxxxxxx> writes: > > > > > > > @Paulo Alcantara That would be great if you can help testing my > > > > changes. Please test with these new changes. > > > > > > OK. > > > > > > >> The super is only used for providing cifs_sb_info::origin_fullpath as key to find the corresponding failover targets in referral cache. > > > > I'm wondering what would happen if there are multiple tcons to the > > > > same origin_fullpath (possibly in different sessions)? > > > > > > That is certainly a problem, indeed. I'm waiting for the DFS tests to > > > finish and then send a series that contains a potential fix for that -- > > > e.g. not sharing TCP servers when mounting DFS shares. We used to not > > > share tcons with DFS mounts because they might contain different prefix > > > paths but connected to same share, however that wasn't enough because > > > multiple DFS mounts may connect to same target servers, although they > > > might failover to completely different servers. > > > > > > > Also, doesn't failover targets apply to each channel under a session? > > > > Shouldn't we switch targets on reconnect of secondary channels too? > > > > > > That's a interesting question. I recall discussing this with Aurelien > > > some time ago while running a few DFS + multichannel tests. > > > > > > So yes, I agree with you that when we successfully reconnect to failover > > > target (primary channel), then we should also update all secondary > > > channels with the new server's ip address and reconnect them. > > > > > > > > -- > > Regards, > > Shyam > > > > -- > Thanks, > > Steve -- Thanks, Steve