Documentation/admin-guide/kernel-parameters.txt | 4 ++++ kernel/cgroup/cgroup.c | 14 +++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-)
We have a need of using favordynmods with cgroup v1, which doesn't support
changing mount flags during remount. Enabling CONFIG_FAVOR_DYNMODS at
build-time is not an option because we want to be able to selectively
enable it for certain systems.
This commit addresses this by introducing the cgroup_favordynmods=
command-line option. This option works for both cgroup v1 and v2 and
also allows for disabling favorynmods when the kernel built with
CONFIG_FAVOR_DYNMODS=y.
Signed-off-by: Luiz Capitulino <luizcap@amazon.com>
---
Documentation/admin-guide/kernel-parameters.txt | 4 ++++
kernel/cgroup/cgroup.c | 14 +++++++++++---
2 files changed, 15 insertions(+), 3 deletions(-)
o v2
- Use __ro_after_init [Waiman]
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0a1731a0f0ef..8b744d39d393 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -580,6 +580,10 @@
named mounts. Specifying both "all" and "named" disables
all v1 hierarchies.
+ cgroup_favordynmods= [KNL] Enable or Disable favordynmods.
+ Format: { "true" | "false" }
+ Defaults to the value of CONFIG_CGROUP_FAVOR_DYNMODS.
+
cgroup.memory= [KNL] Pass options to the cgroup memory controller.
Format: <string>
nosocket -- Disable socket memory accounting.
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 1fb7f562289d..2b7d74304606 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -207,6 +207,8 @@ static u16 have_exit_callback __read_mostly;
static u16 have_release_callback __read_mostly;
static u16 have_canfork_callback __read_mostly;
+static bool have_favordynmods __ro_after_init = IS_ENABLED(CONFIG_CGROUP_FAVOR_DYNMODS);
+
/* cgroup namespace for init task */
struct cgroup_namespace init_cgroup_ns = {
.ns.count = REFCOUNT_INIT(2),
@@ -2243,9 +2245,9 @@ static int cgroup_init_fs_context(struct fs_context *fc)
fc->user_ns = get_user_ns(ctx->ns->user_ns);
fc->global = true;
-#ifdef CONFIG_CGROUP_FAVOR_DYNMODS
- ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
-#endif
+ if (have_favordynmods)
+ ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
+
return 0;
}
@@ -6764,6 +6766,12 @@ static int __init enable_cgroup_debug(char *str)
}
__setup("cgroup_debug", enable_cgroup_debug);
+static int __init cgroup_favordynmods_setup(char *str)
+{
+ return (kstrtobool(str, &have_favordynmods) == 0);
+}
+__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
+
/**
* css_tryget_online_from_dir - get corresponding css from a cgroup dentry
* @dentry: directory dentry of interest
--
2.40.1On Wed, Sep 06, 2023 at 12:57:12AM +0000, Luiz Capitulino <luizcap@amazon.com> wrote: > We have a need of using favordynmods with cgroup v1, which doesn't support > changing mount flags during remount. Enabling CONFIG_FAVOR_DYNMODS at > build-time is not an option because we want to be able to selectively > enable it for certain systems. Could this be implemented by a utility that would read /proc/cmdline (while kernel ignores the arg) and remount respective hierarchies accordingly? Or what do I miss? Thanks, Michal
On 2023-09-07 11:06, Michal Koutný wrote: > On Wed, Sep 06, 2023 at 12:57:12AM +0000, Luiz Capitulino <luizcap@amazon.com> wrote: >> We have a need of using favordynmods with cgroup v1, which doesn't support >> changing mount flags during remount. Enabling CONFIG_FAVOR_DYNMODS at >> build-time is not an option because we want to be able to selectively >> enable it for certain systems. > > Could this be implemented by a utility that would read /proc/cmdline > (while kernel ignores the arg) and remount respective hierarchies > accordingly? Or what do I miss? Yeah, this works for cgroup v2 but my understanding is that cgroup v1 doesn't support changing flags in remount, take a look at cgroup1_reconfigure(). - Luiz
On Thu, Sep 07, 2023 at 11:16:41AM -0400, Luiz Capitulino <luizcap@amazon.com> wrote: > Yeah, this works for cgroup v2 but my understanding is that cgroup v1 > doesn't support changing flags in remount, take a look at > cgroup1_reconfigure(). Ah, didn't notice. Alhtough -- there seems to be a deeper issue -- the mount option doesn't have a per-root semantics. There is only a single cgroup_threadgroup_rwsem afterall. Even with your cmdline option, you may loose the behavior after unmounting any of the v1 hierarchies (cgroup_destroy_root() unconditionally disables it). Or you could still achieve the result by mounting cgroup2 hierarchy with favordynmods. (And unmount it, default root is not ever released.) Maybe it would be the best to have this controllable only via v2 hierarchy (as it's the only documented). (And maybe v1s should not show the option at all.) My 0.02€, Michal
[Resending, looks like I'm having issues with my mail server] On 2023-09-07 11:57, Michal Koutný wrote: > On Thu, Sep 07, 2023 at 11:16:41AM -0400, Luiz Capitulino <luizcap@amazon.com> wrote: >> Yeah, this works for cgroup v2 but my understanding is that cgroup v1 >> doesn't support changing flags in remount, take a look at >> cgroup1_reconfigure(). > > Ah, didn't notice. > Alhtough -- there seems to be a deeper issue -- the mount option doesn't > have a per-root semantics. There is only a single > cgroup_threadgroup_rwsem afterall. > > Even with your cmdline option, you may loose the behavior after > unmounting any of the v1 hierarchies (cgroup_destroy_root() > unconditionally disables it). Good point. I haven't checked this in detail yet, but if CONFIG_CGROUP_FAVOR_DYNMODS has the same behavior then I wouldn't worry much about this. Also, I don't know how common it is to unmount and mount a cgroup hierarchies (if it's not so common then it's even less important). We could also investigate on how to make the flag stick as a follow up work on this. > > Or you could still achieve the result by mounting cgroup2 hierarchy with > favordynmods. (And unmount it, default root is not ever released.) > > Maybe it would be the best to have this controllable only via v2 > hierarchy (as it's the only documented). > (And maybe v1s should not show the option at all.) The main motivation for this patch is really v1 since we can simply remount v2 with favordynmods enabled (although we do find this very useful for v2 as well). Another crazy idea (based on your suggestion to allow only this controllable in v2), would be to make favordynmods enabled by default in v1 w/ the rationale that new behavior changes affect only v2. - Luiz
On 9/6/23 06:27, Luiz Capitulino wrote:
[...]
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index 1fb7f562289d..2b7d74304606 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -207,6 +207,8 @@ static u16 have_exit_callback __read_mostly;
> static u16 have_release_callback __read_mostly;
> static u16 have_canfork_callback __read_mostly;
>
> +static bool have_favordynmods __ro_after_init = IS_ENABLED(CONFIG_CGROUP_FAVOR_DYNMODS);
> +
> /* cgroup namespace for init task */
> struct cgroup_namespace init_cgroup_ns = {
> .ns.count = REFCOUNT_INIT(2),
> @@ -2243,9 +2245,9 @@ static int cgroup_init_fs_context(struct fs_context *fc)
> fc->user_ns = get_user_ns(ctx->ns->user_ns);
> fc->global = true;
>
> -#ifdef CONFIG_CGROUP_FAVOR_DYNMODS
> - ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
> -#endif
> + if (have_favordynmods)
> + ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
> +
> return 0;
> }
>
> @@ -6764,6 +6766,12 @@ static int __init enable_cgroup_debug(char *str)
> }
> __setup("cgroup_debug", enable_cgroup_debug);
>
> +static int __init cgroup_favordynmods_setup(char *str)
> +{
> + return (kstrtobool(str, &have_favordynmods) == 0);
> +}
> +__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
> +
> /**
> * css_tryget_online_from_dir - get corresponding css from a cgroup dentry
> * @dentry: directory dentry of interest
Consider a case where the kernel is compiled with
CONFIG_CGROUP_FAVOR_DYNMODS=n and kernel command line is passed with
cgroup_favordynmods=true, this would set the have_favordynmods to true.
In cgroup_favordynmods_setup(), should it return 0 with a pr_warn(),
when CONFIG_CGROUP_FAVOR_DYNMODS=n in the above case, or is this
expected behavior?
--
Thanks,
Kamalesh
On 9/6/23 02:58, Kamalesh Babulal wrote:
> On 9/6/23 06:27, Luiz Capitulino wrote:
> [...]
>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>> index 1fb7f562289d..2b7d74304606 100644
>> --- a/kernel/cgroup/cgroup.c
>> +++ b/kernel/cgroup/cgroup.c
>> @@ -207,6 +207,8 @@ static u16 have_exit_callback __read_mostly;
>> static u16 have_release_callback __read_mostly;
>> static u16 have_canfork_callback __read_mostly;
>>
>> +static bool have_favordynmods __ro_after_init = IS_ENABLED(CONFIG_CGROUP_FAVOR_DYNMODS);
>> +
>> /* cgroup namespace for init task */
>> struct cgroup_namespace init_cgroup_ns = {
>> .ns.count = REFCOUNT_INIT(2),
>> @@ -2243,9 +2245,9 @@ static int cgroup_init_fs_context(struct fs_context *fc)
>> fc->user_ns = get_user_ns(ctx->ns->user_ns);
>> fc->global = true;
>>
>> -#ifdef CONFIG_CGROUP_FAVOR_DYNMODS
>> - ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>> -#endif
>> + if (have_favordynmods)
>> + ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>> +
>> return 0;
>> }
>>
>> @@ -6764,6 +6766,12 @@ static int __init enable_cgroup_debug(char *str)
>> }
>> __setup("cgroup_debug", enable_cgroup_debug);
>>
>> +static int __init cgroup_favordynmods_setup(char *str)
>> +{
>> + return (kstrtobool(str, &have_favordynmods) == 0);
>> +}
>> +__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
>> +
>> /**
>> * css_tryget_online_from_dir - get corresponding css from a cgroup dentry
>> * @dentry: directory dentry of interest
> Consider a case where the kernel is compiled with
> CONFIG_CGROUP_FAVOR_DYNMODS=n and kernel command line is passed with
> cgroup_favordynmods=true, this would set the have_favordynmods to true.
> In cgroup_favordynmods_setup(), should it return 0 with a pr_warn(),
> when CONFIG_CGROUP_FAVOR_DYNMODS=n in the above case, or is this
> expected behavior?
According to the documentation of __setup:
/*
* NOTE: __setup functions return values:
* @fn returns 1 (or non-zero) if the option argument is "handled"
* and returns 0 if the option argument is "not handled".
*/
So the return value should tell whether the input parameter is a
recognizable true or false value, not whether it is true or false.
kstrtobool returns 0 if it is a recognizable T/F value or -EINVAL
otherwise. So the check is correct. I did double check that before I
ack'ed the patch.
Cheers,
Longman
On 9/6/23 18:29, Waiman Long wrote:
> On 9/6/23 02:58, Kamalesh Babulal wrote:
>> On 9/6/23 06:27, Luiz Capitulino wrote:
>> [...]
>>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>>> index 1fb7f562289d..2b7d74304606 100644
>>> --- a/kernel/cgroup/cgroup.c
>>> +++ b/kernel/cgroup/cgroup.c
>>> @@ -207,6 +207,8 @@ static u16 have_exit_callback __read_mostly;
>>> static u16 have_release_callback __read_mostly;
>>> static u16 have_canfork_callback __read_mostly;
>>> +static bool have_favordynmods __ro_after_init = IS_ENABLED(CONFIG_CGROUP_FAVOR_DYNMODS);
>>> +
>>> /* cgroup namespace for init task */
>>> struct cgroup_namespace init_cgroup_ns = {
>>> .ns.count = REFCOUNT_INIT(2),
>>> @@ -2243,9 +2245,9 @@ static int cgroup_init_fs_context(struct fs_context *fc)
>>> fc->user_ns = get_user_ns(ctx->ns->user_ns);
>>> fc->global = true;
>>> -#ifdef CONFIG_CGROUP_FAVOR_DYNMODS
>>> - ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>>> -#endif
>>> + if (have_favordynmods)
>>> + ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>>> +
>>> return 0;
>>> }
>>> @@ -6764,6 +6766,12 @@ static int __init enable_cgroup_debug(char *str)
>>> }
>>> __setup("cgroup_debug", enable_cgroup_debug);
>>> +static int __init cgroup_favordynmods_setup(char *str)
>>> +{
>>> + return (kstrtobool(str, &have_favordynmods) == 0);
>>> +}
>>> +__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
>>> +
>>> /**
>>> * css_tryget_online_from_dir - get corresponding css from a cgroup dentry
>>> * @dentry: directory dentry of interest
>> Consider a case where the kernel is compiled with
>> CONFIG_CGROUP_FAVOR_DYNMODS=n and kernel command line is passed with
>> cgroup_favordynmods=true, this would set the have_favordynmods to true.
>> In cgroup_favordynmods_setup(), should it return 0 with a pr_warn(),
>> when CONFIG_CGROUP_FAVOR_DYNMODS=n in the above case, or is this
>> expected behavior?
>
> According to the documentation of __setup:
>
> /*
> * NOTE: __setup functions return values:
> * @fn returns 1 (or non-zero) if the option argument is "handled"
> * and returns 0 if the option argument is "not handled".
> */
>
> So the return value should tell whether the input parameter is a recognizable true or false value, not whether it is true or false. kstrtobool returns 0 if it is a recognizable T/F value or -EINVAL otherwise. So the check is correct. I did double check that before I ack'ed the patch.
>
Apologies for not being clear in the previous email. It was in two parts,
where the first one was more of a question, where if a kernel is compiled
with CONFIG_CGROUP_FAVOR_DYNMODS config option disabled and the user
attempts to pass cgroup_favordynmods=true in the kernel command line.
In this scenario, the have_favordynmods is set to true regardless of
the CONFIG_CGROUP_FAVOR_DYNMODS config option being enabled/disabled in
the kernel. This allows the user to set CGRP_ROOT_FAVOR_DYNMODS flag
without enabling the CONFIG_CGROUP_FAVOR_DYNMODS kernel config.
Shouldn't the cgroup_favordynmods kernel parameter be valid only when
the kernel is compiled with CONFIG_CGROUP_FAVOR_DYNMODS=y and allows the
user to only disable it in the kernel command line instead of allowing
them to set/unset have_favordynmods when CONFIG_CGROUP_FAVOR_DYNMODS is
disabled.
If the above assumption is right, that's where the second part was of
email, where I was suggesting the restriction by using ifdef guards in
cgroup_favordynmods_setup(), something like:
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 2b7d74304606..5c7d1a0b1dbe 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6768,7 +6768,11 @@ __setup("cgroup_debug", enable_cgroup_debug);
static int __init cgroup_favordynmods_setup(char *str)
{
+#ifdef CGROUP_FAVOR_DYNMODS
return (kstrtobool(str, &have_favordynmods) == 0);
+#endif
+ pr_warn("Favor Dynmods not supported\n");
+ return 0;
}
__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
--
Thanks,
Kamalesh
[Resending, looks like I'm having issues with my mail server]
On 2023-09-07 05:51, Kamalesh Babulal wrote:
>
> On 9/6/23 18:29, Waiman Long wrote:
>> On 9/6/23 02:58, Kamalesh Babulal wrote:
>>> On 9/6/23 06:27, Luiz Capitulino wrote:
>>> [...]
>>>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>>>> index 1fb7f562289d..2b7d74304606 100644
>>>> --- a/kernel/cgroup/cgroup.c
>>>> +++ b/kernel/cgroup/cgroup.c
>>>> @@ -207,6 +207,8 @@ static u16 have_exit_callback __read_mostly;
>>>> static u16 have_release_callback __read_mostly;
>>>> static u16 have_canfork_callback __read_mostly;
>>>> +static bool have_favordynmods __ro_after_init = IS_ENABLED(CONFIG_CGROUP_FAVOR_DYNMODS);
>>>> +
>>>> /* cgroup namespace for init task */
>>>> struct cgroup_namespace init_cgroup_ns = {
>>>> .ns.count = REFCOUNT_INIT(2),
>>>> @@ -2243,9 +2245,9 @@ static int cgroup_init_fs_context(struct fs_context *fc)
>>>> fc->user_ns = get_user_ns(ctx->ns->user_ns);
>>>> fc->global = true;
>>>> -#ifdef CONFIG_CGROUP_FAVOR_DYNMODS
>>>> - ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>>>> -#endif
>>>> + if (have_favordynmods)
>>>> + ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>>>> +
>>>> return 0;
>>>> }
>>>> @@ -6764,6 +6766,12 @@ static int __init enable_cgroup_debug(char *str)
>>>> }
>>>> __setup("cgroup_debug", enable_cgroup_debug);
>>>> +static int __init cgroup_favordynmods_setup(char *str)
>>>> +{
>>>> + return (kstrtobool(str, &have_favordynmods) == 0);
>>>> +}
>>>> +__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
>>>> +
>>>> /**
>>>> * css_tryget_online_from_dir - get corresponding css from a cgroupdentry
>>>> * @dentry: directory dentry of interest
>>> Consider a case where the kernel is compiled with
>>> CONFIG_CGROUP_FAVOR_DYNMODS=n and kernel command line is passed with
>>> cgroup_favordynmods=true, this would set the have_favordynmods to true.
>>> In cgroup_favordynmods_setup(), should it return 0 with a pr_warn(),
>>> when CONFIG_CGROUP_FAVOR_DYNMODS=n in the above case, or is this
>>> expected behavior?
>>
>> According to the documentation of __setup:
>>
>> /*
>> * NOTE: __setup functions return values:
>> * @fn returns 1 (or non-zero) if the option argument is "handled"
>> * and returns 0 if the option argument is "not handled".
>> */
>>
>> So the return value should tell whether the input parameter is a recognizable true or false value, not whether it is true or false. kstrtobool returns 0 if it is a recognizable T/F value or -EINVAL otherwise. So the check is correct. I did double check that before I ack'ed the patch.
>>
>
> Apologies for not being clear in the previous email. It was in two parts,
> where the first one was more of a question, where if a kernel is compiled
> with CONFIG_CGROUP_FAVOR_DYNMODS config option disabled and the user
> attempts to pass cgroup_favordynmods=true in the kernel command line.
>
> In this scenario, the have_favordynmods is set to true regardless of
> the CONFIG_CGROUP_FAVOR_DYNMODS config option being enabled/disabled in
> the kernel. This allows the user to set CGRP_ROOT_FAVOR_DYNMODS flag
> without enabling the CONFIG_CGROUP_FAVOR_DYNMODS kernel config.
Correct, that's exactly the goal of this patch: to give users the
option to enable/disable favordynmods at boot-time regardless of
CONFIG_FAVOR_DYNMODS.
This is especially useful with cgroup v1 where remounting with
favordynmods is not supported.
> Shouldn't the cgroup_favordynmods kernel parameter be valid only when
> the kernel is compiled with CONFIG_CGROUP_FAVOR_DYNMODS=y and allows the
> user to only disable it in the kernel command line instead of allowing
> them to set/unset have_favordynmods when CONFIG_CGROUP_FAVOR_DYNMODS is
> disabled.
This was my first idea as well, but since we'd allow for enabling why
not allow for disabling as well? Besides, the resulting code is
fairly simple.
> If the above assumption is right, that's where the second part was of
> email, where I was suggesting the restriction by using ifdef guards in
> cgroup_favordynmods_setup(), something like:
>
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index 2b7d74304606..5c7d1a0b1dbe 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -6768,7 +6768,11 @@ __setup("cgroup_debug", enable_cgroup_debug);
>
> static int __init cgroup_favordynmods_setup(char *str)
> {
> +#ifdef CGROUP_FAVOR_DYNMODS
> return (kstrtobool(str, &have_favordynmods) == 0);
> +#endif
> + pr_warn("Favor Dynmods not supported\n");
> + return 0;
> }
Why should we do this? What's the benefit for the user?
> __setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
>
> --
> Thanks,
> Kamalesh
On 9/8/23 00:17, Luiz Capitulino wrote:
[...]
>>>> Consider a case where the kernel is compiled with
>>>> CONFIG_CGROUP_FAVOR_DYNMODS=n and kernel command line is passed with
>>>> cgroup_favordynmods=true, this would set the have_favordynmods to true.
>>>> In cgroup_favordynmods_setup(), should it return 0 with a pr_warn(),
>>>> when CONFIG_CGROUP_FAVOR_DYNMODS=n in the above case, or is this
>>>> expected behavior?
>>>
>>> According to the documentation of __setup:
>>>
>>> /*
>>> * NOTE: __setup functions return values:
>>> * @fn returns 1 (or non-zero) if the option argument is "handled"
>>> * and returns 0 if the option argument is "not handled".
>>> */
>>>
>>> So the return value should tell whether the input parameter is a recognizable true or false value, not whether it is true or false. kstrtobool returns 0 if it is a recognizable T/F value or -EINVAL otherwise. So the check is correct. I did double check that before I ack'ed the patch.
>>>
>>
>> Apologies for not being clear in the previous email. It was in two parts,
>> where the first one was more of a question, where if a kernel is compiled
>> with CONFIG_CGROUP_FAVOR_DYNMODS config option disabled and the user
>> attempts to pass cgroup_favordynmods=true in the kernel command line.
>>
>> In this scenario, the have_favordynmods is set to true regardless of
>> the CONFIG_CGROUP_FAVOR_DYNMODS config option being enabled/disabled in
>> the kernel. This allows the user to set CGRP_ROOT_FAVOR_DYNMODS flag
>> without enabling the CONFIG_CGROUP_FAVOR_DYNMODS kernel config.
>
> Correct, that's exactly the goal of this patch: to give users the
> option to enable/disable favordynmods at boot-time regardless of
> CONFIG_FAVOR_DYNMODS.
>
> This is especially useful with cgroup v1 where remounting with
> favordynmods is not supported.
Thank you so much for explaining. I understand the idea of the
patch better now.
>> Shouldn't the cgroup_favordynmods kernel parameter be valid only when
>> the kernel is compiled with CONFIG_CGROUP_FAVOR_DYNMODS=y and allows the
>> user to only disable it in the kernel command line instead of allowing
>> them to set/unset have_favordynmods when CONFIG_CGROUP_FAVOR_DYNMODS is
>> disabled.
>
> This was my first idea as well, but since we'd allow for enabling why
> not allow for disabling as well? Besides, the resulting code is
> fairly simple.
Agreed, If it's independent of CONFIG_CGROUP_FAVOR_DYNMODS config
option, providing both enable and disable, is useful.
>> If the above assumption is right, that's where the second part was of
>> email, where I was suggesting the restriction by using ifdef guards in
>> cgroup_favordynmods_setup(), something like:
>>
>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>> index 2b7d74304606..5c7d1a0b1dbe 100644
>> --- a/kernel/cgroup/cgroup.c
>> +++ b/kernel/cgroup/cgroup.c
>> @@ -6768,7 +6768,11 @@ __setup("cgroup_debug", enable_cgroup_debug);
>>
>> static int __init cgroup_favordynmods_setup(char *str)
>> {
>> +#ifdef CGROUP_FAVOR_DYNMODS
>> return (kstrtobool(str, &have_favordynmods) == 0);
>> +#endif
>> + pr_warn("Favor Dynmods not supported\n");
>> + return 0;
>> }
>
> Why should we do this? What's the benefit for the user?
This code was constructed on the idea of have_favordynmods, should
be available only when the kernel is compiled with CONFIG_CGROUP_FAVOR_DYNMODS
and it's of no benefit.
>> __setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
>>
--
Thanks,
Kamalesh
On 9/5/23 20:57, Luiz Capitulino wrote:
> We have a need of using favordynmods with cgroup v1, which doesn't support
> changing mount flags during remount. Enabling CONFIG_FAVOR_DYNMODS at
> build-time is not an option because we want to be able to selectively
> enable it for certain systems.
>
> This commit addresses this by introducing the cgroup_favordynmods=
> command-line option. This option works for both cgroup v1 and v2 and
> also allows for disabling favorynmods when the kernel built with
> CONFIG_FAVOR_DYNMODS=y.
>
> Signed-off-by: Luiz Capitulino <luizcap@amazon.com>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 4 ++++
> kernel/cgroup/cgroup.c | 14 +++++++++++---
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> o v2
> - Use __ro_after_init [Waiman]
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 0a1731a0f0ef..8b744d39d393 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -580,6 +580,10 @@
> named mounts. Specifying both "all" and "named" disables
> all v1 hierarchies.
>
> + cgroup_favordynmods= [KNL] Enable or Disable favordynmods.
> + Format: { "true" | "false" }
> + Defaults to the value of CONFIG_CGROUP_FAVOR_DYNMODS.
> +
> cgroup.memory= [KNL] Pass options to the cgroup memory controller.
> Format: <string>
> nosocket -- Disable socket memory accounting.
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index 1fb7f562289d..2b7d74304606 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -207,6 +207,8 @@ static u16 have_exit_callback __read_mostly;
> static u16 have_release_callback __read_mostly;
> static u16 have_canfork_callback __read_mostly;
>
> +static bool have_favordynmods __ro_after_init = IS_ENABLED(CONFIG_CGROUP_FAVOR_DYNMODS);
> +
> /* cgroup namespace for init task */
> struct cgroup_namespace init_cgroup_ns = {
> .ns.count = REFCOUNT_INIT(2),
> @@ -2243,9 +2245,9 @@ static int cgroup_init_fs_context(struct fs_context *fc)
> fc->user_ns = get_user_ns(ctx->ns->user_ns);
> fc->global = true;
>
> -#ifdef CONFIG_CGROUP_FAVOR_DYNMODS
> - ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
> -#endif
> + if (have_favordynmods)
> + ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
> +
> return 0;
> }
>
> @@ -6764,6 +6766,12 @@ static int __init enable_cgroup_debug(char *str)
> }
> __setup("cgroup_debug", enable_cgroup_debug);
>
> +static int __init cgroup_favordynmods_setup(char *str)
> +{
> + return (kstrtobool(str, &have_favordynmods) == 0);
> +}
> +__setup("cgroup_favordynmods=", cgroup_favordynmods_setup);
> +
> /**
> * css_tryget_online_from_dir - get corresponding css from a cgroup dentry
> * @dentry: directory dentry of interest
Reviewed-by: Waiman Long <longman@redhat.com>
© 2016 - 2026 Red Hat, Inc.