Le 02/12/2022 à 23:47, Allen Webb a écrit :
> This adds the modalias sysfs attribute in preparation for its
> implementation.
>
> Signed-off-by: Allen Webb <allenwebb@xxxxxxxxxx>
> ---
> include/linux/module.h | 1 +
> kernel/module/internal.h | 2 ++
> kernel/module/sysfs.c | 33 +++++++++++++++++++++++++++++++++
> kernel/params.c | 7 +++++++
> 4 files changed, 43 insertions(+)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index ec61fb53979a9..0bfa859a21566 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -47,6 +47,7 @@ struct module_kobject {
> struct kobject *drivers_dir;
> struct module_param_attrs *mp;
> struct completion *kobj_completion;
> + struct bin_attribute modalias_attr;
> } __randomize_layout;
>
> struct module_attribute {
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 2e2bf236f5582..8d7ae37584868 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -259,11 +259,13 @@ static inline void add_kallsyms(struct module *mod, const struct load_info *info
> #endif /* CONFIG_KALLSYMS */
>
> #ifdef CONFIG_SYSFS
> +void add_modalias_attr(struct module_kobject *mk);
> int mod_sysfs_setup(struct module *mod, const struct load_info *info,
> struct kernel_param *kparam, unsigned int num_params);
> void mod_sysfs_teardown(struct module *mod);
> void init_param_lock(struct module *mod);
> #else /* !CONFIG_SYSFS */
> +static inline void add_modalias_attr(struct module_kobject *mk) {}
> static inline int mod_sysfs_setup(struct module *mod,
> const struct load_info *info,
> struct kernel_param *kparam,
> diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
> index ce68f821dcd12..8dafec7455fbe 100644
> --- a/kernel/module/sysfs.c
> +++ b/kernel/module/sysfs.c
> @@ -240,6 +240,37 @@ static inline void add_notes_attrs(struct module *mod, const struct load_info *i
> static inline void remove_notes_attrs(struct module *mod) { }
> #endif /* CONFIG_KALLSYMS */
>
> +static ssize_t module_modalias_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr,
> + char *buf, loff_t pos, size_t count)
> +{
> + return 0;
> +}
> +
> +/* Used in kernel/params.c for builtin modules.
> + *
> + * `struct module_kobject` is used instead of `struct module` because for
> + * builtin modules, the `struct module` is not available when this is called.
> + */
You are using network comment style. Other parts of the kernel have
different style. See
https://docs.kernel.org/process/coding-style.html#commenting
> +void add_modalias_attr(struct module_kobject *mk)
> +{
> + sysfs_bin_attr_init(&mk->modalias_attr);
> + mk->modalias_attr.attr.name = "modalias";
> + mk->modalias_attr.attr.mode = 0444;
> + mk->modalias_attr.read = module_modalias_read;
> + if (sysfs_create_bin_file(&mk->kobj, &mk->modalias_attr)) {
> + /* We shouldn't ignore the return type, but there is nothing to
> + * do.
> + */
> + return;
> + }
I would have put the comment before the if, it would have been a single
line comment and you would have avoided the { }
> +}
> +
> +static void remove_modalias_attr(struct module_kobject *mk)
> +{
> + sysfs_remove_bin_file(&mk->kobj, &mk->modalias_attr);
> +}
> +
> static void del_usage_links(struct module *mod)
> {
> #ifdef CONFIG_MODULE_UNLOAD
> @@ -398,6 +429,7 @@ int mod_sysfs_setup(struct module *mod,
>
> add_sect_attrs(mod, info);
> add_notes_attrs(mod, info);
> + add_modalias_attr(&mod->mkobj);
>
> return 0;
>
> @@ -415,6 +447,7 @@ int mod_sysfs_setup(struct module *mod,
>
> static void mod_sysfs_fini(struct module *mod)
> {
> + remove_modalias_attr(&mod->mkobj);
> remove_notes_attrs(mod);
> remove_sect_attrs(mod);
> mod_kobject_put(mod);
> diff --git a/kernel/params.c b/kernel/params.c
> index 5b92310425c50..b7fd5313a3118 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -14,6 +14,12 @@
> #include <linux/ctype.h>
> #include <linux/security.h>
>
> +#ifdef CONFIG_MODULES
> +#include "module/internal.h"
> +#else
> +static inline void add_modalias_attr(struct module_kobject *mk) {}
> +#endif /* !CONFIG_MODULES */
It is odd to include module internal.h outside of module.
If you really need a function from module, I think you should move its
prototype to include/linux/module.h
> +
> #ifdef CONFIG_SYSFS
> /* Protects all built-in parameters, modules use their own param_lock */
> static DEFINE_MUTEX(param_lock);
> @@ -815,6 +821,7 @@ static void __init kernel_add_sysfs_param(const char *name,
> BUG_ON(err);
> kobject_uevent(&mk->kobj, KOBJ_ADD);
> kobject_put(&mk->kobj);
> + add_modalias_attr(mk);
> }
>
> /*
