The resctrl default control group is created during kernel init time.
If the new files are to be added to the default group based on the mount
option, then each file needs to be created separately and call
kernfs_activate() again.
This can be avoided if all the files are created during the mount and
destroyed during the umount. So, move the default group creation
in rdt_get_tree() and removal in rdt_kill_sb().
Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
arch/x86/kernel/cpu/resctrl/internal.h | 1 +
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 50 +++++++++++++++++---------------
2 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 37800724e002..2bd92c0c3b0c 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -602,5 +602,6 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
void __init thread_throttle_mode_init(void);
void __init mbm_config_rftype_init(const char *config);
void rdt_staged_configs_clear(void);
+int rdtgroup_setup_root(struct rdt_fs_context *ctx);
#endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 0805fac04401..a7453c93bad4 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2521,10 +2521,14 @@ static int rdt_get_tree(struct fs_context *fc)
goto out;
}
- ret = rdt_enable_ctx(ctx);
+ ret = rdtgroup_setup_root(ctx);
if (ret)
goto out;
+ ret = rdt_enable_ctx(ctx);
+ if (ret)
+ goto out_root;
+
ret = schemata_list_create();
if (ret) {
schemata_list_destroy();
@@ -2533,6 +2537,12 @@ static int rdt_get_tree(struct fs_context *fc)
closid_init();
+ ret = rdtgroup_add_files(rdtgroup_default.kn, RFTYPE_CTRL_BASE);
+ if (ret)
+ goto out_schemata_free;
+
+ kernfs_activate(rdtgroup_default.kn);
+
ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
if (ret < 0)
goto out_schemata_free;
@@ -2589,6 +2599,8 @@ static int rdt_get_tree(struct fs_context *fc)
schemata_list_destroy();
out_ctx:
rdt_disable_ctx(ctx);
+out_root:
+ kernfs_destroy_root(rdt_root);
out:
rdt_last_cmd_clear();
mutex_unlock(&rdtgroup_mutex);
@@ -2659,7 +2671,6 @@ static int rdt_init_fs_context(struct fs_context *fc)
if (!ctx)
return -ENOMEM;
- ctx->kfc.root = rdt_root;
ctx->kfc.magic = RDTGROUP_SUPER_MAGIC;
fc->fs_private = &ctx->kfc;
fc->ops = &rdt_fs_context_ops;
@@ -2830,6 +2841,7 @@ static void rdt_kill_sb(struct super_block *sb)
rdt_pseudo_lock_release();
rdtgroup_default.mode = RDT_MODE_SHAREABLE;
schemata_list_destroy();
+ kernfs_destroy_root(rdt_root);
static_branch_disable_cpuslocked(&rdt_alloc_enable_key);
static_branch_disable_cpuslocked(&rdt_mon_enable_key);
static_branch_disable_cpuslocked(&rdt_enable_key);
@@ -3711,10 +3723,8 @@ static struct kernfs_syscall_ops rdtgroup_kf_syscall_ops = {
.show_options = rdtgroup_show_options,
};
-static int __init rdtgroup_setup_root(void)
+int rdtgroup_setup_root(struct rdt_fs_context *ctx)
{
- int ret;
-
rdt_root = kernfs_create_root(&rdtgroup_kf_syscall_ops,
KERNFS_ROOT_CREATE_DEACTIVATED |
KERNFS_ROOT_EXTRA_OPEN_PERM_CHECK,
@@ -3722,6 +3732,15 @@ static int __init rdtgroup_setup_root(void)
if (IS_ERR(rdt_root))
return PTR_ERR(rdt_root);
+ ctx->kfc.root = rdt_root;
+
+ rdtgroup_default.kn = kernfs_root_to_node(rdt_root);
+
+ return 0;
+}
+
+static void __init rdtgroup_setup_default(void)
+{
mutex_lock(&rdtgroup_mutex);
rdtgroup_default.closid = 0;
@@ -3731,19 +3750,7 @@ static int __init rdtgroup_setup_root(void)
list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
- ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RFTYPE_CTRL_BASE);
- if (ret) {
- kernfs_destroy_root(rdt_root);
- goto out;
- }
-
- rdtgroup_default.kn = kernfs_root_to_node(rdt_root);
- kernfs_activate(rdtgroup_default.kn);
-
-out:
mutex_unlock(&rdtgroup_mutex);
-
- return ret;
}
static void domain_destroy_mon_state(struct rdt_domain *d)
@@ -3865,13 +3872,11 @@ int __init rdtgroup_init(void)
seq_buf_init(&last_cmd_status, last_cmd_status_buf,
sizeof(last_cmd_status_buf));
- ret = rdtgroup_setup_root();
- if (ret)
- return ret;
+ rdtgroup_setup_default();
ret = sysfs_create_mount_point(fs_kobj, "resctrl");
if (ret)
- goto cleanup_root;
+ return ret;
ret = register_filesystem(&rdt_fs_type);
if (ret)
@@ -3904,8 +3909,6 @@ int __init rdtgroup_init(void)
cleanup_mountpoint:
sysfs_remove_mount_point(fs_kobj, "resctrl");
-cleanup_root:
- kernfs_destroy_root(rdt_root);
return ret;
}
@@ -3915,5 +3918,4 @@ void __exit rdtgroup_exit(void)
debugfs_remove_recursive(debugfs_resctrl);
unregister_filesystem(&rdt_fs_type);
sysfs_remove_mount_point(fs_kobj, "resctrl");
- kernfs_destroy_root(rdt_root);
}
Hi Babu,
Please do note that the subject is no longer accurate.
On 8/11/2023 1:10 PM, Babu Moger wrote:
> The resctrl default control group is created during kernel init time.
> If the new files are to be added to the default group based on the mount
> option, then each file needs to be created separately and call
> kernfs_activate() again.
>
> This can be avoided if all the files are created during the mount and
> destroyed during the umount. So, move the default group creation
> in rdt_get_tree() and removal in rdt_kill_sb().
How about a slight rewording (please feel free to change):
The default resource group and its files are created during kernel
init time. Upcoming changes will make some resctrl files optional
based on a mount parameter. If optional files are to be added to the
default group based on the mount option, then each new file needs to
be created separately and call kernfs_activate() again.
Create all files of the default resource group during resctrl
mount, destroyed during unmount, to avoid scattering resctrl
file addition across two separate code flows.
>
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> arch/x86/kernel/cpu/resctrl/internal.h | 1 +
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 50 +++++++++++++++++---------------
> 2 files changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 37800724e002..2bd92c0c3b0c 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -602,5 +602,6 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
> void __init thread_throttle_mode_init(void);
> void __init mbm_config_rftype_init(const char *config);
> void rdt_staged_configs_clear(void);
> +int rdtgroup_setup_root(struct rdt_fs_context *ctx);
>
> #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 0805fac04401..a7453c93bad4 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2521,10 +2521,14 @@ static int rdt_get_tree(struct fs_context *fc)
> goto out;
> }
>
> - ret = rdt_enable_ctx(ctx);
> + ret = rdtgroup_setup_root(ctx);
> if (ret)
> goto out;
>
> + ret = rdt_enable_ctx(ctx);
> + if (ret)
> + goto out_root;
> +
> ret = schemata_list_create();
> if (ret) {
> schemata_list_destroy();
> @@ -2533,6 +2537,12 @@ static int rdt_get_tree(struct fs_context *fc)
>
> closid_init();
>
> + ret = rdtgroup_add_files(rdtgroup_default.kn, RFTYPE_CTRL_BASE);
> + if (ret)
> + goto out_schemata_free;
> +
> + kernfs_activate(rdtgroup_default.kn);
> +
> ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
> if (ret < 0)
> goto out_schemata_free;
> @@ -2589,6 +2599,8 @@ static int rdt_get_tree(struct fs_context *fc)
> schemata_list_destroy();
> out_ctx:
> rdt_disable_ctx(ctx);
> +out_root:
> + kernfs_destroy_root(rdt_root);
Please ensure that all rdtgroup_setup_root() cleanup is done
here. It seems to me that rdtgroup_default.kn can be left pointing
to freed memory. Since this cleanup is done multiple places (here
and rdt_kill_sb()) it may help to create a helper.
Apart from this the patch looks good to me.
Thank you.
Reinette
Hi Reinette,
On 8/15/23 17:50, Reinette Chatre wrote:
> Hi Babu,
>
> Please do note that the subject is no longer accurate.
Changing to
x86/resctrl: Move default group file creation during mount
>
> On 8/11/2023 1:10 PM, Babu Moger wrote:
>> The resctrl default control group is created during kernel init time.
>> If the new files are to be added to the default group based on the mount
>> option, then each file needs to be created separately and call
>> kernfs_activate() again.
>>
>> This can be avoided if all the files are created during the mount and
>> destroyed during the umount. So, move the default group creation
>> in rdt_get_tree() and removal in rdt_kill_sb().
>
> How about a slight rewording (please feel free to change):
>
> The default resource group and its files are created during kernel
> init time. Upcoming changes will make some resctrl files optional
> based on a mount parameter. If optional files are to be added to the
> default group based on the mount option, then each new file needs to
> be created separately and call kernfs_activate() again.
>
> Create all files of the default resource group during resctrl
> mount, destroyed during unmount, to avoid scattering resctrl
> file addition across two separate code flows.
Sure. Looks good. thanks
>
>
>>
>> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> arch/x86/kernel/cpu/resctrl/internal.h | 1 +
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 50 +++++++++++++++++---------------
>> 2 files changed, 27 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 37800724e002..2bd92c0c3b0c 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -602,5 +602,6 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>> void __init thread_throttle_mode_init(void);
>> void __init mbm_config_rftype_init(const char *config);
>> void rdt_staged_configs_clear(void);
>> +int rdtgroup_setup_root(struct rdt_fs_context *ctx);
>>
>> #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 0805fac04401..a7453c93bad4 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2521,10 +2521,14 @@ static int rdt_get_tree(struct fs_context *fc)
>> goto out;
>> }
>>
>> - ret = rdt_enable_ctx(ctx);
>> + ret = rdtgroup_setup_root(ctx);
>> if (ret)
>> goto out;
>>
>> + ret = rdt_enable_ctx(ctx);
>> + if (ret)
>> + goto out_root;
>> +
>> ret = schemata_list_create();
>> if (ret) {
>> schemata_list_destroy();
>> @@ -2533,6 +2537,12 @@ static int rdt_get_tree(struct fs_context *fc)
>>
>> closid_init();
>>
>> + ret = rdtgroup_add_files(rdtgroup_default.kn, RFTYPE_CTRL_BASE);
>> + if (ret)
>> + goto out_schemata_free;
>> +
>> + kernfs_activate(rdtgroup_default.kn);
>> +
>> ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
>> if (ret < 0)
>> goto out_schemata_free;
>> @@ -2589,6 +2599,8 @@ static int rdt_get_tree(struct fs_context *fc)
>> schemata_list_destroy();
>> out_ctx:
>> rdt_disable_ctx(ctx);
>> +out_root:
>> + kernfs_destroy_root(rdt_root);
>
> Please ensure that all rdtgroup_setup_root() cleanup is done
> here. It seems to me that rdtgroup_default.kn can be left pointing
> to freed memory. Since this cleanup is done multiple places (here
> and rdt_kill_sb()) it may help to create a helper.
Ok. Sure. Will do.
--
Thanks
Babu Moger
Hi Babu, On 8/16/2023 12:14 PM, Moger, Babu wrote: > Hi Reinette, > > On 8/15/23 17:50, Reinette Chatre wrote: >> Hi Babu, >> >> Please do note that the subject is no longer accurate. > > Changing to > > x86/resctrl: Move default group file creation during mount > How about: x86/resctrl: Move default group file creation to mount Reinette
© 2016 - 2025 Red Hat, Inc.