lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e30cf8a8-b58b-45fc-9df2-adcd93c1a095@redhat.com>
Date: Thu, 18 Sep 2025 10:50:50 +0200
From: David Hildenbrand <david@...hat.com>
To: Ye Liu <ye.liu@...ux.dev>, Michal Hocko <mhocko@...e.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Ye Liu <liuye@...inos.cn>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka
 <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
 Suren Baghdasaryan <surenb@...gle.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/vmstat: add header line to /proc/buddyinfo output

On 18.09.25 10:31, Ye Liu wrote:
> 
> 
> 在 2025/9/18 16:16, David Hildenbrand 写道:
>> On 18.09.25 10:11, Ye Liu wrote:
>>>
>>>
>>> 在 2025/9/18 15:29, Michal Hocko 写道:
>>>> On Thu 18-09-25 15:17:40, Ye Liu wrote:
>>>>> From: Ye Liu <liuye@...inos.cn>
>>>>>
>>>>> Add a header line to /proc/buddyinfo that shows the order numbers
>>>>> for better readability and clarity.
>>>>>
>>>>> Before:
>>>>> Node 0, zone      DMA      0      0      0      0      0      0      0 ...
>>>>> Node 0, zone    DMA32      5      8      6      6      7      5      8 ...
>>>>> Node 0, zone   Normal   1113    351    138     65     38     31     25 ...
>>>>>
>>>>> After:
>>>>> Free pages per order       0      1      2      3      4      5      6 ...
>>>>> Node 0, zone      DMA      0      0      0      0      0      0      0 ...
>>>>> Node 0, zone    DMA32      5      8      6      6      7      5      8 ...
>>>>> Node 0, zone   Normal   1113    351    138     65     38     31     25 ...
>>>>
>>>> Why is this needed? And have you considered tha this might break
>>>> existing parsers of the file?
>>>>
>>>
>>> Thanks for the review.
>>>
>>> The reason for this change is simply to make /proc/buddyinfo self-describing.
>>> Right now you have to know which column is which order; with a header it’s
>>> obvious. This is similar to what /proc/pagetypeinfo already does, e.g.:
>>>
>>>     Page block order: 9
>>>     Pages per block: 512
>>>
>>>     Free pages count per migrate type at order   0 1 2 3 ...
>>>     Node 0, zone DMA, type Unmovable             0 0 0 ...
>>>
>>> Regarding existing parsers: the patch does not change any of the existing
>>> “Node … zone …” lines, it only adds a single header line before them. Most
>>> parsers match “Node” lines and ignore everything else, so the risk should be
>>> low. If you know of any existing parser that this would break, please let
>>> me know so I can address it.
>>
>> What if there is a single one out there that has hardcoded to skip the first line only?
> 
> I understand there may be no way to be fully compatible with all existing
> parsers. However, /proc/buddyinfo is essentially raw data intended for
> human and tool consumption, and parsers are expected to be robust against
> format changes.

Let's take a look at a random one: pcp-buddyinfo.

Reading the source code [1] of the /proc/buddyinfo parser in
refresh_proc_buddyinfo() I have my doubts:


     if ((fp = linux_statsfile("/proc/buddyinfo", buf, sizeof(buf))) == NULL)
         return -oserror();

     while (fgets(buf,sizeof(buf),fp) != NULL) {
         char node_name[128];
         char *zone_name;
         unsigned int values[SPLIT_MAX];

         i = read_node_name(buf, node_name);
         i+=6; /* erase ", zone" */
         read_buddyinfo(buf+i, read_buf, MAX_ORDER+1); /* read zone name and page order */
         zone_name=read_buf[0];
         for (i=0; i < MAX_ORDER; i++)
             values[i] = strtoul(read_buf[i+1], NULL, 10);
         for (i=0; i < proc_buddyinfo->nbuddys; i++) {
             if (strcmp(proc_buddyinfo->buddys[i].node_name, node_name)==0 &&
		strcmp(proc_buddyinfo->buddys[i].zone_name, zone_name)==0)
                 break;
         }

Do you think that one would survive your changes?


https://github.com/performancecopilot/pcp/blob/main/src/pmdas/linux/proc_buddyinfo.c

-- 
Cheers

David / dhildenb


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ