If we add ``thp_anon=32,64KB:always`` to the kernel command line, we
will see the following error:

[    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting

This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
as [KMG] must follow each number to especify its unit. So, the correct
format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.

Therefore, adjust the documentation to reflect the correct format of the
parameter ``thp_anon=``.

Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 2 +-
 Documentation/admin-guide/mm/transhuge.rst      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1518343bbe22..1666576acc0e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6688,7 +6688,7 @@
 			0: no polling (default)
 
 	thp_anon=	[KNL]
-			Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
+			Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
 			state is one of "always", "madvise", "never" or "inherit".
 			Control the default behavior of the system with respect
 			to anonymous transparent hugepages.
diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
index 203ba7aaf5fc..745055c3dc09 100644
--- a/Documentation/admin-guide/mm/transhuge.rst
+++ b/Documentation/admin-guide/mm/transhuge.rst
@@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
 kernel command line.
 
 Alternatively, each supported anonymous THP size can be controlled by
-passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
+passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
 where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
 supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
 ``never`` or ``inherit``.
-- 
2.46.2

On 27.10.24 18:36, Maíra Canal wrote:
> If we add ``thp_anon=32,64KB:always`` to the kernel command line, we
> will see the following error:

^ did you mean "64K" instead of "64KB" ?

> 
> [    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting
> 
> This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
> as [KMG] must follow each number to especify its unit. So, the correct
> format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.
> 
> Therefore, adjust the documentation to reflect the correct format of the
> parameter ``thp_anon=``.
> 
> Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")

As Barry says, this is a doc fix and we should make that clearer in the 
subject. With that:

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

On Mon, Oct 28, 2024 at 1:58 AM Maíra Canal <mcanal@igalia.com> wrote:
>
> If we add ``thp_anon=32,64KB:always`` to the kernel command line, we
> will see the following error:
>
> [    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting
>
> This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
> as [KMG] must follow each number to especify its unit. So, the correct
> format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.

what if 32768,64K: always?

>
> Therefore, adjust the documentation to reflect the correct format of the
> parameter ``thp_anon=``.
>
> Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 2 +-
>  Documentation/admin-guide/mm/transhuge.rst      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1518343bbe22..1666576acc0e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6688,7 +6688,7 @@
>                         0: no polling (default)
>
>         thp_anon=       [KNL]
> -                       Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
> +                       Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
>                         state is one of "always", "madvise", "never" or "inherit".
>                         Control the default behavior of the system with respect
>                         to anonymous transparent hugepages.
> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
> index 203ba7aaf5fc..745055c3dc09 100644
> --- a/Documentation/admin-guide/mm/transhuge.rst
> +++ b/Documentation/admin-guide/mm/transhuge.rst
> @@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
>  kernel command line.
>
>  Alternatively, each supported anonymous THP size can be controlled by
> -passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
> +passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
>  where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
>  supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
>  ``never`` or ``inherit``.
> --
> 2.46.2
>

Hi Barry,

On 27/10/24 16:52, Barry Song wrote:
> On Mon, Oct 28, 2024 at 1:58 AM Maíra Canal <mcanal@igalia.com> wrote:
>>
>> If we add ``thp_anon=32,64KB:always`` to the kernel command line, we
>> will see the following error:
>>
>> [    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting
>>
>> This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
>> as [KMG] must follow each number to especify its unit. So, the correct
>> format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.
> 
> what if 32768,64K: always?

``32768,64K:always`` works. From the kernel parameters documentation, I
see that:

"Finally, the [KMG] suffix is commonly described after a number of
kernel parameter values. These ‘K’, ‘M’, and ‘G’ letters represent the
_binary_ multipliers ‘Kilo’, ‘Mega’, and ‘Giga’, equaling 2^10, 2^20,
and 2^30 bytes respectively. Such letter suffixes can also be entirely
omitted"

AFAIU this means that [KMG] can be omitted if we use bytes. But if we
don't use bytes, it cannot be omitted.

Best Regards,
- Maíra

> 
>>
>> Therefore, adjust the documentation to reflect the correct format of the
>> parameter ``thp_anon=``.
>>
>> Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   Documentation/admin-guide/kernel-parameters.txt | 2 +-
>>   Documentation/admin-guide/mm/transhuge.rst      | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 1518343bbe22..1666576acc0e 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -6688,7 +6688,7 @@
>>                          0: no polling (default)
>>
>>          thp_anon=       [KNL]
>> -                       Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
>> +                       Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
>>                          state is one of "always", "madvise", "never" or "inherit".
>>                          Control the default behavior of the system with respect
>>                          to anonymous transparent hugepages.
>> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
>> index 203ba7aaf5fc..745055c3dc09 100644
>> --- a/Documentation/admin-guide/mm/transhuge.rst
>> +++ b/Documentation/admin-guide/mm/transhuge.rst
>> @@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
>>   kernel command line.
>>
>>   Alternatively, each supported anonymous THP size can be controlled by
>> -passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
>> +passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
>>   where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
>>   supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
>>   ``never`` or ``inherit``.
>> --
>> 2.46.2
>>

On Mon, Oct 28, 2024 at 9:36 AM Maíra Canal <mcanal@igalia.com> wrote:
>
> Hi Barry,
>
> On 27/10/24 16:52, Barry Song wrote:
> > On Mon, Oct 28, 2024 at 1:58 AM Maíra Canal <mcanal@igalia.com> wrote:
> >>
> >> If we add ``thp_anon=32,64KB:always`` to the kernel command line, we
> >> will see the following error:
> >>
> >> [    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting
> >>
> >> This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
> >> as [KMG] must follow each number to especify its unit. So, the correct
> >> format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.
> >
> > what if 32768,64K: always?
>
> ``32768,64K:always`` works. From the kernel parameters documentation, I
> see that:
>
> "Finally, the [KMG] suffix is commonly described after a number of
> kernel parameter values. These ‘K’, ‘M’, and ‘G’ letters represent the
> _binary_ multipliers ‘Kilo’, ‘Mega’, and ‘Giga’, equaling 2^10, 2^20,
> and 2^30 bytes respectively. Such letter suffixes can also be entirely
> omitted"
>
> AFAIU this means that [KMG] can be omitted if we use bytes. But if we
> don't use bytes, it cannot be omitted.

Thanks! Could we change the subject of this commit to "fix the doc" without
mentioning format fixes? we are obviously only fixing the doc. With that,
please feel free to add:

Acked-by: Barry Song <baohua@kernel.org>

>
> Best Regards,
> - Maíra
>
> >
> >>
> >> Therefore, adjust the documentation to reflect the correct format of the
> >> parameter ``thp_anon=``.
> >>
> >> Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
> >> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> >> ---
> >>   Documentation/admin-guide/kernel-parameters.txt | 2 +-
> >>   Documentation/admin-guide/mm/transhuge.rst      | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index 1518343bbe22..1666576acc0e 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -6688,7 +6688,7 @@
> >>                          0: no polling (default)
> >>
> >>          thp_anon=       [KNL]
> >> -                       Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
> >> +                       Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
> >>                          state is one of "always", "madvise", "never" or "inherit".
> >>                          Control the default behavior of the system with respect
> >>                          to anonymous transparent hugepages.
> >> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
> >> index 203ba7aaf5fc..745055c3dc09 100644
> >> --- a/Documentation/admin-guide/mm/transhuge.rst
> >> +++ b/Documentation/admin-guide/mm/transhuge.rst
> >> @@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
> >>   kernel command line.
> >>
> >>   Alternatively, each supported anonymous THP size can be controlled by
> >> -passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
> >> +passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
> >>   where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
> >>   supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
> >>   ``never`` or ``inherit``.
> >> --
> >> 2.46.2
> >>
>

Thanks
barry