Re: [PATCH v2] Documentation/cgroup-v2: Update memory.{stat,numa_stat} description to reflect possible units

[Jonathan Corbet <corbet@lwn.net>]

"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> writes:

> Hi Jonathan,
>
>
> On 14/01/2025 01:49, Jonathan Corbet wrote:
>> Li Zhijian <lizhijian@fujitsu.com> writes:
>> 
>>> The description of the memory.{stat,numa_stat} file has been updated to
>>> clarify that the output values can be in bytes or pages.
>>> This change ensures that users are aware that the unit of measurement for
>>> memory values can vary and should be verified by consulting the memory.stat
>>>
>>> It's known that
>>> workingset_*, pg* are counted in pages
>>>
>>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>a
>>> ---
>>> V2: updated the document as suggestion from Michal
>>>      updated subject and commit log
>>> ---
>>>   Documentation/admin-guide/cgroup-v2.rst | 9 +++++----
>>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
>>> index 315ede811c9d..0a43be0c32d1 100644
>>> --- a/Documentation/admin-guide/cgroup-v2.rst
>>> +++ b/Documentation/admin-guide/cgroup-v2.rst
>>> @@ -1427,7 +1427,7 @@ The following nested keys are defined.
>>>   	types of memory, type-specific details, and other information
>>>   	on the state and past events of the memory management system.
>>>   
>>> -	All memory amounts are in bytes.
>>> +	All memory amounts are in bytes unless said otherwise.
>>>   
>>>   	The entries are ordered to be human readable, and new entries
>>>   	can show up in the middle. Don't rely on items remaining in a
>>> @@ -1673,11 +1673,12 @@ The following nested keys are defined.
>>>   	application performance by combining this information with the
>>>   	application's CPU allocation.
>>>   
>>> -	All memory amounts are in bytes.
>>> -
>>>   	The output format of memory.numa_stat is::
>>>   
>>> -	  type N0=<bytes in node 0> N1=<bytes in node 1> ...
>>> +	  type N0=<value for node 0> N1=<value for node 1> ...
>>> +
>>> +        The 'value' can be in bytes or pages, depending on the specific
>>> +        type of memory. To determine the unit, refer to the memory.stat.
>> 
>> This seems like useful information - but can we really not give better
>> guidance to our readers on how to interpret this value?  What in "the
>> memory.stat" will tell them which units are in use?
>
> Let me quote a piece of the numa.stat:
>
> In pages:
>>          pgdemote_khugepaged                                                   
>>                Number of pages demoted by khugepaged.
>
> In bytes:
>>          file                                                                  
>>                Amount of memory used to cache filesystem data,                 
>>                including tmpfs and shared memory. 
>
> Prior to this reference to `memory.stat`, the previous `memory.numa_stat` also
> relied on `memory.stat` to determine which entries were present.
> Therefore, adding this current reference to `memory.stat` does not introduce
> additional complexity.
>
> 1680         The 'value' can be in bytes or pages, depending on the specific
> 1681         type of memory. To determine the unit, refer to the memory.stat.
> 1682
> 1683         The entries are ordered to be human readable, and new entries
> 1684         can show up in the middle. Don't rely on items remaining in a
> 1685         fixed position; use the keys to look up specific values!
> 1686
> 1687         The entries can refer to the memory.stat.   <<< the original reference

...but neither does it help our reader.  Can we at least point to
something that would help them to make sense of this value?

>> (Even better, could we fix the code to always return the same units
>> without breaking something somewhere?)
>
> Of course, I am not opposed to having all entries use the same unit.
> At a glance, there are quite a few entries within `memory.stat` that are
> actually measured in pages. Do we truly request to this significant modification?

No, I am not asking you to do that - I was just thinking that it could
be a good idea.  But there may be reasons for why things are the way
they are, and I do not know if such a change would be accepted by the
relevant maintainers or not.

jon