RE: [PATCH v2 2/2] fs: proc: Include cma info in proc/meminfo

[PINTU KUMAR]

----- Original Message -----
> From: Gioh Kim <gioh.kim@xxxxxxx>
> To: PINTU KUMAR <pintu.k@xxxxxxxxxxx>; akpm@xxxxxxxxxxxxxxxxxxxx;
riel@xxxxxxxxxx; aquini@xxxxxxxxxx; paul.gortmaker@xxxxxxxxxxxxx;
jmarchan@xxxxxxxxxx; lcapitulino@xxxxxxxxxx;
kirill.shutemov@xxxxxxxxxxxxxxx; m.szyprowski@xxxxxxxxxxx;
aneesh.kumar@xxxxxxxxxxxxxxxxxx; iamjoonsoo.kim@xxxxxxx; mina86@xxxxxxxxxx;
lauraa@xxxxxxxxxxxxxx; mgorman@xxxxxxx; rientjes@xxxxxxxxxx; hannes@cmpxchg.
org; vbabka@xxxxxxx; sasha.levin@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
linux-mm@xxxxxxxxx
> Cc: pintu_agarwal@xxxxxxxxx; cpgs@xxxxxxxxxxx; vishnu.ps@xxxxxxxxxxx;
rohit.kr@xxxxxxxxxxx; ed.savinay@xxxxxxxxxxx
> Sent: Friday, 24 October 2014 3:40 PM
> Subject: Re: [PATCH v2 2/2] fs: proc: Include cma info in proc/meminfo
> 
> 
> 
> 2014-10-24 오후 5:57, PINTU KUMAR 쓴 글:
>> Hi,
>> 
>> ----- Original Message -----
>>> From: Gioh Kim <gioh.kim@xxxxxxx>
>>> To: Pintu Kumar <pintu.k@xxxxxxxxxxx>; akpm@xxxxxxxxxxxxxxxxxxxx;
>> riel@xxxxxxxxxx; aquini@xxxxxxxxxx; paul.gortmaker@xxxxxxxxxxxxx;
>> jmarchan@xxxxxxxxxx; lcapitulino@xxxxxxxxxx;
>> kirill.shutemov@xxxxxxxxxxxxxxx; m.szyprowski@xxxxxxxxxxx;
>> aneesh.kumar@xxxxxxxxxxxxxxxxxx; iamjoonsoo.kim@xxxxxxx;
mina86@xxxxxxxxxx;
>> lauraa@xxxxxxxxxxxxxx; mgorman@xxxxxxx; rientjes@xxxxxxxxxx; 
> hannes@cmpxchg.
>> org; vbabka@xxxxxxx; sasha.levin@xxxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx;
>> linux-mm@xxxxxxxxx
>>> Cc: pintu_agarwal@xxxxxxxxx; cpgs@xxxxxxxxxxx; vishnu.ps@xxxxxxxxxxx;
>> rohit.kr@xxxxxxxxxxx; ed.savinay@xxxxxxxxxxx
>>> Sent: Thursday, 23 October 2014 5:49 AM
>>> Subject: Re: [PATCH v2 2/2] fs: proc: Include cma info in proc/meminfo
>>> 
>>> 
>>> 
>>> 2014-10-22 오후 11:06, Pintu Kumar 쓴 글:
>>>> This patch include CMA info (CMATotal, CMAFree) in /proc/meminfo.
>>>> Currently, in a CMA enabled system, if somebody wants to know the
>>>> total CMA size declared, there is no way to tell, other than the 
> dmesg
>>>> or /var/log/messages logs.
>>>> With this patch we are showing the CMA info as part of meminfo, so 
> that
>>>> it can be determined at any point of time.
>>>> This will be populated only when CMA is enabled.
>>>> 
>>>> Below is the sample output from a ARM based device with RAM:512MB 
> and
>>> CMA:16MB.
>>>> 
>>>> MemTotal:        471172 kB
>>>> MemFree:          111712 kB
>>>> MemAvailable:    271172 kB
>>>> .
>>>> .
>>>> .
>>>> CmaTotal:          16384 kB
>>>> CmaFree:            6144 kB
>>>> 
>>>> This patch also fix below checkpatch errors that were found during 
> these
>>> changes.
>>> 
>>> Why don't you split patch for it?
>>> I think there's a rule not to mix separate patchs.
>>> 
>> 
>> Last time when we submitted separate patches for checkpatch errors, it
was
>> suggested to
>> Include these kinds of fixes along with some meaningful patches together.
>> So, we included it in same patch.
>> 
>>>> 
>>>> ERROR: space required after that ',' (ctx:ExV)
>>>> 199: FILE: fs/proc/meminfo.c:199:
>>>> +      ,atomic_long_read(&num_poisoned_pages) << 
> (PAGE_SHIFT -
>>> 10)
>>>>             ^
>>>> 
>>>> ERROR: space required after that ',' (ctx:ExV)
>>>> 202: FILE: fs/proc/meminfo.c:202:
>>>> +      ,K(global_page_state(NR_ANON_TRANSPARENT_HUGEPAGES) *
>>>>             ^
>>>> 
>>>> ERROR: space required after that ',' (ctx:ExV)
>>>> 206: FILE: fs/proc/meminfo.c:206:
>>>> +      ,K(totalcma_pages)
>>>>             ^
>>>> 
>>>> total: 3 errors, 0 warnings, 2 checks, 236 lines checked
>>>> 
>>>> Signed-off-by: Pintu Kumar <pintu.k@xxxxxxxxxxx>
>>>> Signed-off-by: Vishnu Pratap Singh <vishnu.ps@xxxxxxxxxxx>
>>>> ---
>>>>     fs/proc/meminfo.c |  15 +++++++++++++--
>>>>     1 file changed, 13 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
>>>> index aa1eee0..d3ebf2e 100644
>>>> --- a/fs/proc/meminfo.c
>>>> +++ b/fs/proc/meminfo.c
>>>> @@ -12,6 +12,9 @@
>>>>     #include <linux/vmstat.h>
>>>>     #include <linux/atomic.h>
>>>>     #include <linux/vmalloc.h>
>>>> +#ifdef CONFIG_CMA
>>>> +#include <linux/cma.h>
>>>> +#endif
>>>>     #include <asm/page.h>
>>>>     #include <asm/pgtable.h>
>>>>     #include "internal.h"
>>>> @@ -138,6 +141,10 @@ static int meminfo_proc_show(struct seq_file 
> *m,
>> void
>>> *v)
>>>>     #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>>             "AnonHugePages:  %8lu kB\n"
>>>>     #endif
>>>> +#ifdef CONFIG_CMA
>>>> +        "CmaTotal:      %8lu kB\n"
>>>> +        "CmaFree:        %8lu kB\n"
>>>> +#endif
>>>>             ,
>>>>             K(i.totalram),
>>>>             K(i.freeram),
>>>> @@ -187,12 +194,16 @@ static int meminfo_proc_show(struct seq_file 
> *m,
>> void
>>> *v)
>>>>             vmi.used >> 10,
>>>>             vmi.largest_chunk >> 10
>>>>     #ifdef CONFIG_MEMORY_FAILURE
>>>> -        ,atomic_long_read(&num_poisoned_pages) << 
> (PAGE_SHIFT -
>>> 10)
>>>> +        , atomic_long_read(&num_poisoned_pages) << 
> (PAGE_SHIFT -
>>> 10)
>>>>     #endif
>>>>     #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>> -        ,K(global_page_state(NR_ANON_TRANSPARENT_HUGEPAGES) *
>>>> +        , K(global_page_state(NR_ANON_TRANSPARENT_HUGEPAGES) *
>>>>               HPAGE_PMD_NR)
>>>>     #endif
>>>> +#ifdef CONFIG_CMA
>>>> +        , K(totalcma_pages)
>>>> +        , K(global_page_state(NR_FREE_CMA_PAGES))
>>>> +#endif
>>>>             );
>>> 
>>> Just for sure, are zoneinfo and pagetypeinfo not suitable?
>>> 
>> 
>> I think zoneinfo shows only current free cma pages.
>> Same is the case with vmstat.
>> # cat /proc/zoneinfo | grep cma
>>       nr_free_cma  2560
>> # cat /proc/vmstat | grep cma
>> nr_free_cma 2560
> 
> We could add a line to show total cma pages like following and add it all.
> # cat /proc/zoneinfo | grep cma
>       nr_total_cma XXXX
>       nr_free_cma  2560
> 
> Yes. each zone can have cma area and it is annoying to add it all.
> But IMO it is rare and not difficult.
> And I think it'd better that zoneinfo has nr_total_cma line.
> 
> I think you already considered my thoughts and choosed /proc/meminfo for
a 
> certain reason.
> Why is /proc/meminfo better?
> 
We already had a plan to include CMA info in meminfo, but first we wanted
to have totalcma_pages available.
So, we developed it as second patch, if first is accepted.
We have chosen meminfo, because, we thought it's better to capture all
information at one place.
We have a tool where we capture free/cached/swap memory info, along with
CMA info.
So, reading from single proc entry it is better.
And I am sure there will be many other user space tool like that, and it
will be easy to integrate CMA info to them.
For example; we can integrate CMA info in "free" command, if required.

Also, when we saw /proc/meminfo, we saw that other similar fields are also
present here.
Such as;
SwapTotal:             0 kB
SwapFree:              0 kB
VmallocTotal:     499712 kB
VmallocUsed:       31576 kB

> Andrew already accepted it. I'm not against your idea. Just curious.
> 
>> 
>>> I don't know HOTPLUG feature so I'm just asking for sure.
>>> Does HOTPLUG not need printing message like this?
>>> 
>> 
>> Sorry, I am also not sure what hotplug feature you are referring to.
> 
> I mean "memory hotplug" feature.
> Forget it ;-)
> 
>> 
>>> Thanks a lot.
>>> 
>>> 
>>>>     
>>>>         hugetlb_report_meminfo(m);
>>>> 
>>> 
>>> --
>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
>>> see: http://www.linux-mm.org/ .
>>> Don't email: <a href=mailto:"dont@xxxxxxxxx";>
>>> email@xxxxxxxxx </a>
> 
>>> 
>> 
>> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> 
> email@xxxxxxxxx </a>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/