Re: [PATCH 2/2] nvme: keyring: fix conditional compilation

Keith Busch
2023-11-07T17:49:56Z

On Fri, Oct 27, 2023 at 11:21:50AM +0200, Christoph Hellwig wrote:
> On Fri, Oct 27, 2023 at 11:08:23AM +0200, Hannes Reinecke wrote:
> > obj-$(CONFIG_MOD1) += mod1.o mod-common.o
> > obj-$(CONFIG_MOD2) += mod2.o mod-common.o
> >
> > and mod-common contains a 'module_init()' and a 'module_exit()' function,
> > what happens if I load mod2 after mod1?
>
> mod-common is already loaded and won't be loaded again.
>
> > And what happens if I unload mod2, but keep mod1?
>
> mod-common doesn't become available for unloading until all
> modules using it are unloaded first.

Looks like this works, atop patch 1:

---
diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index 46d7a537dbc2e..c35b3a287a910 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -151,7 +151,7 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
}
EXPORT_SYMBOL_GPL(nvme_tls_psk_default);

-int nvme_keyring_init(void)
+static int __init nvme_keyring_init(void)
{
int err;

@@ -171,14 +171,14 @@ int nvme_keyring_init(void)
}
return 0;
}
-EXPORT_SYMBOL_GPL(nvme_keyring_init);

-void nvme_keyring_exit(void)
+static void __exit nvme_keyring_exit(void)
{
unregister_key_type(&nvme_tls_psk_key_type);
key_revoke(nvme_keyring);
key_put(nvme_keyring);
}
-EXPORT_SYMBOL_GPL(nvme_keyring_exit);

MODULE_LICENSE("GPL v2");
+module_init(nvme_keyring_init);
+module_exit(nvme_keyring_exit);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 75a1b58a7a436..683b694afdead 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4737,16 +4737,11 @@ static int __init nvme_core_init(void)
result = PTR_ERR(nvme_ns_chr_class);
goto unregister_generic_ns;
}
- result = nvme_keyring_init();
- if (result)
- goto destroy_ns_chr;
result = nvme_init_auth();
if (result)
- goto keyring_exit;
+ goto destroy_ns_chr;
return 0;

-keyring_exit:
- nvme_keyring_exit();
destroy_ns_chr:
class_destroy(nvme_ns_chr_class);
unregister_generic_ns:
@@ -4770,7 +4765,6 @@ static int __init nvme_core_init(void)
static void __exit nvme_core_exit(void)
{
nvme_exit_auth();
- nvme_keyring_exit();
class_destroy(nvme_ns_chr_class);
class_destroy(nvme_subsys_class);
class_destroy(nvme_class);
diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h
index 4efea9dd967c1..f4f6634e85846 100644
--- a/include/linux/nvme-keyring.h
+++ b/include/linux/nvme-keyring.h
diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index 46d7a537dbc2e..c35b3a287a910 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -151,7 +151,7 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
}
EXPORT_SYMBOL_GPL(nvme_tls_psk_default);

-int nvme_keyring_init(void)
+static int __init nvme_keyring_init(void)
{
int err;

@@ -171,14 +171,14 @@ int nvme_keyring_init(void)
}
return 0;
}
-EXPORT_SYMBOL_GPL(nvme_keyring_init);

-void nvme_keyring_exit(void)
+static void __exit nvme_keyring_exit(void)
{
unregister_key_type(&nvme_tls_psk_key_type);
key_revoke(nvme_keyring);
key_put(nvme_keyring);
}
-EXPORT_SYMBOL_GPL(nvme_keyring_exit);

MODULE_LICENSE("GPL v2");
+module_init(nvme_keyring_init);
+module_exit(nvme_keyring_exit);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 75a1b58a7a436..683b694afdead 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4737,16 +4737,11 @@ static int __init nvme_core_init(void)
result = PTR_ERR(nvme_ns_chr_class);
goto unregister_generic_ns;
}
- result = nvme_keyring_init();
- if (result)
- goto destroy_ns_chr;
result = nvme_init_auth();
if (result)
- goto keyring_exit;
+ goto destroy_ns_chr;
return 0;

-keyring_exit:
- nvme_keyring_exit();
destroy_ns_chr:
class_destroy(nvme_ns_chr_class);
unregister_generic_ns:
@@ -4770,7 +4765,6 @@ static int __init nvme_core_init(void)
static void __exit nvme_core_exit(void)
{
nvme_exit_auth();
- nvme_keyring_exit();
class_destroy(nvme_ns_chr_class);
class_destroy(nvme_subsys_class);
class_destroy(nvme_class);
diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h
index 4efea9dd967c1..f4f6634e85846 100644
--- a/include/linux/nvme-keyring.h
+++ b/include/linux/nvme-keyring.h
@@ -6,14 +6,12 @@
#ifndef _NVME_KEYRING_H
#define _NVME_KEYRING_H

-#ifdef CONFIG_NVME_KEYRING
+#if IS_ENABLED(CONFIG_NVME_KEYRING)

key_serial_t nvme_tls_psk_default(struct key *keyring,
const char *hostnqn, const char *subnqn);

key_serial_t nvme_keyring_id(void);
-int nvme_keyring_init(void);
-void nvme_keyring_exit(void);

#else

@@ -26,11 +24,6 @@ static inline key_serial_t nvme_keyring_id(void)
{
return 0;
}
-static inline int nvme_keyring_init(void)
-{
- return 0;
-}
-static inline void nvme_keyring_exit(void) {}

#endif /* !CONFIG_NVME_KEYRING */
#endif /* _NVME_KEYRING_H */
@@ -6,14 +6,12 @@
#ifndef _NVME_KEYRING_H
#define _NVME_KEYRING_H

-#ifdef CONFIG_NVME_KEYRING
+#if IS_ENABLED(CONFIG_NVME_KEYRING)

key_serial_t nvme_tls_psk_default(struct key *keyring,
const char *hostnqn, const char *subnqn);

key_serial_t nvme_keyring_id(void);
-int nvme_keyring_init(void);
-void nvme_keyring_exit(void);

#else

@@ -26,11 +24,6 @@ static inline key_serial_t nvme_keyring_id(void)
{
return 0;
}
-static inline int nvme_keyring_init(void)
-{
- return 0;
-}
-static inline void nvme_keyring_exit(void) {}

#endif /* !CONFIG_NVME_KEYRING */
#endif /* _NVME_KEYRING_H */
--