Re: [PATCH] cifs: wait for tcon resource_id before getting fscache super

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



These are what I had downloaded and tentatively merged into
cifs-2.6.git for-next

On Fri, Dec 3, 2021 at 3:23 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
>
> The logic for initializing tcon->resource_id is done inside
> cifs_root_iget. fscache super cookie relies on this for aux
> data. So we need to push the fscache initialization to this
> later point during mount.
>
> Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
> ---
>  fs/cifs/connect.c | 6 ------
>  fs/cifs/fscache.c | 2 +-
>  fs/cifs/inode.c   | 7 +++++++
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 6b705026da1a3..eee994b0925ff 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3046,12 +3046,6 @@ static int mount_get_conns(struct mount_ctx *mnt_ctx)
>   cifs_dbg(VFS, "read only mount of RW share\n");
>   /* no need to log a RW mount of a typical RW share */
>   }
> - /*
> - * The cookie is initialized from volume info returned above.
> - * Inside cifs_fscache_get_super_cookie it checks
> - * that we do not get super cookie twice.
> - */
> - cifs_fscache_get_super_cookie(tcon);
>   }
>
>   /*
> diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
> index 7e409a38a2d7c..f4da693760c11 100644
> --- a/fs/cifs/fscache.c
> +++ b/fs/cifs/fscache.c
> @@ -92,7 +92,7 @@ void cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
>   * In the future, as we integrate with newer fscache features,
>   * we may want to instead add a check if cookie has changed
>   */
> - if (tcon->fscache == NULL)
> + if (tcon->fscache)
>   return;
>
>   sharename = extract_sharename(tcon->treeName);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 82848412ad852..96d083db17372 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1376,6 +1376,13 @@ struct inode *cifs_root_iget(struct super_block *sb)
>   inode = ERR_PTR(rc);
>   }
>
> + /*
> + * The cookie is initialized from volume info returned above.
> + * Inside cifs_fscache_get_super_cookie it checks
> + * that we do not get super cookie twice.
> + */
> + cifs_fscache_get_super_cookie(tcon);
> +
>  out:
>   kfree(path);
>   free_xid(xid);



-- 
Thanks,

Steve
From d70e50047c7daa3de17c7c41a0c4ef4f9e4443c9 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Date: Thu, 2 Dec 2021 07:14:42 +0000
Subject: [PATCH] cifs: wait for tcon resource_id before getting fscache super

The logic for initializing tcon->resource_id is done inside
cifs_root_iget. fscache super cookie relies on this for aux
data. So we need to push the fscache initialization to this
later point during mount.

Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
---
 fs/cifs/connect.c | 6 ------
 fs/cifs/fscache.c | 2 +-
 fs/cifs/inode.c   | 7 +++++++
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 6b705026da1a3..eee994b0925ff 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3046,12 +3046,6 @@ static int mount_get_conns(struct mount_ctx *mnt_ctx)
 				cifs_dbg(VFS, "read only mount of RW share\n");
 			/* no need to log a RW mount of a typical RW share */
 		}
-		/*
-		 * The cookie is initialized from volume info returned above.
-		 * Inside cifs_fscache_get_super_cookie it checks
-		 * that we do not get super cookie twice.
-		 */
-		cifs_fscache_get_super_cookie(tcon);
 	}
 
 	/*
diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index 7e409a38a2d7c..f4da693760c11 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -92,7 +92,7 @@ void cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
 	 * In the future, as we integrate with newer fscache features,
 	 * we may want to instead add a check if cookie has changed
 	 */
-	if (tcon->fscache == NULL)
+	if (tcon->fscache)
 		return;
 
 	sharename = extract_sharename(tcon->treeName);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 82848412ad852..96d083db17372 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1376,6 +1376,13 @@ struct inode *cifs_root_iget(struct super_block *sb)
 		inode = ERR_PTR(rc);
 	}
 
+	/*
+	 * The cookie is initialized from volume info returned above.
+	 * Inside cifs_fscache_get_super_cookie it checks
+	 * that we do not get super cookie twice.
+	 */
+	cifs_fscache_get_super_cookie(tcon);
+
 out:
 	kfree(path);
 	free_xid(xid);
From 089dd629653b857b6096966e9d2df301653a36ea Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Date: Thu, 2 Dec 2021 07:30:00 +0000
Subject: [PATCH] cifs: add server conn_id to fscache client cookie

The fscache client cookie uses the server address
(and port) as the cookie key. This is a problem when
nosharesock is used. Two different connections will
use duplicate cookies. Avoid this by adding
server->conn_id to the key, so that it's guaranteed
that cookie will not be duplicated.

Also, for secondary channels of a session, copy the
fscache pointer from the primary channel. The primary
channel is guaranteed not to go away as long as secondary
channels are in use.

Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
---
 fs/cifs/connect.c |  2 ++
 fs/cifs/fscache.c | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index eee994b0925f..d8822e835cc4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1562,6 +1562,8 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
 	/* fscache server cookies are based on primary channel only */
 	if (!CIFS_SERVER_IS_CHAN(tcp_ses))
 		cifs_fscache_get_client_cookie(tcp_ses);
+	else
+		tcp_ses->fscache = tcp_ses->primary_server->fscache;
 
 	/* queue echo request delayed work */
 	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index f4da693760c1..1db3437f3b7d 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -24,6 +24,7 @@ struct cifs_server_key {
 		struct in_addr	ipv4_addr;
 		struct in6_addr	ipv6_addr;
 	};
+	__u64 conn_id;
 } __packed;
 
 /*
@@ -37,6 +38,14 @@ void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
 	struct cifs_server_key key;
 	uint16_t key_len = sizeof(key.hdr);
 
+	/*
+	 * Check if cookie was already initialized so don't reinitialize it.
+	 * In the future, as we integrate with newer fscache features,
+	 * we may want to instead add a check if cookie has changed
+	 */
+	if (server->fscache)
+		return;
+
 	memset(&key, 0, sizeof(key));
 
 	/*
@@ -62,6 +71,7 @@ void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
 		server->fscache = NULL;
 		return;
 	}
+	key.conn_id = server->conn_id;
 
 	server->fscache =
 		fscache_acquire_cookie(cifs_fscache_netfs.primary_index,
From a9a62cdfe26c782dadd0b94ab529b883426d0acd Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Date: Thu, 2 Dec 2021 07:46:54 +0000
Subject: [PATCH] cifs: avoid use of dstaddr as key for fscache client cookie

server->dstaddr can change when the DNS mapping for the
server hostname changes. But conn_id is a u64 counter
that is incremented each time a new TCP connection
is setup. So use only that as a key.

Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
---
 fs/cifs/fscache.c | 38 +-------------------------------------
 1 file changed, 1 insertion(+), 37 deletions(-)

diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index 1db3437f3b7d..003c5f1f4dfb 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -16,14 +16,6 @@
  * Key layout of CIFS server cache index object
  */
 struct cifs_server_key {
-	struct {
-		uint16_t	family;		/* address family */
-		__be16		port;		/* IP port */
-	} hdr;
-	union {
-		struct in_addr	ipv4_addr;
-		struct in6_addr	ipv6_addr;
-	};
 	__u64 conn_id;
 } __packed;
 
@@ -32,11 +24,7 @@ struct cifs_server_key {
  */
 void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
 {
-	const struct sockaddr *sa = (struct sockaddr *) &server->dstaddr;
-	const struct sockaddr_in *addr = (struct sockaddr_in *) sa;
-	const struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) sa;
 	struct cifs_server_key key;
-	uint16_t key_len = sizeof(key.hdr);
 
 	/*
 	 * Check if cookie was already initialized so don't reinitialize it.
@@ -47,36 +35,12 @@ void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
 		return;
 
 	memset(&key, 0, sizeof(key));
-
-	/*
-	 * Should not be a problem as sin_family/sin6_family overlays
-	 * sa_family field
-	 */
-	key.hdr.family = sa->sa_family;
-	switch (sa->sa_family) {
-	case AF_INET:
-		key.hdr.port = addr->sin_port;
-		key.ipv4_addr = addr->sin_addr;
-		key_len += sizeof(key.ipv4_addr);
-		break;
-
-	case AF_INET6:
-		key.hdr.port = addr6->sin6_port;
-		key.ipv6_addr = addr6->sin6_addr;
-		key_len += sizeof(key.ipv6_addr);
-		break;
-
-	default:
-		cifs_dbg(VFS, "Unknown network family '%d'\n", sa->sa_family);
-		server->fscache = NULL;
-		return;
-	}
 	key.conn_id = server->conn_id;
 
 	server->fscache =
 		fscache_acquire_cookie(cifs_fscache_netfs.primary_index,
 				       &cifs_fscache_server_index_def,
-				       &key, key_len,
+				       &key, sizeof(key),
 				       NULL, 0,
 				       server, 0, true);
 	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/linux-cachefs

[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]
  Powered by Linux