Re: [v1 2/3] include: move lookup_or_create_module_kobject()/to_module* to module.h

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

 





Le 04/02/2025 à 06:22, Shyam Saini a écrit :
[Vous ne recevez pas souvent de courriers de shyamsaini@xxxxxxxxxxxxxxxxxxx. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]

Move the following to module.h to allow common usages:
  - lookup_or_create_module_kobject()
  - to_module_attr
  - to_module_kobject

This makes lookup_or_create_module_kobject() as inline.

Why an inline ? It looks quite big for an inline. Why not just make it global ?


Signed-off-by: Shyam Saini <shyamsaini@xxxxxxxxxxxxxxxxxxx>
---
  include/linux/module.h | 39 +++++++++++++++++++++++++++++++++++++++
  kernel/params.c        | 39 ---------------------------------------
  2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 12f8a7d4fc1c..ba5f5e6c3927 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -894,8 +894,47 @@ static inline void *module_writable_address(struct module *mod, void *loc)
  #endif /* CONFIG_MODULES */

  #ifdef CONFIG_SYSFS
+/* sysfs output in /sys/modules/XYZ/parameters/ */
+#define to_module_attr(n) container_of_const(n, struct module_attribute, attr)
+#define to_module_kobject(n) container_of(n, struct module_kobject, kobj)
  extern struct kset *module_kset;
  extern const struct kobj_type module_ktype;
+
+static inline struct module_kobject * __init lookup_or_create_module_kobject(const char *name)
+{
+       struct module_kobject *mk;
+       struct kobject *kobj;
+       int err;
+
+       kobj = kset_find_obj(module_kset, name);
+       if (kobj) {
+               mk = to_module_kobject(kobj);

Would look cleaner without the else. Something like:

	if (kobj)
		return to_module_kobject(kobj);

	mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
...


+       } else {
+               mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
+               BUG_ON(!mk);

Why a BUG_ON() ? Why not just return NULL and let the caller handle the failure ?

+
+               mk->mod = THIS_MODULE;
+               mk->kobj.kset = module_kset;
+               err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
+                                          "%s", name);
+#ifdef CONFIG_MODULES

This #ifdef is not needed, module_uevent is declared at all time in module.h, IS_ENABLED(CONFIG_MODULES) should be enough.

+               if (!err)
+                       err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
+#endif
+               if (err) {
+                       kobject_put(&mk->kobj);
+                       pr_crit("Adding module '%s' to sysfs failed (%d), the system may be unstable.\n",
+                               name, err);
+                       return NULL;
+               }
+
+               /* So that we hold reference in both cases. */
+               kobject_get(&mk->kobj);
+       }
+
+       return mk;
+}
+
  #endif /* CONFIG_SYSFS */

  #define symbol_request(x) try_then_request_module(symbol_get(x), "symbol:" #x)
diff --git a/kernel/params.c b/kernel/params.c
index 4b43baaf7c83..3c0798b79f76 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -537,10 +537,6 @@ const struct kernel_param_ops param_ops_string = {
  };
  EXPORT_SYMBOL(param_ops_string);

-/* sysfs output in /sys/modules/XYZ/parameters/ */
-#define to_module_attr(n) container_of_const(n, struct module_attribute, attr)
-#define to_module_kobject(n) container_of(n, struct module_kobject, kobj)
-
  struct param_attribute
  {
         struct module_attribute mattr;
@@ -763,41 +759,6 @@ void destroy_params(const struct kernel_param *params, unsigned num)
                         params[i].ops->free(params[i].arg);
  }

-static struct module_kobject * __init lookup_or_create_module_kobject(const char *name)
-{
-       struct module_kobject *mk;
-       struct kobject *kobj;
-       int err;
-
-       kobj = kset_find_obj(module_kset, name);
-       if (kobj) {
-               mk = to_module_kobject(kobj);
-       } else {
-               mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
-               BUG_ON(!mk);
-
-               mk->mod = THIS_MODULE;
-               mk->kobj.kset = module_kset;
-               err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
-                                          "%s", name);
-#ifdef CONFIG_MODULES
-               if (!err)
-                       err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
-#endif
-               if (err) {
-                       kobject_put(&mk->kobj);
-                       pr_crit("Adding module '%s' to sysfs failed (%d), the system may be unstable.\n",
-                               name, err);
-                       return NULL;
-               }
-
-               /* So that we hold reference in both cases. */
-               kobject_get(&mk->kobj);
-       }
-
-       return mk;
-}
-
  static void __init kernel_add_sysfs_param(const char *name,
                                           const struct kernel_param *kparam,
                                           unsigned int name_skip)
--
2.34.1







[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux