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
