* [PATCH] cifs: serialize all mount attempts
@ 2022-01-18 2:16 Ronnie Sahlberg
2022-01-18 2:41 ` Steve French
2022-01-18 3:42 ` Steve French
0 siblings, 2 replies; 6+ messages in thread
From: Ronnie Sahlberg @ 2022-01-18 2:16 UTC (permalink / raw)
To: linux-cifs; +Cc: Steve French
RHBZ: 2008434
Some servers, such as Windows2016 have a very low number of concurrent mounts that
they allow from each client.
This can be a problem if you have a more than a handful (==3 in this case)
of cifs entries in your fstab and cause a number of the mounts there to randomly fail.
Add a global mutex and use it to serialize all mount attempts.
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
fs/cifs/fs_context.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index e3ed25dc6f3f..7ec35f3f0a5f 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -37,6 +37,8 @@
#include "rfc1002pdu.h"
#include "fs_context.h"
+static DEFINE_MUTEX(cifs_mount_mutex);
+
static const match_table_t cifs_smb_version_tokens = {
{ Smb_1, SMB1_VERSION_STRING },
{ Smb_20, SMB20_VERSION_STRING},
@@ -707,10 +709,14 @@ static int smb3_get_tree_common(struct fs_context *fc)
static int smb3_get_tree(struct fs_context *fc)
{
int err = smb3_fs_context_validate(fc);
+ int ret;
if (err)
return err;
- return smb3_get_tree_common(fc);
+ mutex_lock(&cifs_mount_mutex);
+ ret = smb3_get_tree_common(fc);
+ mutex_unlock(&cifs_mount_mutex);
+ return ret;
}
static void smb3_fs_context_free(struct fs_context *fc)
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: serialize all mount attempts
2022-01-18 2:16 [PATCH] cifs: serialize all mount attempts Ronnie Sahlberg
@ 2022-01-18 2:41 ` Steve French
2022-01-18 3:42 ` Steve French
1 sibling, 0 replies; 6+ messages in thread
From: Steve French @ 2022-01-18 2:41 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs
Would this slow things down much at boot time when you do have e.g. 3
mounts ... would end up serializing a bit more?
On Mon, Jan 17, 2022 at 8:17 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 2008434
>
> Some servers, such as Windows2016 have a very low number of concurrent mounts that
> they allow from each client.
> This can be a problem if you have a more than a handful (==3 in this case)
> of cifs entries in your fstab and cause a number of the mounts there to randomly fail.
>
> Add a global mutex and use it to serialize all mount attempts.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
> fs/cifs/fs_context.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index e3ed25dc6f3f..7ec35f3f0a5f 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -37,6 +37,8 @@
> #include "rfc1002pdu.h"
> #include "fs_context.h"
>
> +static DEFINE_MUTEX(cifs_mount_mutex);
> +
> static const match_table_t cifs_smb_version_tokens = {
> { Smb_1, SMB1_VERSION_STRING },
> { Smb_20, SMB20_VERSION_STRING},
> @@ -707,10 +709,14 @@ static int smb3_get_tree_common(struct fs_context *fc)
> static int smb3_get_tree(struct fs_context *fc)
> {
> int err = smb3_fs_context_validate(fc);
> + int ret;
>
> if (err)
> return err;
> - return smb3_get_tree_common(fc);
> + mutex_lock(&cifs_mount_mutex);
> + ret = smb3_get_tree_common(fc);
> + mutex_unlock(&cifs_mount_mutex);
> + return ret;
> }
>
> static void smb3_fs_context_free(struct fs_context *fc)
> --
> 2.30.2
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: serialize all mount attempts
2022-01-18 2:16 [PATCH] cifs: serialize all mount attempts Ronnie Sahlberg
2022-01-18 2:41 ` Steve French
@ 2022-01-18 3:42 ` Steve French
1 sibling, 0 replies; 6+ messages in thread
From: Steve French @ 2022-01-18 3:42 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs
What would happen if on boot you had 3 servers in fstab, the first two
of which time out due to bad address (or in my case port 445 blocked
for the internet but not for local servers in the house), does it slow
down bootup? Does it slow down the mount to the 3rd server (which is
reachable)?
Would it slow things down too much in some cases?
On Mon, Jan 17, 2022 at 8:17 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 2008434
>
> Some servers, such as Windows2016 have a very low number of concurrent mounts that
> they allow from each client.
> This can be a problem if you have a more than a handful (==3 in this case)
> of cifs entries in your fstab and cause a number of the mounts there to randomly fail.
>
> Add a global mutex and use it to serialize all mount attempts.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
> fs/cifs/fs_context.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index e3ed25dc6f3f..7ec35f3f0a5f 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -37,6 +37,8 @@
> #include "rfc1002pdu.h"
> #include "fs_context.h"
>
> +static DEFINE_MUTEX(cifs_mount_mutex);
> +
> static const match_table_t cifs_smb_version_tokens = {
> { Smb_1, SMB1_VERSION_STRING },
> { Smb_20, SMB20_VERSION_STRING},
> @@ -707,10 +709,14 @@ static int smb3_get_tree_common(struct fs_context *fc)
> static int smb3_get_tree(struct fs_context *fc)
> {
> int err = smb3_fs_context_validate(fc);
> + int ret;
>
> if (err)
> return err;
> - return smb3_get_tree_common(fc);
> + mutex_lock(&cifs_mount_mutex);
> + ret = smb3_get_tree_common(fc);
> + mutex_unlock(&cifs_mount_mutex);
> + return ret;
> }
>
> static void smb3_fs_context_free(struct fs_context *fc)
> --
> 2.30.2
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cifs: serialize all mount attempts
@ 2022-01-18 7:45 Ronnie Sahlberg
2022-01-18 16:23 ` Steve French
2022-01-20 9:38 ` Aurélien Aptel
0 siblings, 2 replies; 6+ messages in thread
From: Ronnie Sahlberg @ 2022-01-18 7:45 UTC (permalink / raw)
To: linux-cifs; +Cc: Steve French
RHBZ: 2008434
If we try to perform multiple concurrent mounts ot the same server we might
end up in a situation where:
Thread #1 Thread #2
creates TCP connection
Issues NegotiateProtocol
... Pick the TCP connection for Thread #1
Issue a new NegotiateProtocol
which then leads to the the server kills off the session.
There are also other a similar race where several threads ending up
withe their own unique tcp connection that all go to the same server structure ....
The most straightforward way to fix these races with concurrent mounts are to serialize
them. I.e. only allow one mount to be in progress at a time.
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
fs/cifs/fs_context.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index e3ed25dc6f3f..7ec35f3f0a5f 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -37,6 +37,8 @@
#include "rfc1002pdu.h"
#include "fs_context.h"
+static DEFINE_MUTEX(cifs_mount_mutex);
+
static const match_table_t cifs_smb_version_tokens = {
{ Smb_1, SMB1_VERSION_STRING },
{ Smb_20, SMB20_VERSION_STRING},
@@ -707,10 +709,14 @@ static int smb3_get_tree_common(struct fs_context *fc)
static int smb3_get_tree(struct fs_context *fc)
{
int err = smb3_fs_context_validate(fc);
+ int ret;
if (err)
return err;
- return smb3_get_tree_common(fc);
+ mutex_lock(&cifs_mount_mutex);
+ ret = smb3_get_tree_common(fc);
+ mutex_unlock(&cifs_mount_mutex);
+ return ret;
}
static void smb3_fs_context_free(struct fs_context *fc)
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: serialize all mount attempts
2022-01-18 7:45 Ronnie Sahlberg
@ 2022-01-18 16:23 ` Steve French
2022-01-20 9:38 ` Aurélien Aptel
1 sibling, 0 replies; 6+ messages in thread
From: Steve French @ 2022-01-18 16:23 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs
Am curious why serializing on a per-socket mutex
(TCP_Server_info)->srv_mutex doesn't work?
On Tue, Jan 18, 2022 at 1:45 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 2008434
>
> If we try to perform multiple concurrent mounts ot the same server we might
> end up in a situation where:
> Thread #1 Thread #2
> creates TCP connection
> Issues NegotiateProtocol
> ... Pick the TCP connection for Thread #1
> Issue a new NegotiateProtocol
>
> which then leads to the the server kills off the session.
> There are also other a similar race where several threads ending up
> withe their own unique tcp connection that all go to the same server structure ....
>
> The most straightforward way to fix these races with concurrent mounts are to serialize
> them. I.e. only allow one mount to be in progress at a time.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
> fs/cifs/fs_context.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index e3ed25dc6f3f..7ec35f3f0a5f 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -37,6 +37,8 @@
> #include "rfc1002pdu.h"
> #include "fs_context.h"
>
> +static DEFINE_MUTEX(cifs_mount_mutex);
> +
> static const match_table_t cifs_smb_version_tokens = {
> { Smb_1, SMB1_VERSION_STRING },
> { Smb_20, SMB20_VERSION_STRING},
> @@ -707,10 +709,14 @@ static int smb3_get_tree_common(struct fs_context *fc)
> static int smb3_get_tree(struct fs_context *fc)
> {
> int err = smb3_fs_context_validate(fc);
> + int ret;
>
> if (err)
> return err;
> - return smb3_get_tree_common(fc);
> + mutex_lock(&cifs_mount_mutex);
> + ret = smb3_get_tree_common(fc);
> + mutex_unlock(&cifs_mount_mutex);
> + return ret;
> }
>
> static void smb3_fs_context_free(struct fs_context *fc)
> --
> 2.30.2
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: serialize all mount attempts
2022-01-18 7:45 Ronnie Sahlberg
2022-01-18 16:23 ` Steve French
@ 2022-01-20 9:38 ` Aurélien Aptel
1 sibling, 0 replies; 6+ messages in thread
From: Aurélien Aptel @ 2022-01-20 9:38 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs, Steve French
On Wed, Jan 19, 2022 at 9:49 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 2008434
>
> If we try to perform multiple concurrent mounts ot the same server we might
> end up in a situation where:
> Thread #1 Thread #2
> creates TCP connection
> Issues NegotiateProtocol
> ... Pick the TCP connection for Thread #1
> Issue a new NegotiateProtocol
checking server->tcpStatus state should prevent this situation no?
> which then leads to the the server kills off the session.
> There are also other a similar race where several threads ending up
> withe their own unique tcp connection that all go to the same server structure ....
>
> The most straightforward way to fix these races with concurrent mounts are to serialize
> them. I.e. only allow one mount to be in progress at a time.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-20 9:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-18 2:16 [PATCH] cifs: serialize all mount attempts Ronnie Sahlberg
2022-01-18 2:41 ` Steve French
2022-01-18 3:42 ` Steve French
-- strict thread matches above, loose matches on Subject: below --
2022-01-18 7:45 Ronnie Sahlberg
2022-01-18 16:23 ` Steve French
2022-01-20 9:38 ` Aurélien Aptel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox