Hello
This patch serie suppress the following warning:
kernel/module.c: In function 'add_usage_links':
kernel/module.c:1653:6: warning: variable 'nowarn' set but not used [-Wunused-but-set-variable]
Changes since v1:
- renamed out_unreg_usage_links to out_unreg_modinfo_attrs
- added extra newline
- added missing call to del_usage_link() in case of add_usage_link() error
- added patch #1
Corentin Labbe (2):
kernel/module.c: Invert add_usage_link and del_usage_link functions
kernel/module.c: suppress warning about unused nowarn variable
kernel/module.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
--
2.13.0
This patch fix the following warning:
kernel/module.c: In function 'add_usage_links':
kernel/module.c:1653:6: warning: variable 'nowarn' set but not used [-Wunused-but-set-variable]
Signed-off-by: Corentin Labbe <[email protected]>
---
kernel/module.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 983b81f6d4ba..6b2592193730 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1675,19 +1675,24 @@ static void del_usage_links(struct module *mod)
#endif
}
-static void add_usage_links(struct module *mod)
+static int add_usage_links(struct module *mod)
{
+ int ret = 0;
#ifdef CONFIG_MODULE_UNLOAD
struct module_use *use;
- int nowarn;
mutex_lock(&module_mutex);
list_for_each_entry(use, &mod->target_list, target_list) {
- nowarn = sysfs_create_link(use->target->holders_dir,
- &mod->mkobj.kobj, mod->name);
+ ret = sysfs_create_link(use->target->holders_dir,
+ &mod->mkobj.kobj, mod->name);
+ if (ret)
+ break;
}
mutex_unlock(&module_mutex);
+ if (ret)
+ del_usage_links(mod);
#endif
+ return ret;
}
static int module_add_modinfo_attrs(struct module *mod)
@@ -1798,13 +1803,18 @@ static int mod_sysfs_setup(struct module *mod,
if (err)
goto out_unreg_param;
- add_usage_links(mod);
+ err = add_usage_links(mod);
+ if (err)
+ goto out_unreg_modinfo_attrs;
+
add_sect_attrs(mod, info);
add_notes_attrs(mod, info);
kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
return 0;
+out_unreg_modinfo_attrs:
+ module_remove_modinfo_attrs(mod);
out_unreg_param:
module_param_sysfs_remove(mod);
out_unreg_holders:
--
2.13.0
This patch just swap del_usage_link() before add_usage_link().
Signed-off-by: Corentin Labbe <[email protected]>
---
kernel/module.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index e3e9dbba6a5b..983b81f6d4ba 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1663,29 +1663,29 @@ static inline void remove_notes_attrs(struct module *mod)
}
#endif /* CONFIG_KALLSYMS */
-static void add_usage_links(struct module *mod)
+static void del_usage_links(struct module *mod)
{
#ifdef CONFIG_MODULE_UNLOAD
struct module_use *use;
- int nowarn;
mutex_lock(&module_mutex);
- list_for_each_entry(use, &mod->target_list, target_list) {
- nowarn = sysfs_create_link(use->target->holders_dir,
- &mod->mkobj.kobj, mod->name);
- }
+ list_for_each_entry(use, &mod->target_list, target_list)
+ sysfs_remove_link(use->target->holders_dir, mod->name);
mutex_unlock(&module_mutex);
#endif
}
-static void del_usage_links(struct module *mod)
+static void add_usage_links(struct module *mod)
{
#ifdef CONFIG_MODULE_UNLOAD
struct module_use *use;
+ int nowarn;
mutex_lock(&module_mutex);
- list_for_each_entry(use, &mod->target_list, target_list)
- sysfs_remove_link(use->target->holders_dir, mod->name);
+ list_for_each_entry(use, &mod->target_list, target_list) {
+ nowarn = sysfs_create_link(use->target->holders_dir,
+ &mod->mkobj.kobj, mod->name);
+ }
mutex_unlock(&module_mutex);
#endif
}
--
2.13.0
+++ Corentin Labbe [06/06/17 14:17 +0200]:
>This patch just swap del_usage_link() before add_usage_link().
>
>Signed-off-by: Corentin Labbe <[email protected]>
Could you combine this with the 2nd patch? By itself this patch
doesn't tell us much. Additionally, could you explain in the changelog
(of the 2nd patch) why they needed to be swapped (i.e., so
del_usage_links() can be called from add_usage_links()).
Thanks!
Jessica
> kernel/module.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index e3e9dbba6a5b..983b81f6d4ba 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -1663,29 +1663,29 @@ static inline void remove_notes_attrs(struct module *mod)
> }
> #endif /* CONFIG_KALLSYMS */
>
>-static void add_usage_links(struct module *mod)
>+static void del_usage_links(struct module *mod)
> {
> #ifdef CONFIG_MODULE_UNLOAD
> struct module_use *use;
>- int nowarn;
>
> mutex_lock(&module_mutex);
>- list_for_each_entry(use, &mod->target_list, target_list) {
>- nowarn = sysfs_create_link(use->target->holders_dir,
>- &mod->mkobj.kobj, mod->name);
>- }
>+ list_for_each_entry(use, &mod->target_list, target_list)
>+ sysfs_remove_link(use->target->holders_dir, mod->name);
> mutex_unlock(&module_mutex);
> #endif
> }
>
>-static void del_usage_links(struct module *mod)
>+static void add_usage_links(struct module *mod)
> {
> #ifdef CONFIG_MODULE_UNLOAD
> struct module_use *use;
>+ int nowarn;
>
> mutex_lock(&module_mutex);
>- list_for_each_entry(use, &mod->target_list, target_list)
>- sysfs_remove_link(use->target->holders_dir, mod->name);
>+ list_for_each_entry(use, &mod->target_list, target_list) {
>+ nowarn = sysfs_create_link(use->target->holders_dir,
>+ &mod->mkobj.kobj, mod->name);
>+ }
> mutex_unlock(&module_mutex);
> #endif
> }
>--
>2.13.0
>
On Mon, Jun 19, 2017 at 06:26:23PM +0200, Jessica Yu wrote:
> +++ Corentin Labbe [06/06/17 14:17 +0200]:
> >This patch just swap del_usage_link() before add_usage_link().
> >
> >Signed-off-by: Corentin Labbe <[email protected]>
>
> Could you combine this with the 2nd patch? By itself this patch
> doesn't tell us much. Additionally, could you explain in the changelog
> (of the 2nd patch) why they needed to be swapped (i.e., so
> del_usage_links() can be called from add_usage_links()).
>
> Thanks!
>
> Jessica
>
I think that its against the rule of atomic/simple patch.
Perhaps, the first patch miss some "why I do it"
Anyway I will send a new version as you requested
Regards
+++ Corentin Labbe [20/06/17 08:45 +0200]:
>On Mon, Jun 19, 2017 at 06:26:23PM +0200, Jessica Yu wrote:
>> +++ Corentin Labbe [06/06/17 14:17 +0200]:
>> >This patch just swap del_usage_link() before add_usage_link().
>> >
>> >Signed-off-by: Corentin Labbe <[email protected]>
>>
>> Could you combine this with the 2nd patch? By itself this patch
>> doesn't tell us much. Additionally, could you explain in the changelog
>> (of the 2nd patch) why they needed to be swapped (i.e., so
>> del_usage_links() can be called from add_usage_links()).
>>
>> Thanks!
>>
>> Jessica
>>
>
>I think that its against the rule of atomic/simple patch.
>Perhaps, the first patch miss some "why I do it"
>
>Anyway I will send a new version as you requested
Hi Corentin,
I've folded the first patch with the second and applied them to
modules-next, no need to resend the patchset :)
Thanks!
Jessica