[PATCH v2 0/4] net/9p: optimize transport module loading

[PATCH v2 0/4] net/9p: optimize transport module loading

[Thomas Weißschuh]

This is a continuation of the single patch
"net/9p: autoload transport modules".

Patch 1 is a cleaned up version of the original patch.

Patch 2 splits the filedescriptor-based transports into their own module.

Patch 3 adds autoloading for the xen transport. Please note that this is
completely untested, but other xenbus drivers do the same.

Patch 4 adds some fallback transport loading from modules if none is usable at
the moment.

Changes since v1:
( https://lore.kernel.org/netdev/20211017134611.4330-1-linux@weissschuh.net/ )

* Fix warnings
* Split FD transport into its own module
* Autoload xen transport when xenbus device is present
* Load transports from modules when none is specified and loaded

Thomas Weißschuh (4):
  net/9p: autoload transport modules
  9p/trans_fd: split into dedicated module
  9p/xen: autoload when xenbus service is available
  net/p9: load default transports

 include/net/9p/9p.h        |  2 --
 include/net/9p/transport.h |  8 +++++++-
 net/9p/Kconfig             |  7 +++++++
 net/9p/Makefile            |  5 ++++-
 net/9p/mod.c               | 41 ++++++++++++++++++++++++++++++--------
 net/9p/trans_fd.c          | 14 +++++++++++--
 net/9p/trans_rdma.c        |  1 +
 net/9p/trans_virtio.c      |  1 +
 net/9p/trans_xen.c         |  2 ++
 9 files changed, 67 insertions(+), 14 deletions(-)


base-commit: cc0356d6a02e064387c16a83cb96fe43ef33181e
-- 
2.33.1

[PATCH v2 1/4] net/9p: autoload transport modules

[Thomas Weißschuh]

Automatically load transport modules based on the trans= parameter
passed to mount.
This removes the requirement for the user to know which module to use.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/net/9p/transport.h |  8 +++++++-
 net/9p/mod.c               | 30 ++++++++++++++++++++++++------
 net/9p/trans_rdma.c        |  1 +
 net/9p/trans_virtio.c      |  1 +
 net/9p/trans_xen.c         |  1 +
 5 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/net/9p/transport.h b/include/net/9p/transport.h
index 3eb4261b2958..b9a009534f99 100644
--- a/include/net/9p/transport.h
+++ b/include/net/9p/transport.h
@@ -11,6 +11,8 @@
 #ifndef NET_9P_TRANSPORT_H
 #define NET_9P_TRANSPORT_H
 
+#include <linux/module.h>
+
 #define P9_DEF_MIN_RESVPORT	(665U)
 #define P9_DEF_MAX_RESVPORT	(1023U)
 
@@ -52,7 +54,11 @@ struct p9_trans_module {
 
 void v9fs_register_trans(struct p9_trans_module *m);
 void v9fs_unregister_trans(struct p9_trans_module *m);
-struct p9_trans_module *v9fs_get_trans_by_name(char *s);
+struct p9_trans_module *v9fs_get_trans_by_name(const char *s);
 struct p9_trans_module *v9fs_get_default_trans(void);
 void v9fs_put_trans(struct p9_trans_module *m);
+
+#define MODULE_ALIAS_9P(transport) \
+	MODULE_ALIAS("9p-" transport)
+
 #endif /* NET_9P_TRANSPORT_H */
diff --git a/net/9p/mod.c b/net/9p/mod.c
index 5126566850bd..c95416c1d1a2 100644
--- a/net/9p/mod.c
+++ b/net/9p/mod.c
@@ -12,6 +12,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/module.h>
+#include <linux/kmod.h>
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/moduleparam.h>
@@ -87,12 +88,7 @@ void v9fs_unregister_trans(struct p9_trans_module *m)
 }
 EXPORT_SYMBOL(v9fs_unregister_trans);
 
-/**
- * v9fs_get_trans_by_name - get transport with the matching name
- * @s: string identifying transport
- *
- */
-struct p9_trans_module *v9fs_get_trans_by_name(char *s)
+static struct p9_trans_module *_p9_get_trans_by_name(const char *s)
 {
 	struct p9_trans_module *t, *found = NULL;
 
@@ -106,6 +102,28 @@ struct p9_trans_module *v9fs_get_trans_by_name(char *s)
 		}
 
 	spin_unlock(&v9fs_trans_lock);
+
+	return found;
+}
+
+/**
+ * v9fs_get_trans_by_name - get transport with the matching name
+ * @s: string identifying transport
+ *
+ */
+struct p9_trans_module *v9fs_get_trans_by_name(const char *s)
+{
+	struct p9_trans_module *found = NULL;
+
+	found = _p9_get_trans_by_name(s);
+
+#ifdef CONFIG_MODULES
+	if (!found) {
+		request_module("9p-%s", s);
+		found = _p9_get_trans_by_name(s);
+	}
+#endif
+
 	return found;
 }
 EXPORT_SYMBOL(v9fs_get_trans_by_name);
diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index af0a8a6cd3fd..480fd27760d7 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -767,6 +767,7 @@ static void __exit p9_trans_rdma_exit(void)
 
 module_init(p9_trans_rdma_init);
 module_exit(p9_trans_rdma_exit);
+MODULE_ALIAS_9P("rdma");
 
 MODULE_AUTHOR("Tom Tucker <tom@opengridcomputing.com>");
 MODULE_DESCRIPTION("RDMA Transport for 9P");
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 490a4c900339..bd5a89c4960d 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -794,6 +794,7 @@ static void __exit p9_virtio_cleanup(void)
 
 module_init(p9_virtio_init);
 module_exit(p9_virtio_cleanup);
+MODULE_ALIAS_9P("virtio");
 
 MODULE_DEVICE_TABLE(virtio, id_table);
 MODULE_AUTHOR("Eric Van Hensbergen <ericvh@gmail.com>");
diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 3ec1a51a6944..e264dcee019a 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -552,6 +552,7 @@ static int p9_trans_xen_init(void)
 	return rc;
 }
 module_init(p9_trans_xen_init);
+MODULE_ALIAS_9P("xen");
 
 static void p9_trans_xen_exit(void)
 {
-- 
2.33.1

[PATCH v2 2/4] 9p/trans_fd: split into dedicated module

[Thomas Weißschuh]

This allows these transports only to be used when needed.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/net/9p/9p.h |  2 --
 net/9p/Kconfig      |  7 +++++++
 net/9p/Makefile     |  5 ++++-
 net/9p/mod.c        |  2 --
 net/9p/trans_fd.c   | 14 ++++++++++++--
 5 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
index 03614de86942..f420f8cb378d 100644
--- a/include/net/9p/9p.h
+++ b/include/net/9p/9p.h
@@ -553,6 +553,4 @@ struct p9_fcall {
 int p9_errstr2errno(char *errstr, int len);
 
 int p9_error_init(void);
-int p9_trans_fd_init(void);
-void p9_trans_fd_exit(void);
 #endif /* NET_9P_H */
diff --git a/net/9p/Kconfig b/net/9p/Kconfig
index 64468c49791f..af601129f1bb 100644
--- a/net/9p/Kconfig
+++ b/net/9p/Kconfig
@@ -15,6 +15,13 @@ menuconfig NET_9P
 
 if NET_9P
 
+config NET_9P_FD
+	depends on VIRTIO
+	tristate "9P FD Transport"
+	help
+	  This builds support for transports over TCP, Unix sockets and
+	  filedescriptors.
+
 config NET_9P_VIRTIO
 	depends on VIRTIO
 	tristate "9P Virtio Transport"
diff --git a/net/9p/Makefile b/net/9p/Makefile
index aa0a5641e5d0..1df9b344c30b 100644
--- a/net/9p/Makefile
+++ b/net/9p/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_NET_9P) := 9pnet.o
+obj-$(CONFIG_NET_9P_FD) += 9pnet_fd.o
 obj-$(CONFIG_NET_9P_XEN) += 9pnet_xen.o
 obj-$(CONFIG_NET_9P_VIRTIO) += 9pnet_virtio.o
 obj-$(CONFIG_NET_9P_RDMA) += 9pnet_rdma.o
@@ -9,9 +10,11 @@ obj-$(CONFIG_NET_9P_RDMA) += 9pnet_rdma.o
 	client.o \
 	error.o \
 	protocol.o \
-	trans_fd.o \
 	trans_common.o \
 
+9pnet_fd-objs := \
+	trans_fd.o \
+
 9pnet_virtio-objs := \
 	trans_virtio.o \
 
diff --git a/net/9p/mod.c b/net/9p/mod.c
index c95416c1d1a2..8f1d067b272e 100644
--- a/net/9p/mod.c
+++ b/net/9p/mod.c
@@ -182,7 +182,6 @@ static int __init init_p9(void)
 
 	p9_error_init();
 	pr_info("Installing 9P2000 support\n");
-	p9_trans_fd_init();
 
 	return ret;
 }
@@ -196,7 +195,6 @@ static void __exit exit_p9(void)
 {
 	pr_info("Unloading 9P2000 support\n");
 
-	p9_trans_fd_exit();
 	p9_client_exit();
 }
 
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 007bbcc68010..e3f4a7a5c845 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -1092,6 +1092,7 @@ static struct p9_trans_module p9_tcp_trans = {
 	.show_options = p9_fd_show_options,
 	.owner = THIS_MODULE,
 };
+MODULE_ALIAS_9P("tcp");
 
 static struct p9_trans_module p9_unix_trans = {
 	.name = "unix",
@@ -1105,6 +1106,7 @@ static struct p9_trans_module p9_unix_trans = {
 	.show_options = p9_fd_show_options,
 	.owner = THIS_MODULE,
 };
+MODULE_ALIAS_9P("unix");
 
 static struct p9_trans_module p9_fd_trans = {
 	.name = "fd",
@@ -1118,6 +1120,7 @@ static struct p9_trans_module p9_fd_trans = {
 	.show_options = p9_fd_show_options,
 	.owner = THIS_MODULE,
 };
+MODULE_ALIAS_9P("fd");
 
 /**
  * p9_poll_workfn - poll worker thread
@@ -1151,7 +1154,7 @@ static void p9_poll_workfn(struct work_struct *work)
 	p9_debug(P9_DEBUG_TRANS, "finish\n");
 }
 
-int p9_trans_fd_init(void)
+static int __init p9_trans_fd_init(void)
 {
 	v9fs_register_trans(&p9_tcp_trans);
 	v9fs_register_trans(&p9_unix_trans);
@@ -1160,10 +1163,17 @@ int p9_trans_fd_init(void)
 	return 0;
 }
 
-void p9_trans_fd_exit(void)
+static void __exit p9_trans_fd_exit(void)
 {
 	flush_work(&p9_poll_work);
 	v9fs_unregister_trans(&p9_tcp_trans);
 	v9fs_unregister_trans(&p9_unix_trans);
 	v9fs_unregister_trans(&p9_fd_trans);
 }
+
+module_init(p9_trans_fd_init);
+module_exit(p9_trans_fd_exit);
+
+MODULE_AUTHOR("Eric Van Hensbergen <ericvh@gmail.com>");
+MODULE_DESCRIPTION("Filedescriptor Transport for 9P");
+MODULE_LICENSE("GPL");
-- 
2.33.1

[PATCH v2 3/4] 9p/xen: autoload when xenbus service is available

[Thomas Weißschuh]

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 net/9p/trans_xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index e264dcee019a..9c4c565f92e4 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -561,6 +561,7 @@ static void p9_trans_xen_exit(void)
 }
 module_exit(p9_trans_xen_exit);
 
+MODULE_ALIAS("xen:9pfs");
 MODULE_AUTHOR("Stefano Stabellini <stefano@aporeto.com>");
 MODULE_DESCRIPTION("Xen Transport for 9P");
 MODULE_LICENSE("GPL");
-- 
2.33.1

[PATCH v2 4/4] net/p9: load default transports

[Thomas Weißschuh]

Now that all transports are split into modules it may happen that no
transports are registered when v9fs_get_default_trans() is called.
When that is the case try to load more transports from modules.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 net/9p/mod.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/9p/mod.c b/net/9p/mod.c
index 8f1d067b272e..7bb875cd279f 100644
--- a/net/9p/mod.c
+++ b/net/9p/mod.c
@@ -128,6 +128,10 @@ struct p9_trans_module *v9fs_get_trans_by_name(const char *s)
 }
 EXPORT_SYMBOL(v9fs_get_trans_by_name);
 
+static const char * const v9fs_default_transports[] = {
+	"virtio", "tcp", "fd", "unix", "xen", "rdma",
+};
+
 /**
  * v9fs_get_default_trans - get the default transport
  *
@@ -136,6 +140,7 @@ EXPORT_SYMBOL(v9fs_get_trans_by_name);
 struct p9_trans_module *v9fs_get_default_trans(void)
 {
 	struct p9_trans_module *t, *found = NULL;
+	int i;
 
 	spin_lock(&v9fs_trans_lock);
 
@@ -153,6 +158,10 @@ struct p9_trans_module *v9fs_get_default_trans(void)
 			}
 
 	spin_unlock(&v9fs_trans_lock);
+
+	for (i = 0; !found && i < ARRAY_SIZE(v9fs_default_transports); i++)
+		found = v9fs_get_trans_by_name(v9fs_default_transports[i]);
+
 	return found;
 }
 EXPORT_SYMBOL(v9fs_get_default_trans);
-- 
2.33.1

Re: [PATCH v2 4/4] net/p9: load default transports

[Thomas Weißschuh]

Hi Dominique,

On 2021-11-03 20:38+0100, Thomas Weißschuh wrote:
> Now that all transports are split into modules it may happen that no
> transports are registered when v9fs_get_default_trans() is called.
> When that is the case try to load more transports from modules.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  net/9p/mod.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/9p/mod.c b/net/9p/mod.c
> index 8f1d067b272e..7bb875cd279f 100644
> --- a/net/9p/mod.c
> +++ b/net/9p/mod.c
> @@ -128,6 +128,10 @@ struct p9_trans_module *v9fs_get_trans_by_name(const char *s)
>  }
>  EXPORT_SYMBOL(v9fs_get_trans_by_name);
>  
> +static const char * const v9fs_default_transports[] = {
> +	"virtio", "tcp", "fd", "unix", "xen", "rdma",
> +};
> +
>  /**
>   * v9fs_get_default_trans - get the default transport
>   *
> @@ -136,6 +140,7 @@ EXPORT_SYMBOL(v9fs_get_trans_by_name);
>  struct p9_trans_module *v9fs_get_default_trans(void)
>  {
>  	struct p9_trans_module *t, *found = NULL;
> +	int i;
>  
>  	spin_lock(&v9fs_trans_lock);
>  
> @@ -153,6 +158,10 @@ struct p9_trans_module *v9fs_get_default_trans(void)
>  			}
>  
>  	spin_unlock(&v9fs_trans_lock);
> +
> +	for (i = 0; !found && i < ARRAY_SIZE(v9fs_default_transports); i++)
> +		found = v9fs_get_trans_by_name(v9fs_default_transports[i]);
> +
>  	return found;
>  }
>  EXPORT_SYMBOL(v9fs_get_default_trans);
> -- 
> 2.33.1

I did not notice that you already had applied "net/9p: autoload transport modules"
to your tree when sending this series.

Please note that in this series I modified patch 1 a bit, from the ony you
applied, to prevent warnings in patch 4.
Concretely I modified the prototypes of `v9fs_get_trans_by_name()` and
`_p9_get_trans_by_name()` to take const parameters.

Feel free to roll those changes into this patch when applying or I can resend
the patch/series.

Thomas

Re: [PATCH v2 4/4] net/p9: load default transports

[Dominique Martinet]

Hi,

Thomas Weißschuh wrote on Mon, Nov 08, 2021 at 07:50:34PM +0100:
> I did not notice that you already had applied "net/9p: autoload transport modules"
> to your tree when sending this series.
> 
> Please note that in this series I modified patch 1 a bit, from the ony you
> applied, to prevent warnings in patch 4.
> Concretely I modified the prototypes of `v9fs_get_trans_by_name()` and
> `_p9_get_trans_by_name()` to take const parameters.
> 
> Feel free to roll those changes into this patch when applying or I can resend
> the patch/series.

Thanks for the heads up, it's ok -- I'll move the constification of
these functions to patch 4 myself.

I've just sent my pull request to Linus so will take your patches to
my for-next branch when that's merged.
-- 
Dominique

Re: [PATCH v2 2/4] 9p/trans_fd: split into dedicated module

[Dominique Martinet]

Hi Thomas,

it's been a while but I had a second look as I intend on submitting this
next week, just a small fixup on the Kconfig entry

Thomas Weißschuh wrote on Wed, Nov 03, 2021 at 08:38:21PM +0100:
> diff --git a/net/9p/Kconfig b/net/9p/Kconfig
> index 64468c49791f..af601129f1bb 100644
> --- a/net/9p/Kconfig
> +++ b/net/9p/Kconfig
> @@ -15,6 +15,13 @@ menuconfig NET_9P
>  
>  if NET_9P
>  
> +config NET_9P_FD
> +	depends on VIRTIO

I think that's just a copypaste leftover from NET_9P_VIRTIO ?
Since it used to be code within NET_9P and it's within a if NET_9P it
shouldn't depend on anything.

Also for compatibility I'd suggest we keep it on by default at this
point, e.g. add 'default NET_9P' to this block:


diff --git a/net/9p/Kconfig b/net/9p/Kconfig
index af601129f1bb..deabbd376cb1 100644
--- a/net/9p/Kconfig
+++ b/net/9p/Kconfig
@@ -16,7 +16,7 @@ menuconfig NET_9P
 if NET_9P
 
 config NET_9P_FD
-       depends on VIRTIO
+       default NET_9P
        tristate "9P FD Transport"
        help
          This builds support for transports over TCP, Unix sockets and


I'll just fixup the commit with a word in the message unless you have a
problem with it, please let me know! :)

-- 
Dominique

Re: [PATCH v2 2/4] 9p/trans_fd: split into dedicated module

[Thomas Weißschuh]

Hi Dominique,

On 2022-01-10 09:57+0900, Dominique Martinet wrote:
> Hi Thomas,
> 
> it's been a while but I had a second look as I intend on submitting this
> next week, just a small fixup on the Kconfig entry
> 
> Thomas Weißschuh wrote on Wed, Nov 03, 2021 at 08:38:21PM +0100:
> > diff --git a/net/9p/Kconfig b/net/9p/Kconfig
> > index 64468c49791f..af601129f1bb 100644
> > --- a/net/9p/Kconfig
> > +++ b/net/9p/Kconfig
> > @@ -15,6 +15,13 @@ menuconfig NET_9P
> >  
> >  if NET_9P
> >  
> > +config NET_9P_FD
> > +	depends on VIRTIO
> 
> I think that's just a copypaste leftover from NET_9P_VIRTIO ?
> Since it used to be code within NET_9P and it's within a if NET_9P it
> shouldn't depend on anything.
> 
> Also for compatibility I'd suggest we keep it on by default at this
> point, e.g. add 'default NET_9P' to this block:

Yes, you are correct on both points.

> diff --git a/net/9p/Kconfig b/net/9p/Kconfig
> index af601129f1bb..deabbd376cb1 100644
> --- a/net/9p/Kconfig
> +++ b/net/9p/Kconfig
> @@ -16,7 +16,7 @@ menuconfig NET_9P
>  if NET_9P
>  
>  config NET_9P_FD
> -       depends on VIRTIO
> +       default NET_9P
>         tristate "9P FD Transport"
>         help
>           This builds support for transports over TCP, Unix sockets and
> 
> 
> I'll just fixup the commit with a word in the message unless you have a
> problem with it, please let me know! :)

Looks good, thanks!

Thomas