linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] cifs: simplify SWN code with dummy funcs instead of ifdefs
@ 2021-03-18 17:55 Aurélien Aptel
  2021-03-24  8:57 ` Samuel Cabrero
  0 siblings, 1 reply; 5+ messages in thread
From: Aurélien Aptel @ 2021-03-18 17:55 UTC (permalink / raw)
  To: linux-cifs; +Cc: smfrench, scabrero, Aurelien Aptel

From: Aurelien Aptel <aaptel@suse.com>

This commit doesn't change the logic of SWN.

Add dummy implementation of SWN functions when SWN is disabled instead
of using ifdef sections.

The dummy functions get optimized out, this leads to clearer code and
compile time type-checking regardless of config options with no
runtime penalty.

Leave the simple ifdefs section as-is.

A single bitfield (bool foo:1) on its own will use up one int. Move
tcon->use_witness out of ifdefs with the other tcon bitfields.

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/cifs_debug.c |  8 +-------
 fs/cifs/cifs_swn.h   | 27 +++++++++++++++++++++++++++
 fs/cifs/cifsfs.c     |  2 --
 fs/cifs/cifsglob.h   |  4 +---
 fs/cifs/connect.c    | 26 ++++----------------------
 5 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 0f3e64667a35..e19ddfb2b741 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -24,9 +24,7 @@
 #ifdef CONFIG_CIFS_SMB_DIRECT
 #include "smbdirect.h"
 #endif
-#ifdef CONFIG_CIFS_SWN_UPCALL
 #include "cifs_swn.h"
-#endif
 
 void
 cifs_dump_mem(char *label, void *data, int length)
@@ -119,10 +117,8 @@ static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon)
 		seq_printf(m, " POSIX Extensions");
 	if (tcon->ses->server->ops->dump_share_caps)
 		tcon->ses->server->ops->dump_share_caps(m, tcon);
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	if (tcon->use_witness)
 		seq_puts(m, " Witness");
-#endif
 
 	if (tcon->need_reconnect)
 		seq_puts(m, "\tDISCONNECTED ");
@@ -491,10 +487,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 
 	spin_unlock(&cifs_tcp_ses_lock);
 	seq_putc(m, '\n');
-
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	cifs_swn_dump(m);
-#endif
+
 	/* BB add code to dump additional info such as TCP session info now */
 	return 0;
 }
diff --git a/fs/cifs/cifs_swn.h b/fs/cifs/cifs_swn.h
index 236ecd4959d5..8a9d2a5c9077 100644
--- a/fs/cifs/cifs_swn.h
+++ b/fs/cifs/cifs_swn.h
@@ -7,11 +7,13 @@
 
 #ifndef _CIFS_SWN_H
 #define _CIFS_SWN_H
+#include "cifsglob.h"
 
 struct cifs_tcon;
 struct sk_buff;
 struct genl_info;
 
+#ifdef CONFIG_CIFS_SWN_UPCALL
 extern int cifs_swn_register(struct cifs_tcon *tcon);
 
 extern int cifs_swn_unregister(struct cifs_tcon *tcon);
@@ -22,4 +24,29 @@ extern void cifs_swn_dump(struct seq_file *m);
 
 extern void cifs_swn_check(void);
 
+static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server)
+{
+	if (server->use_swn_dstaddr) {
+		server->dstaddr = server->swn_dstaddr;
+		return true;
+	}
+	return false;
+}
+
+static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server)
+{
+	server->use_swn_dstaddr = false;
+}
+
+#else
+
+static inline int cifs_swn_register(struct cifs_tcon *tcon) { return 0; }
+static inline int cifs_swn_unregister(struct cifs_tcon *tcon) { return 0; }
+static inline int cifs_swn_notify(struct sk_buff *s, struct genl_info *i) { return 0; }
+static inline void cifs_swn_dump(struct seq_file *m) {}
+static inline void cifs_swn_check(void) {}
+static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server) { return false; }
+static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server) {}
+
+#endif /* CONFIG_CIFS_SWN_UPCALL */
 #endif /* _CIFS_SWN_H */
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 099ad9f3660b..58b9de3bd106 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -655,10 +655,8 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
 		seq_printf(s, ",multichannel,max_channels=%zu",
 			   tcon->ses->chan_max);
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	if (tcon->use_witness)
 		seq_puts(s, ",witness");
-#endif
 
 	return 0;
 }
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 31fc8695abd6..26df87a3bcbe 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1070,6 +1070,7 @@ struct cifs_tcon {
 	bool use_resilient:1; /* use resilient instead of durable handles */
 	bool use_persistent:1; /* use persistent instead of durable handles */
 	bool no_lease:1;    /* Do not request leases on files or directories */
+	bool use_witness:1; /* use witness protocol */
 	__le32 capabilities;
 	__u32 share_flags;
 	__u32 maximal_access;
@@ -1094,9 +1095,6 @@ struct cifs_tcon {
 	int remap:2;
 	struct list_head ulist; /* cache update list */
 #endif
-#ifdef CONFIG_CIFS_SWN_UPCALL
-	bool use_witness:1; /* use witness protocol */
-#endif
 };
 
 /*
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index eec8a2052da2..354b903b3a28 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -62,9 +62,7 @@
 #include "dfs_cache.h"
 #endif
 #include "fs_context.h"
-#ifdef CONFIG_CIFS_SWN_UPCALL
 #include "cifs_swn.h"
-#endif
 
 extern mempool_t *cifs_req_poolp;
 extern bool disable_legacy_dialects;
@@ -314,12 +312,8 @@ cifs_reconnect(struct TCP_Server_Info *server)
 
 		mutex_lock(&server->srv_mutex);
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
-		if (server->use_swn_dstaddr) {
-			server->dstaddr = server->swn_dstaddr;
-		} else {
-#endif
 
+		if (!cifs_swn_set_server_dstaddr(server)) {
 #ifdef CONFIG_CIFS_DFS_UPCALL
 			/*
 			 * Set up next DFS target server (if any) for reconnect. If DFS
@@ -328,10 +322,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
 			 */
 			reconn_set_next_dfs_target(server, cifs_sb, &tgt_list, &tgt_it);
 #endif
-
-#ifdef CONFIG_CIFS_SWN_UPCALL
 		}
-#endif
 
 		if (cifs_rdma_enabled(server))
 			rc = smbd_reconnect(server);
@@ -348,9 +339,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
 			if (server->tcpStatus != CifsExiting)
 				server->tcpStatus = CifsNeedNegotiate;
 			spin_unlock(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SWN_UPCALL
-			server->use_swn_dstaddr = false;
-#endif
+			cifs_swn_reset_server_dstaddr(server);
 			mutex_unlock(&server->srv_mutex);
 		}
 	} while (server->tcpStatus == CifsNeedReconnect);
@@ -415,10 +404,8 @@ cifs_echo_request(struct work_struct *work)
 		cifs_dbg(FYI, "Unable to send echo request to server: %s\n",
 			 server->hostname);
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	/* Check witness registrations */
 	cifs_swn_check();
-#endif
 
 requeue_echo:
 	queue_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
@@ -1994,7 +1981,6 @@ cifs_put_tcon(struct cifs_tcon *tcon)
 		return;
 	}
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	if (tcon->use_witness) {
 		int rc;
 
@@ -2004,7 +1990,6 @@ cifs_put_tcon(struct cifs_tcon *tcon)
 					__func__, rc);
 		}
 	}
-#endif
 
 	list_del_init(&tcon->tcon_list);
 	spin_unlock(&cifs_tcp_ses_lock);
@@ -2166,9 +2151,9 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 		}
 		tcon->use_resilient = true;
 	}
-#ifdef CONFIG_CIFS_SWN_UPCALL
+
 	tcon->use_witness = false;
-	if (ctx->witness) {
+	if (IS_ENABLED(CONFIG_CIFS_SWN_UPCALL) && ctx->witness) {
 		if (ses->server->vals->protocol_id >= SMB30_PROT_ID) {
 			if (tcon->capabilities & SMB2_SHARE_CAP_CLUSTER) {
 				/*
@@ -2194,7 +2179,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 			goto out_fail;
 		}
 	}
-#endif
 
 	/* If the user really knows what they are doing they can override */
 	if (tcon->share_flags & SMB2_SHAREFLAG_NO_CACHING) {
@@ -3862,9 +3846,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
 	ctx->sectype = master_tcon->ses->sectype;
 	ctx->sign = master_tcon->ses->sign;
 	ctx->seal = master_tcon->seal;
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	ctx->witness = master_tcon->use_witness;
-#endif
 
 	rc = cifs_set_vol_auth(ctx, master_tcon->ses);
 	if (rc) {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] cifs: simplify SWN code with dummy funcs instead of ifdefs
  2021-03-18 17:55 [PATCH v1] cifs: simplify SWN code with dummy funcs instead of ifdefs Aurélien Aptel
@ 2021-03-24  8:57 ` Samuel Cabrero
  2021-04-09  4:28   ` Steve French
  0 siblings, 1 reply; 5+ messages in thread
From: Samuel Cabrero @ 2021-03-24  8:57 UTC (permalink / raw)
  To: Aurélien Aptel, linux-cifs; +Cc: smfrench, scabrero

On Thu, 2021-03-18 at 18:55 +0100, Aurélien Aptel wrote:
> From: Aurelien Aptel <aaptel@suse.com>
> 
> This commit doesn't change the logic of SWN.
> 
> Add dummy implementation of SWN functions when SWN is disabled instead
> of using ifdef sections.
> 
> The dummy functions get optimized out, this leads to clearer code and
> compile time type-checking regardless of config options with no
> runtime penalty.
> 
> Leave the simple ifdefs section as-is.
> 
> A single bitfield (bool foo:1) on its own will use up one int. Move
> tcon->use_witness out of ifdefs with the other tcon bitfields.
> 
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>

Thanks Aurelien, it LGTM.

Reviewed-by: Samuel Cabrero <scabrero@suse.de>

-- 
Samuel Cabrero / SUSE Labs Samba Team
GPG: D7D6 E259 F91C F0B3 2E61 1239 3655 6EC9 7051 0856
scabrero@suse.com
scabrero@suse.de


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] cifs: simplify SWN code with dummy funcs instead of ifdefs
  2021-03-24  8:57 ` Samuel Cabrero
@ 2021-04-09  4:28   ` Steve French
  2021-04-09 14:31     ` [PATCH v2] " Aurélien Aptel
  0 siblings, 1 reply; 5+ messages in thread
From: Steve French @ 2021-04-09  4:28 UTC (permalink / raw)
  To: scabrero; +Cc: Aurélien Aptel, CIFS

Aurelien,
This didn't apply to current for-next when I tried applying it.  Can
you check and rebase the patch on current for-next?  Seems ok
otherwise.

On Wed, Mar 24, 2021 at 3:57 AM Samuel Cabrero <scabrero@suse.de> wrote:
>
> On Thu, 2021-03-18 at 18:55 +0100, Aurélien Aptel wrote:
> > From: Aurelien Aptel <aaptel@suse.com>
> >
> > This commit doesn't change the logic of SWN.
> >
> > Add dummy implementation of SWN functions when SWN is disabled instead
> > of using ifdef sections.
> >
> > The dummy functions get optimized out, this leads to clearer code and
> > compile time type-checking regardless of config options with no
> > runtime penalty.
> >
> > Leave the simple ifdefs section as-is.
> >
> > A single bitfield (bool foo:1) on its own will use up one int. Move
> > tcon->use_witness out of ifdefs with the other tcon bitfields.
> >
> > Signed-off-by: Aurelien Aptel <aaptel@suse.com>
>
> Thanks Aurelien, it LGTM.
>
> Reviewed-by: Samuel Cabrero <scabrero@suse.de>
>
> --
> Samuel Cabrero / SUSE Labs Samba Team
> GPG: D7D6 E259 F91C F0B3 2E61 1239 3655 6EC9 7051 0856
> scabrero@suse.com
> scabrero@suse.de
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] cifs: simplify SWN code with dummy funcs instead of ifdefs
  2021-04-09  4:28   ` Steve French
@ 2021-04-09 14:31     ` Aurélien Aptel
  2021-04-09 18:49       ` Steve French
  0 siblings, 1 reply; 5+ messages in thread
From: Aurélien Aptel @ 2021-04-09 14:31 UTC (permalink / raw)
  To: Steve French, scabrero; +Cc: Aurélien Aptel, CIFS

From: Aurelien Aptel <aaptel@suse.com>

This commit doesn't change the logic of SWN.

Add dummy implementation of SWN functions when SWN is disabled instead
of using ifdef sections.

The dummy functions get optimized out, this leads to clearer code and
compile time type-checking regardless of config options with no
runtime penalty.

Leave the simple ifdefs section as-is.

A single bitfield (bool foo:1) on its own will use up one int. Move
tcon->use_witness out of ifdefs with the other tcon bitfields.

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
changes since v1:
* updated to apply on current for-next


 fs/cifs/cifs_debug.c |  8 +-------
 fs/cifs/cifs_swn.h   | 27 +++++++++++++++++++++++++++
 fs/cifs/cifsfs.c     |  2 --
 fs/cifs/cifsglob.h   |  4 +---
 fs/cifs/connect.c    | 25 ++++---------------------
 5 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 88a7958170ee..d8ae961a510f 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -23,9 +23,7 @@
 #ifdef CONFIG_CIFS_SMB_DIRECT
 #include "smbdirect.h"
 #endif
-#ifdef CONFIG_CIFS_SWN_UPCALL
 #include "cifs_swn.h"
-#endif
 
 void
 cifs_dump_mem(char *label, void *data, int length)
@@ -118,10 +116,8 @@ static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon)
 		seq_printf(m, " POSIX Extensions");
 	if (tcon->ses->server->ops->dump_share_caps)
 		tcon->ses->server->ops->dump_share_caps(m, tcon);
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	if (tcon->use_witness)
 		seq_puts(m, " Witness");
-#endif
 
 	if (tcon->need_reconnect)
 		seq_puts(m, "\tDISCONNECTED ");
@@ -490,10 +486,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 
 	spin_unlock(&cifs_tcp_ses_lock);
 	seq_putc(m, '\n');
-
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	cifs_swn_dump(m);
-#endif
+
 	/* BB add code to dump additional info such as TCP session info now */
 	return 0;
 }
diff --git a/fs/cifs/cifs_swn.h b/fs/cifs/cifs_swn.h
index 236ecd4959d5..8a9d2a5c9077 100644
--- a/fs/cifs/cifs_swn.h
+++ b/fs/cifs/cifs_swn.h
@@ -7,11 +7,13 @@
 
 #ifndef _CIFS_SWN_H
 #define _CIFS_SWN_H
+#include "cifsglob.h"
 
 struct cifs_tcon;
 struct sk_buff;
 struct genl_info;
 
+#ifdef CONFIG_CIFS_SWN_UPCALL
 extern int cifs_swn_register(struct cifs_tcon *tcon);
 
 extern int cifs_swn_unregister(struct cifs_tcon *tcon);
@@ -22,4 +24,29 @@ extern void cifs_swn_dump(struct seq_file *m);
 
 extern void cifs_swn_check(void);
 
+static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server)
+{
+	if (server->use_swn_dstaddr) {
+		server->dstaddr = server->swn_dstaddr;
+		return true;
+	}
+	return false;
+}
+
+static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server)
+{
+	server->use_swn_dstaddr = false;
+}
+
+#else
+
+static inline int cifs_swn_register(struct cifs_tcon *tcon) { return 0; }
+static inline int cifs_swn_unregister(struct cifs_tcon *tcon) { return 0; }
+static inline int cifs_swn_notify(struct sk_buff *s, struct genl_info *i) { return 0; }
+static inline void cifs_swn_dump(struct seq_file *m) {}
+static inline void cifs_swn_check(void) {}
+static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server) { return false; }
+static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server) {}
+
+#endif /* CONFIG_CIFS_SWN_UPCALL */
 #endif /* _CIFS_SWN_H */
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5ddd20b62484..1b65ff9e9189 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -656,10 +656,8 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
 		seq_printf(s, ",multichannel,max_channels=%zu",
 			   tcon->ses->chan_max);
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	if (tcon->use_witness)
 		seq_puts(s, ",witness");
-#endif
 
 	return 0;
 }
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ec824ab8c5ca..dec0620ccca4 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1070,6 +1070,7 @@ struct cifs_tcon {
 	bool use_resilient:1; /* use resilient instead of durable handles */
 	bool use_persistent:1; /* use persistent instead of durable handles */
 	bool no_lease:1;    /* Do not request leases on files or directories */
+	bool use_witness:1; /* use witness protocol */
 	__le32 capabilities;
 	__u32 share_flags;
 	__u32 maximal_access;
@@ -1094,9 +1095,6 @@ struct cifs_tcon {
 	int remap:2;
 	struct list_head ulist; /* cache update list */
 #endif
-#ifdef CONFIG_CIFS_SWN_UPCALL
-	bool use_witness:1; /* use witness protocol */
-#endif
 };
 
 /*
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 24668eb006c6..35dbb9c836ea 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -62,9 +62,7 @@
 #include "dfs_cache.h"
 #endif
 #include "fs_context.h"
-#ifdef CONFIG_CIFS_SWN_UPCALL
 #include "cifs_swn.h"
-#endif
 
 extern mempool_t *cifs_req_poolp;
 extern bool disable_legacy_dialects;
@@ -314,12 +312,8 @@ cifs_reconnect(struct TCP_Server_Info *server)
 
 		mutex_lock(&server->srv_mutex);
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
-		if (server->use_swn_dstaddr) {
-			server->dstaddr = server->swn_dstaddr;
-		} else {
-#endif
 
+		if (!cifs_swn_set_server_dstaddr(server)) {
 #ifdef CONFIG_CIFS_DFS_UPCALL
 		if (cifs_sb && cifs_sb->origin_fullpath)
 			/*
@@ -344,9 +338,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
 #endif
 
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 		}
-#endif
 
 		if (cifs_rdma_enabled(server))
 			rc = smbd_reconnect(server);
@@ -363,9 +355,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
 			if (server->tcpStatus != CifsExiting)
 				server->tcpStatus = CifsNeedNegotiate;
 			spin_unlock(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SWN_UPCALL
-			server->use_swn_dstaddr = false;
-#endif
+			cifs_swn_reset_server_dstaddr(server);
 			mutex_unlock(&server->srv_mutex);
 		}
 	} while (server->tcpStatus == CifsNeedReconnect);
@@ -430,10 +420,8 @@ cifs_echo_request(struct work_struct *work)
 		cifs_dbg(FYI, "Unable to send echo request to server: %s\n",
 			 server->hostname);
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	/* Check witness registrations */
 	cifs_swn_check();
-#endif
 
 requeue_echo:
 	queue_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
@@ -2009,7 +1997,6 @@ cifs_put_tcon(struct cifs_tcon *tcon)
 		return;
 	}
 
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	if (tcon->use_witness) {
 		int rc;
 
@@ -2019,7 +2006,6 @@ cifs_put_tcon(struct cifs_tcon *tcon)
 					__func__, rc);
 		}
 	}
-#endif
 
 	list_del_init(&tcon->tcon_list);
 	spin_unlock(&cifs_tcp_ses_lock);
@@ -2181,9 +2167,9 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 		}
 		tcon->use_resilient = true;
 	}
-#ifdef CONFIG_CIFS_SWN_UPCALL
+
 	tcon->use_witness = false;
-	if (ctx->witness) {
+	if (IS_ENABLED(CONFIG_CIFS_SWN_UPCALL) && ctx->witness) {
 		if (ses->server->vals->protocol_id >= SMB30_PROT_ID) {
 			if (tcon->capabilities & SMB2_SHARE_CAP_CLUSTER) {
 				/*
@@ -2209,7 +2195,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 			goto out_fail;
 		}
 	}
-#endif
 
 	/* If the user really knows what they are doing they can override */
 	if (tcon->share_flags & SMB2_SHAREFLAG_NO_CACHING) {
@@ -3877,9 +3862,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
 	ctx->sectype = master_tcon->ses->sectype;
 	ctx->sign = master_tcon->ses->sign;
 	ctx->seal = master_tcon->seal;
-#ifdef CONFIG_CIFS_SWN_UPCALL
 	ctx->witness = master_tcon->use_witness;
-#endif
 
 	rc = cifs_set_vol_auth(ctx, master_tcon->ses);
 	if (rc) {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] cifs: simplify SWN code with dummy funcs instead of ifdefs
  2021-04-09 14:31     ` [PATCH v2] " Aurélien Aptel
@ 2021-04-09 18:49       ` Steve French
  0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2021-04-09 18:49 UTC (permalink / raw)
  To: Aurélien Aptel; +Cc: scabrero, CIFS

tentatively merged into cifs-2.6.git for-next pending more testing

On Fri, Apr 9, 2021 at 9:32 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> From: Aurelien Aptel <aaptel@suse.com>
>
> This commit doesn't change the logic of SWN.
>
> Add dummy implementation of SWN functions when SWN is disabled instead
> of using ifdef sections.
>
> The dummy functions get optimized out, this leads to clearer code and
> compile time type-checking regardless of config options with no
> runtime penalty.
>
> Leave the simple ifdefs section as-is.
>
> A single bitfield (bool foo:1) on its own will use up one int. Move
> tcon->use_witness out of ifdefs with the other tcon bitfields.
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
> changes since v1:
> * updated to apply on current for-next
>
>
>  fs/cifs/cifs_debug.c |  8 +-------
>  fs/cifs/cifs_swn.h   | 27 +++++++++++++++++++++++++++
>  fs/cifs/cifsfs.c     |  2 --
>  fs/cifs/cifsglob.h   |  4 +---
>  fs/cifs/connect.c    | 25 ++++---------------------
>  5 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 88a7958170ee..d8ae961a510f 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -23,9 +23,7 @@
>  #ifdef CONFIG_CIFS_SMB_DIRECT
>  #include "smbdirect.h"
>  #endif
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>  #include "cifs_swn.h"
> -#endif
>
>  void
>  cifs_dump_mem(char *label, void *data, int length)
> @@ -118,10 +116,8 @@ static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon)
>                 seq_printf(m, " POSIX Extensions");
>         if (tcon->ses->server->ops->dump_share_caps)
>                 tcon->ses->server->ops->dump_share_caps(m, tcon);
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>         if (tcon->use_witness)
>                 seq_puts(m, " Witness");
> -#endif
>
>         if (tcon->need_reconnect)
>                 seq_puts(m, "\tDISCONNECTED ");
> @@ -490,10 +486,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
>
>         spin_unlock(&cifs_tcp_ses_lock);
>         seq_putc(m, '\n');
> -
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>         cifs_swn_dump(m);
> -#endif
> +
>         /* BB add code to dump additional info such as TCP session info now */
>         return 0;
>  }
> diff --git a/fs/cifs/cifs_swn.h b/fs/cifs/cifs_swn.h
> index 236ecd4959d5..8a9d2a5c9077 100644
> --- a/fs/cifs/cifs_swn.h
> +++ b/fs/cifs/cifs_swn.h
> @@ -7,11 +7,13 @@
>
>  #ifndef _CIFS_SWN_H
>  #define _CIFS_SWN_H
> +#include "cifsglob.h"
>
>  struct cifs_tcon;
>  struct sk_buff;
>  struct genl_info;
>
> +#ifdef CONFIG_CIFS_SWN_UPCALL
>  extern int cifs_swn_register(struct cifs_tcon *tcon);
>
>  extern int cifs_swn_unregister(struct cifs_tcon *tcon);
> @@ -22,4 +24,29 @@ extern void cifs_swn_dump(struct seq_file *m);
>
>  extern void cifs_swn_check(void);
>
> +static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server)
> +{
> +       if (server->use_swn_dstaddr) {
> +               server->dstaddr = server->swn_dstaddr;
> +               return true;
> +       }
> +       return false;
> +}
> +
> +static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server)
> +{
> +       server->use_swn_dstaddr = false;
> +}
> +
> +#else
> +
> +static inline int cifs_swn_register(struct cifs_tcon *tcon) { return 0; }
> +static inline int cifs_swn_unregister(struct cifs_tcon *tcon) { return 0; }
> +static inline int cifs_swn_notify(struct sk_buff *s, struct genl_info *i) { return 0; }
> +static inline void cifs_swn_dump(struct seq_file *m) {}
> +static inline void cifs_swn_check(void) {}
> +static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server) { return false; }
> +static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server) {}
> +
> +#endif /* CONFIG_CIFS_SWN_UPCALL */
>  #endif /* _CIFS_SWN_H */
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 5ddd20b62484..1b65ff9e9189 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -656,10 +656,8 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>                 seq_printf(s, ",multichannel,max_channels=%zu",
>                            tcon->ses->chan_max);
>
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>         if (tcon->use_witness)
>                 seq_puts(s, ",witness");
> -#endif
>
>         return 0;
>  }
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index ec824ab8c5ca..dec0620ccca4 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1070,6 +1070,7 @@ struct cifs_tcon {
>         bool use_resilient:1; /* use resilient instead of durable handles */
>         bool use_persistent:1; /* use persistent instead of durable handles */
>         bool no_lease:1;    /* Do not request leases on files or directories */
> +       bool use_witness:1; /* use witness protocol */
>         __le32 capabilities;
>         __u32 share_flags;
>         __u32 maximal_access;
> @@ -1094,9 +1095,6 @@ struct cifs_tcon {
>         int remap:2;
>         struct list_head ulist; /* cache update list */
>  #endif
> -#ifdef CONFIG_CIFS_SWN_UPCALL
> -       bool use_witness:1; /* use witness protocol */
> -#endif
>  };
>
>  /*
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 24668eb006c6..35dbb9c836ea 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -62,9 +62,7 @@
>  #include "dfs_cache.h"
>  #endif
>  #include "fs_context.h"
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>  #include "cifs_swn.h"
> -#endif
>
>  extern mempool_t *cifs_req_poolp;
>  extern bool disable_legacy_dialects;
> @@ -314,12 +312,8 @@ cifs_reconnect(struct TCP_Server_Info *server)
>
>                 mutex_lock(&server->srv_mutex);
>
> -#ifdef CONFIG_CIFS_SWN_UPCALL
> -               if (server->use_swn_dstaddr) {
> -                       server->dstaddr = server->swn_dstaddr;
> -               } else {
> -#endif
>
> +               if (!cifs_swn_set_server_dstaddr(server)) {
>  #ifdef CONFIG_CIFS_DFS_UPCALL
>                 if (cifs_sb && cifs_sb->origin_fullpath)
>                         /*
> @@ -344,9 +338,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
>  #endif
>
>
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>                 }
> -#endif
>
>                 if (cifs_rdma_enabled(server))
>                         rc = smbd_reconnect(server);
> @@ -363,9 +355,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
>                         if (server->tcpStatus != CifsExiting)
>                                 server->tcpStatus = CifsNeedNegotiate;
>                         spin_unlock(&GlobalMid_Lock);
> -#ifdef CONFIG_CIFS_SWN_UPCALL
> -                       server->use_swn_dstaddr = false;
> -#endif
> +                       cifs_swn_reset_server_dstaddr(server);
>                         mutex_unlock(&server->srv_mutex);
>                 }
>         } while (server->tcpStatus == CifsNeedReconnect);
> @@ -430,10 +420,8 @@ cifs_echo_request(struct work_struct *work)
>                 cifs_dbg(FYI, "Unable to send echo request to server: %s\n",
>                          server->hostname);
>
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>         /* Check witness registrations */
>         cifs_swn_check();
> -#endif
>
>  requeue_echo:
>         queue_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
> @@ -2009,7 +1997,6 @@ cifs_put_tcon(struct cifs_tcon *tcon)
>                 return;
>         }
>
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>         if (tcon->use_witness) {
>                 int rc;
>
> @@ -2019,7 +2006,6 @@ cifs_put_tcon(struct cifs_tcon *tcon)
>                                         __func__, rc);
>                 }
>         }
> -#endif
>
>         list_del_init(&tcon->tcon_list);
>         spin_unlock(&cifs_tcp_ses_lock);
> @@ -2181,9 +2167,9 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>                 }
>                 tcon->use_resilient = true;
>         }
> -#ifdef CONFIG_CIFS_SWN_UPCALL
> +
>         tcon->use_witness = false;
> -       if (ctx->witness) {
> +       if (IS_ENABLED(CONFIG_CIFS_SWN_UPCALL) && ctx->witness) {
>                 if (ses->server->vals->protocol_id >= SMB30_PROT_ID) {
>                         if (tcon->capabilities & SMB2_SHARE_CAP_CLUSTER) {
>                                 /*
> @@ -2209,7 +2195,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>                         goto out_fail;
>                 }
>         }
> -#endif
>
>         /* If the user really knows what they are doing they can override */
>         if (tcon->share_flags & SMB2_SHAREFLAG_NO_CACHING) {
> @@ -3877,9 +3862,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
>         ctx->sectype = master_tcon->ses->sectype;
>         ctx->sign = master_tcon->ses->sign;
>         ctx->seal = master_tcon->seal;
> -#ifdef CONFIG_CIFS_SWN_UPCALL
>         ctx->witness = master_tcon->use_witness;
> -#endif
>
>         rc = cifs_set_vol_auth(ctx, master_tcon->ses);
>         if (rc) {
> --
> 2.30.0
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-09 18:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 17:55 [PATCH v1] cifs: simplify SWN code with dummy funcs instead of ifdefs Aurélien Aptel
2021-03-24  8:57 ` Samuel Cabrero
2021-04-09  4:28   ` Steve French
2021-04-09 14:31     ` [PATCH v2] " Aurélien Aptel
2021-04-09 18:49       ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).