Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1461492 > unrolled thread

[PACTH v2 0/3] Implement /proc/<pid>/totmaps

Started byrobert.foss@collabora.com
First post2016-08-13 00:10 +0200
Last post2016-08-19 07:20 +0200
Articles 20 on this page of 38 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [PACTH v2 0/3] Implement /proc/<pid>/totmaps robert.foss@collabora.com - 2016-08-13 00:10 +0200
    [PACTH v2 3/3] Documentation/filesystems: Added /proc/PID/totmaps documentation robert.foss@collabora.com - 2016-08-13 00:10 +0200
    Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-14 11:10 +0200
      Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Robert Foss <robert.foss@collabora.com> - 2016-08-15 15:10 +0200
        Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-15 15:50 +0200
          Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Robert Foss <robert.foss@collabora.com> - 2016-08-15 18:30 +0200
            Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-16 09:20 +0200
              Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Robert Foss <robert.foss@collabora.com> - 2016-08-16 18:50 +0200
                Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-17 10:30 +0200
                  Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Jann Horn <jann@thejh.net> - 2016-08-17 11:40 +0200
                    Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-17 15:10 +0200
                      Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Robert Foss <robert.foss@collabora.com> - 2016-08-17 18:50 +0200
                      Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-17 21:10 +0200
                        Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-18 09:50 +0200
                          Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-19 03:10 +0200
                            Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Robert Foss <robert.foss@collabora.com> - 2016-08-19 04:00 +0200
                              Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-19 08:30 +0200
                            Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Minchan Kim <minchan@kernel.org> - 2016-08-19 04:30 +0200
                              Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-19 08:50 +0200
                              Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-19 11:10 +0200
                                Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-19 20:30 +0200
                                Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Minchan Kim <minchan@kernel.org> - 2016-08-22 02:10 +0200
                                  Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-22 09:50 +0200
                                    Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Minchan Kim <minchan@kernel.org> - 2016-08-22 16:20 +0200
                                      Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Robert Foss <robert.foss@collabora.com> - 2016-08-22 16:40 +0200
                                      Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-22 18:50 +0200
                                        Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-22 19:30 +0200
                                          Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-22 19:50 +0200
                                            Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-23 10:30 +0200
                                              utime accounting regression since 4.6 (was: Re: [PACTH v2 0/3]  Implement /proc/<pid>/totmaps) Michal Hocko <mhocko@kernel.org> - 2016-08-23 16:40 +0200
                                                Re: utime accounting regression since 4.6 (was: Re: [PACTH v2 0/3]  Implement /proc/<pid>/totmaps) Rik van Riel <riel@redhat.com> - 2016-08-23 23:50 +0200
                            Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-19 08:50 +0200
                              Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-19 10:00 +0200
                                Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-19 20:00 +0200
                                  Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Michal Hocko <mhocko@kernel.org> - 2016-08-22 10:00 +0200
                                    Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-23 00:50 +0200
                                      Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Marcin Jabrzyk <m.jabrzyk@samsung.com> - 2016-08-24 12:20 +0200
                          Re: [PACTH v2 0/3] Implement /proc/<pid>/totmaps Sonny Rao <sonnyrao@chromium.org> - 2016-08-19 07:20 +0200

Page 1 of 2  [1] 2  Next page →


#1461492 — [PACTH v2 0/3] Implement /proc/<pid>/totmaps

Fromrobert.foss@collabora.com
Date2016-08-13 00:10 +0200
Subject[PACTH v2 0/3] Implement /proc/<pid>/totmaps
Message-ID<s5sOl-7GA-3@gated-at.bofh.it>
From: Robert Foss <robert.foss@collabora.com>

This series implements /proc/PID/totmaps, a tool for retrieving summarized
information about the mappings of a process.

Changes since v1:
- Removed IS_ERR check from get_task_mm() function
- Changed comment format
- Moved proc_totmaps_operations declaration inside internal.h
- Switched to using do_maps_open() in totmaps_open() function,
  which provides privilege checking
- Error handling reworked for totmaps_open() function
- Switched to stack allocated struct mem_size_stats mss_sum in
  totmaps_proc_show() function
- Removed get_task_mm() in totmaps_proc_show() since priv->mm
  already is available
- Added support to proc_map_release() fork priv==NULL, to allow
  function to be used for all failure cases
- Added proc_totmaps_op and for it helper functions
- Added documention in separate patch
- Removed totmaps_release() since it was just a wrapper for
  proc_map_release()


Robert Foss (3):
  mm, proc: Implement /proc/<pid>/totmaps
  Documentation/filesystems: Fixed typo
  Documentation/filesystems: Added /proc/PID/totmaps documentation

 Documentation/filesystems/proc.txt |  23 ++++++-
 fs/proc/base.c                     |   1 +
 fs/proc/internal.h                 |   3 +
 fs/proc/task_mmu.c                 | 134 +++++++++++++++++++++++++++++++++++++
 4 files changed, 160 insertions(+), 1 deletion(-)

-- 
2.7.4

[toc] | [next] | [standalone]


#1461495 — [PACTH v2 3/3] Documentation/filesystems: Added /proc/PID/totmaps documentation

Fromrobert.foss@collabora.com
Date2016-08-13 00:10 +0200
Subject[PACTH v2 3/3] Documentation/filesystems: Added /proc/PID/totmaps documentation
Message-ID<s5sOm-7GA-15@gated-at.bofh.it>
In reply to#1461492
From: Robert Foss <robert.foss@collabora.com>

Added documentation covering /proc/PID/totmaps.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 Documentation/filesystems/proc.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 7d001be..c06ff33 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -11,6 +11,7 @@ Version 1.3                                              Kernel version 2.2.12
 					      Kernel version 2.4.0-test11-pre4
 ------------------------------------------------------------------------------
 fixes/update part 1.1  Stefani Seibold <stefani@seibold.net>       June 9 2009
+add totmaps            Robert Foss <robert.foss@collabora.com>  August 12 2016
 
 Table of Contents
 -----------------
@@ -147,6 +148,8 @@ Table 1-1: Process specific entries in /proc
  stack		Report full stack trace, enable via CONFIG_STACKTRACE
  smaps		an extension based on maps, showing the memory consumption of
 		each mapping and flags associated with it
+ totmaps        an extenssion based on maps, showing the total memory
+                consumption of all mappings
  numa_maps	an extension based on maps, showing the memory locality and
 		binding policy as well as mem usage (in pages) of each mapping.
 ..............................................................................
@@ -512,6 +515,24 @@ be vanished or the reverse -- new added.
 This file is only present if the CONFIG_MMU kernel configuration option is
 enabled.
 
+The /proc/PID/totmaps is an extension based on maps, showing the memory
+consumption totals for all of the process's mappings. It lists the sums of the
+same statistics as /proc/PID/smaps.
+
+The process' mappings will be summarized as a series of lines like the
+following:
+
+Rss:                4256 kB
+Pss:                1170 kB
+Shared_Clean:       2720 kB
+Shared_Dirty:       1136 kB
+Private_Clean:         0 kB
+Private_Dirty:       400 kB
+Referenced:         4256 kB
+Anonymous:          1536 kB
+AnonHugePages:         0 kB
+Swap:                  0 kB
+
 The /proc/PID/clear_refs is used to reset the PG_Referenced and ACCESSED/YOUNG
 bits on both physical and virtual pages associated with a process, and the
 soft-dirty bit on pte (see Documentation/vm/soft-dirty.txt for details).
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1461737

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-14 11:10 +0200
Message-ID<s5ZAC-5TR-19@gated-at.bofh.it>
In reply to#1461492
On Fri 12-08-16 18:04:19, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> This series implements /proc/PID/totmaps, a tool for retrieving summarized
> information about the mappings of a process.

The changelog is absolutely missing the usecase. Why do we need this?
Why existing interfaces are not sufficient?

> Changes since v1:
> - Removed IS_ERR check from get_task_mm() function
> - Changed comment format
> - Moved proc_totmaps_operations declaration inside internal.h
> - Switched to using do_maps_open() in totmaps_open() function,
>   which provides privilege checking
> - Error handling reworked for totmaps_open() function
> - Switched to stack allocated struct mem_size_stats mss_sum in
>   totmaps_proc_show() function
> - Removed get_task_mm() in totmaps_proc_show() since priv->mm
>   already is available
> - Added support to proc_map_release() fork priv==NULL, to allow
>   function to be used for all failure cases
> - Added proc_totmaps_op and for it helper functions
> - Added documention in separate patch
> - Removed totmaps_release() since it was just a wrapper for
>   proc_map_release()
> 
> 
> Robert Foss (3):
>   mm, proc: Implement /proc/<pid>/totmaps
>   Documentation/filesystems: Fixed typo
>   Documentation/filesystems: Added /proc/PID/totmaps documentation
> 
>  Documentation/filesystems/proc.txt |  23 ++++++-
>  fs/proc/base.c                     |   1 +
>  fs/proc/internal.h                 |   3 +
>  fs/proc/task_mmu.c                 | 134 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 160 insertions(+), 1 deletion(-)
> 
> -- 
> 2.7.4
> 

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1462762

FromRobert Foss <robert.foss@collabora.com>
Date2016-08-15 15:10 +0200
Message-ID<s6pOq-60A-3@gated-at.bofh.it>
In reply to#1461737

On 2016-08-14 05:04 AM, Michal Hocko wrote:
> On Fri 12-08-16 18:04:19, robert.foss@collabora.com wrote:
>> From: Robert Foss <robert.foss@collabora.com>
>>
>> This series implements /proc/PID/totmaps, a tool for retrieving summarized
>> information about the mappings of a process.
>
> The changelog is absolutely missing the usecase. Why do we need this?
> Why existing interfaces are not sufficient?

You are absolutely right, more info information is in 1/3.
But the gist of it is that it provides a faster and more convenient way 
of accessing the information in /proc/PID/smaps.

>
>> Changes since v1:
>> - Removed IS_ERR check from get_task_mm() function
>> - Changed comment format
>> - Moved proc_totmaps_operations declaration inside internal.h
>> - Switched to using do_maps_open() in totmaps_open() function,
>>   which provides privilege checking
>> - Error handling reworked for totmaps_open() function
>> - Switched to stack allocated struct mem_size_stats mss_sum in
>>   totmaps_proc_show() function
>> - Removed get_task_mm() in totmaps_proc_show() since priv->mm
>>   already is available
>> - Added support to proc_map_release() fork priv==NULL, to allow
>>   function to be used for all failure cases
>> - Added proc_totmaps_op and for it helper functions
>> - Added documention in separate patch
>> - Removed totmaps_release() since it was just a wrapper for
>>   proc_map_release()
>>
>>
>> Robert Foss (3):
>>   mm, proc: Implement /proc/<pid>/totmaps
>>   Documentation/filesystems: Fixed typo
>>   Documentation/filesystems: Added /proc/PID/totmaps documentation
>>
>>  Documentation/filesystems/proc.txt |  23 ++++++-
>>  fs/proc/base.c                     |   1 +
>>  fs/proc/internal.h                 |   3 +
>>  fs/proc/task_mmu.c                 | 134 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 160 insertions(+), 1 deletion(-)
>>
>> --
>> 2.7.4
>>
>

[toc] | [prev] | [next] | [standalone]


#1462787

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-15 15:50 +0200
Message-ID<s6qr7-6fG-3@gated-at.bofh.it>
In reply to#1462762
On Mon 15-08-16 09:00:04, Robert Foss wrote:
> 
> 
> On 2016-08-14 05:04 AM, Michal Hocko wrote:
> > On Fri 12-08-16 18:04:19, robert.foss@collabora.com wrote:
> > > From: Robert Foss <robert.foss@collabora.com>
> > > 
> > > This series implements /proc/PID/totmaps, a tool for retrieving summarized
> > > information about the mappings of a process.
> > 
> > The changelog is absolutely missing the usecase. Why do we need this?
> > Why existing interfaces are not sufficient?
> 
> You are absolutely right, more info information is in 1/3.

Patch 1 is silent about the use case as well. It is usually recommended
to describe the motivation for the change in the cover letter.

> But the gist of it is that it provides a faster and more convenient way of
> accessing the information in /proc/PID/smaps.

I am sorry to insist but this is far from a description I was hoping
for. Why do we need a more convenient API? Please note that this is a
userspace API which we will have to maintain for ever. We have made many
mistakes in the past where exporting some information made sense at the
time while it turned out being a mistake only later on. So let's make
sure we will not fall into the same trap again.

So please make sure you describe the use case, why the current API is
insufficient and why it cannot be tweaked to provide the information you
are looking for.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1462978

FromRobert Foss <robert.foss@collabora.com>
Date2016-08-15 18:30 +0200
Message-ID<s6sVY-7S0-31@gated-at.bofh.it>
In reply to#1462787

On 2016-08-15 09:42 AM, Michal Hocko wrote:
> On Mon 15-08-16 09:00:04, Robert Foss wrote:
>>
>>
>> On 2016-08-14 05:04 AM, Michal Hocko wrote:
>>> On Fri 12-08-16 18:04:19, robert.foss@collabora.com wrote:
>>>> From: Robert Foss <robert.foss@collabora.com>
>>>>
>>>> This series implements /proc/PID/totmaps, a tool for retrieving summarized
>>>> information about the mappings of a process.
>>>
>>> The changelog is absolutely missing the usecase. Why do we need this?
>>> Why existing interfaces are not sufficient?
>>
>> You are absolutely right, more info information is in 1/3.
>
> Patch 1 is silent about the use case as well. It is usually recommended
> to describe the motivation for the change in the cover letter.

I'll change it for v3.

>
>> But the gist of it is that it provides a faster and more convenient way of
>> accessing the information in /proc/PID/smaps.
>
> I am sorry to insist but this is far from a description I was hoping
> for. Why do we need a more convenient API? Please note that this is a
> userspace API which we will have to maintain for ever. We have made many
> mistakes in the past where exporting some information made sense at the
> time while it turned out being a mistake only later on. So let's make
> sure we will not fall into the same trap again.
>
> So please make sure you describe the use case, why the current API is
> insufficient and why it cannot be tweaked to provide the information you
> are looking for.
>

I'll add a more elaborate description to the v3 cover letter.
In v1, there was a discussion which I think presented the practical 
applications rather well:

https://lkml.org/lkml/2016/8/9/628

or the qoute from Sonny Rao pasted below:

 > The use case is to speed up monitoring of
 > memory consumption in environments where RSS isn't precise.
 >
 > For example Chrome tends to many processes which have hundreds of VMAs
 > with a substantial amount of shared memory, and the error of using
 > RSS rather than PSS tends to be very large when looking at overall
 > memory consumption.  PSS isn't kept as a single number that's exported
 > like RSS, so to calculate PSS means having to parse a very large smaps
 > file.
 >
 > This process is slow and has to be repeated for many processes, and we
 > found that the just act of doing the parsing was taking up a
 > significant amount of CPU time, so this patch is an attempt to make
 > that process cheaper.

If a reformatted version of this still isn't adequate or desirable for 
the cover-letter, please give me another heads up.

Thanks!

[toc] | [prev] | [next] | [standalone]


#1463472

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-16 09:20 +0200
Message-ID<s6GPf-8sj-3@gated-at.bofh.it>
In reply to#1462978
On Mon 15-08-16 12:25:10, Robert Foss wrote:
> 
> 
> On 2016-08-15 09:42 AM, Michal Hocko wrote:
[...]
> > The use case is to speed up monitoring of
> > memory consumption in environments where RSS isn't precise.
> >
> > For example Chrome tends to many processes which have hundreds of VMAs
> > with a substantial amount of shared memory, and the error of using
> > RSS rather than PSS tends to be very large when looking at overall
> > memory consumption.  PSS isn't kept as a single number that's exported
> > like RSS, so to calculate PSS means having to parse a very large smaps
> > file.
> >
> > This process is slow and has to be repeated for many processes, and we
> > found that the just act of doing the parsing was taking up a
> > significant amount of CPU time, so this patch is an attempt to make
> > that process cheaper.

Well, this is slow because it requires the pte walk otherwise you cannot
know how many ptes map the particular shared page. Your patch
(totmaps_proc_show) does the very same page table walk because in fact
it is unavoidable. So what exactly is the difference except for the
userspace parsing which is quite trivial e.g. my currently running Firefox
has
$ awk '/^[0-9a-f]/{print}' /proc/4950/smaps | wc -l
984

quite some VMAs, yet parsing it spends basically all the time in the kernel...

$ /usr/bin/time -v awk '/^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}' /proc/4950/smaps 
rss:1112288 pss:1096435
        Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/4950/smaps"
        User time (seconds): 0.00
        System time (seconds): 0.02
        Percent of CPU this job got: 91%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.02

So I am not really sure I see the performance benefit.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1463935

FromRobert Foss <robert.foss@collabora.com>
Date2016-08-16 18:50 +0200
Message-ID<s6PIR-5tq-5@gated-at.bofh.it>
In reply to#1463472

On 2016-08-16 03:12 AM, Michal Hocko wrote:
> On Mon 15-08-16 12:25:10, Robert Foss wrote:
>>
>>
>> On 2016-08-15 09:42 AM, Michal Hocko wrote:
> [...]
>>> The use case is to speed up monitoring of
>>> memory consumption in environments where RSS isn't precise.
>>>
>>> For example Chrome tends to many processes which have hundreds of VMAs
>>> with a substantial amount of shared memory, and the error of using
>>> RSS rather than PSS tends to be very large when looking at overall
>>> memory consumption.  PSS isn't kept as a single number that's exported
>>> like RSS, so to calculate PSS means having to parse a very large smaps
>>> file.
>>>
>>> This process is slow and has to be repeated for many processes, and we
>>> found that the just act of doing the parsing was taking up a
>>> significant amount of CPU time, so this patch is an attempt to make
>>> that process cheaper.
>
> Well, this is slow because it requires the pte walk otherwise you cannot
> know how many ptes map the particular shared page. Your patch
> (totmaps_proc_show) does the very same page table walk because in fact
> it is unavoidable. So what exactly is the difference except for the
> userspace parsing which is quite trivial e.g. my currently running Firefox
> has
> $ awk '/^[0-9a-f]/{print}' /proc/4950/smaps | wc -l
> 984
>
> quite some VMAs, yet parsing it spends basically all the time in the kernel...
>
> $ /usr/bin/time -v awk '/^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}' /proc/4950/smaps
> rss:1112288 pss:1096435
>         Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/4950/smaps"
>         User time (seconds): 0.00
>         System time (seconds): 0.02
>         Percent of CPU this job got: 91%
>         Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.02
>
> So I am not really sure I see the performance benefit.
>

I did some performance measurements of my own, and it would seem like 
there is about a 2x performance gain to be had. To me that is 
substantial, and a larger gain than commonly seen.

There naturally also the benefit that this is a lot easier to interact 
with programmatically.

$ ps aux | grep firefox
robertfoss   5025 24.3 13.7 3562820 2219616 ?     Rl   Aug15 277:44 
/usr/lib/firefox/firefox https://allg.one/xpb
$ awk '/^[0-9a-f]/{print}' /proc/5025/smaps | wc -l
1503


$ /usr/bin/time -v -p zsh -c "(repeat 25 {cat /proc/5025/totmaps})"
[...]
	Command being timed: "zsh -c (repeat 25 {cat /proc/5025/totmaps})"
	User time (seconds): 0.00
	System time (seconds): 0.40
	Percent of CPU this job got: 90%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.45


$ /usr/bin/time -v -p zsh -c "repeat 25 { awk '/^Rss/{rss+=\$2} 
/^Pss/{pss+=\$2} END {printf \"rss:%d pss:%d\n\", rss, pss}\' 
/proc/5025/smaps }"
[...]
	Command being timed: "zsh -c repeat 25 { awk '/^Rss/{rss+=$2} 
/^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}\' 
/proc/5025/smaps }"
	User time (seconds): 0.37
	System time (seconds): 0.45
	Percent of CPU this job got: 92%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.89

[toc] | [prev] | [next] | [standalone]


#1464410

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-17 10:30 +0200
Message-ID<s74oy-6Tx-13@gated-at.bofh.it>
In reply to#1463935
On Tue 16-08-16 12:46:51, Robert Foss wrote:
[...]
> $ /usr/bin/time -v -p zsh -c "repeat 25 { awk '/^Rss/{rss+=\$2}
> /^Pss/{pss+=\$2} END {printf \"rss:%d pss:%d\n\", rss, pss}\'
> /proc/5025/smaps }"
> [...]
> 	Command being timed: "zsh -c repeat 25 { awk '/^Rss/{rss+=$2}
> /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}\' /proc/5025/smaps
> }"
> 	User time (seconds): 0.37
> 	System time (seconds): 0.45
> 	Percent of CPU this job got: 92%
> 	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.89

This is really unexpected. Where is the user time spent? Anyway, rather
than measuring some random processes I've tried to measure something
resembling the worst case. So I've created a simple program to mmap as
much as possible:

#include <sys/mman.h>
#include <sys/types.h>
#include <unistd.h>
#include <stdio.h>
int main()
{
	while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
		;

	printf("pid:%d\n", getpid());
	pause();
	return 0;
}

so depending on /proc/sys/vm/max_map_count you will get the maximum
possible mmaps. I am using a default so 65k mappings. Then I have
retried your 25x file parsing:
$ cat s.sh
#!/bin/sh

pid=$1
for i in $(seq 25)
do
	awk '/^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}' /proc/$pid/smaps
done

But I am getting different results from you:
$ awk '/^[0-9a-f]/{print}' /proc/14808/smaps | wc -l
65532
[...]
        Command being timed: "sh s.sh 14808"
        User time (seconds): 0.00
        System time (seconds): 20.10
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:20.20

The results are stable when I try multiple times, in fact there
shouldn't be any reason for them not to be. Then I went on to increase
max_map_count to 250k and that behaves consistently:
$ awk '/^[0-9a-f]/{print}' /proc/16093/smaps | wc -l     
250002
[...]
        Command being timed: "sh s.sh 16093"
        User time (seconds): 0.00
        System time (seconds): 77.93
        Percent of CPU this job got: 98%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 1:19.09

So with a reasonable user space the parsing is really not all that time
consuming wrt. smaps handling. That being said I am still very skeptical
about a dedicated proc file which accomplishes what userspace can done
in a trivial way.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1464444

FromJann Horn <jann@thejh.net>
Date2016-08-17 11:40 +0200
Message-ID<s75ui-7zL-11@gated-at.bofh.it>
In reply to#1464410

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 17, 2016 at 10:22:00AM +0200, Michal Hocko wrote:
> On Tue 16-08-16 12:46:51, Robert Foss wrote:
> [...]
> > $ /usr/bin/time -v -p zsh -c "repeat 25 { awk '/^Rss/{rss+=\$2}
> > /^Pss/{pss+=\$2} END {printf \"rss:%d pss:%d\n\", rss, pss}\'
> > /proc/5025/smaps }"
> > [...]
> > 	Command being timed: "zsh -c repeat 25 { awk '/^Rss/{rss+=$2}
> > /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}\' /proc/5025/smaps
> > }"
> > 	User time (seconds): 0.37
> > 	System time (seconds): 0.45
> > 	Percent of CPU this job got: 92%
> > 	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.89
> 
> This is really unexpected. Where is the user time spent? Anyway, rather
> than measuring some random processes I've tried to measure something
> resembling the worst case. So I've created a simple program to mmap as
> much as possible:
> 
> #include <sys/mman.h>
> #include <sys/types.h>
> #include <unistd.h>
> #include <stdio.h>
> int main()
> {
> 	while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
> 		;
> 
> 	printf("pid:%d\n", getpid());
> 	pause();
> 	return 0;
> }

Ah, nice, that's a reasonable test program. :)


> So with a reasonable user space the parsing is really not all that time
> consuming wrt. smaps handling. That being said I am still very skeptical
> about a dedicated proc file which accomplishes what userspace can done
> in a trivial way.

Now, since your numbers showed that all the time is spent in the kernel,
also create this test program to just read that file over and over again:

$ cat justreadloop.c
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sched.h>
#include <unistd.h>
#include <err.h>
#include <stdio.h>

char buf[1000000];

int main(int argc, char **argv) {
  printf("pid:%d\n", getpid());
  while (1) {
    int fd = open(argv[1], O_RDONLY);
    if (fd < 0) continue;
    if (read(fd, buf, sizeof(buf)) < 0)
      err(1, "read");
    close(fd);
  }
}
$ gcc -Wall -o justreadloop justreadloop.c
$ 

Now launch your test:

$ ./mapstuff 
pid:29397

point justreadloop at it:

$ ./justreadloop /proc/29397/smaps
pid:32567

... and then check the performance stats of justreadloop:

# perf top -p 32567

This is what I see:

Samples: 232K of event 'cycles:ppp', Event count (approx.): 60448424325
Overhead  Shared Object     Symbol
  30,43%  [kernel]          [k] format_decode
   9,12%  [kernel]          [k] number
   7,66%  [kernel]          [k] vsnprintf
   7,06%  [kernel]          [k] __lock_acquire
   3,23%  [kernel]          [k] lock_release
   2,85%  [kernel]          [k] debug_lockdep_rcu_enabled
   2,25%  [kernel]          [k] skip_atoi
   2,13%  [kernel]          [k] lock_acquire
   2,05%  [kernel]          [k] show_smap

That's at least 30.43% + 9.12% + 7.66% = 47.21% of the task's kernel
time spent on evaluating format strings. The new interface
wouldn't have to spend that much time on format strings because there
isn't so much text to format. (My kernel is built with a
bunch of debug options - the results might look very different on
distro kernels or so, so please try this yourself.)

I guess it could be argued that this is not just a problem with
smaps, but also a problem with format strings (or text-based interfaces
in general) just being slow in general.

(Here is a totally random and crazy thought: Can we put something into
the kernel build process that replaces printf calls that use simple
format strings with equivalent non-printf calls? Move the cost of
evaluating the format string to compile time?)

[toc] | [prev] | [next] | [standalone]


#1464570

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-17 15:10 +0200
Message-ID<s78Lw-1rP-31@gated-at.bofh.it>
In reply to#1464444
On Wed 17-08-16 11:31:25, Jann Horn wrote:
> On Wed, Aug 17, 2016 at 10:22:00AM +0200, Michal Hocko wrote:
> > On Tue 16-08-16 12:46:51, Robert Foss wrote:
> > [...]
> > > $ /usr/bin/time -v -p zsh -c "repeat 25 { awk '/^Rss/{rss+=\$2}
> > > /^Pss/{pss+=\$2} END {printf \"rss:%d pss:%d\n\", rss, pss}\'
> > > /proc/5025/smaps }"
> > > [...]
> > > 	Command being timed: "zsh -c repeat 25 { awk '/^Rss/{rss+=$2}
> > > /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}\' /proc/5025/smaps
> > > }"
> > > 	User time (seconds): 0.37
> > > 	System time (seconds): 0.45
> > > 	Percent of CPU this job got: 92%
> > > 	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.89
> > 
> > This is really unexpected. Where is the user time spent? Anyway, rather
> > than measuring some random processes I've tried to measure something
> > resembling the worst case. So I've created a simple program to mmap as
> > much as possible:
> > 
> > #include <sys/mman.h>
> > #include <sys/types.h>
> > #include <unistd.h>
> > #include <stdio.h>
> > int main()
> > {
> > 	while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
> > 		;
> > 
> > 	printf("pid:%d\n", getpid());
> > 	pause();
> > 	return 0;
> > }
> 
> Ah, nice, that's a reasonable test program. :)
> 
> 
> > So with a reasonable user space the parsing is really not all that time
> > consuming wrt. smaps handling. That being said I am still very skeptical
> > about a dedicated proc file which accomplishes what userspace can done
> > in a trivial way.
> 
> Now, since your numbers showed that all the time is spent in the kernel,
> also create this test program to just read that file over and over again:
> 
> $ cat justreadloop.c
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <sched.h>
> #include <unistd.h>
> #include <err.h>
> #include <stdio.h>
> 
> char buf[1000000];
> 
> int main(int argc, char **argv) {
>   printf("pid:%d\n", getpid());
>   while (1) {
>     int fd = open(argv[1], O_RDONLY);
>     if (fd < 0) continue;
>     if (read(fd, buf, sizeof(buf)) < 0)
>       err(1, "read");
>     close(fd);
>   }
> }
> $ gcc -Wall -o justreadloop justreadloop.c
> $ 
> 
> Now launch your test:
> 
> $ ./mapstuff 
> pid:29397
> 
> point justreadloop at it:
> 
> $ ./justreadloop /proc/29397/smaps
> pid:32567
> 
> ... and then check the performance stats of justreadloop:
> 
> # perf top -p 32567
> 
> This is what I see:
> 
> Samples: 232K of event 'cycles:ppp', Event count (approx.): 60448424325
> Overhead  Shared Object     Symbol
>   30,43%  [kernel]          [k] format_decode
>    9,12%  [kernel]          [k] number
>    7,66%  [kernel]          [k] vsnprintf
>    7,06%  [kernel]          [k] __lock_acquire
>    3,23%  [kernel]          [k] lock_release
>    2,85%  [kernel]          [k] debug_lockdep_rcu_enabled
>    2,25%  [kernel]          [k] skip_atoi
>    2,13%  [kernel]          [k] lock_acquire
>    2,05%  [kernel]          [k] show_smap

This is a lot! I would expect the rmap walk to consume more but it even
doesn't show up in the top consumers.
 
> That's at least 30.43% + 9.12% + 7.66% = 47.21% of the task's kernel
> time spent on evaluating format strings. The new interface
> wouldn't have to spend that much time on format strings because there
> isn't so much text to format.

well, this is true of course but I would much rather try to reduce the
overhead of smaps file than add a new file. The following should help
already. I've measured ~7% systime cut down. I guess there is still some
room for improvements but I have to say I'm far from being convinced about
a new proc file just because we suck at dumping information to the
userspace. If this was something like /proc/<pid>/stat which is
essentially read all the time then it would be a different question but
is the rss, pss going to be all that often? If yes why? These are the
questions which should be answered before we even start considering the
implementation.
---
From 2a6883a7278ff8979808cb8e2dbcefe5ea3bf672 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Wed, 17 Aug 2016 14:00:13 +0200
Subject: [PATCH] proc, smaps: reduce printing overhead

seq_printf (used by show_smap) can be pretty expensive when dumping a
lot of numbers.  Say we would like to get Rss and Pss from a particular
process.  In order to measure a pathological case let's generate as many
mappings as possible:

$ cat max_mmap.c
int main()
{
	while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
		;

	printf("pid:%d\n", getpid());
	pause();
	return 0;
}

$ awk '/^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}' /proc/$pid/smaps

would do a trick. The whole runtime is in the kernel space which is not
that that unexpected because smaps is not the cheapest one (we have to
do rmap walk etc.).

        Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/3050/smaps"
        User time (seconds): 0.01
        System time (seconds): 0.44
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.47

But the perf says:
    22.55%  awk      [kernel.kallsyms]  [k] format_decode
    14.65%  awk      [kernel.kallsyms]  [k] vsnprintf
     6.40%  awk      [kernel.kallsyms]  [k] number
     2.53%  awk      [kernel.kallsyms]  [k] shmem_mapping
     2.53%  awk      [kernel.kallsyms]  [k] show_smap
     1.81%  awk      [kernel.kallsyms]  [k] lock_acquire

we are spending most of the time actually generating the output which is
quite lame. Let's replace seq_printf by seq_puts and seq_put_decimal_ull.
This will give us:
        Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/3067/smaps"
        User time (seconds): 0.00
        System time (seconds): 0.41
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.42

which will give us ~7% improvement. Perf says:
    28.87%  awk      [kernel.kallsyms]  [k] seq_puts
     5.30%  awk      [kernel.kallsyms]  [k] vsnprintf
     4.54%  awk      [kernel.kallsyms]  [k] format_decode
     3.73%  awk      [kernel.kallsyms]  [k] show_smap
     2.56%  awk      [kernel.kallsyms]  [k] shmem_mapping
     1.92%  awk      [kernel.kallsyms]  [k] number
     1.80%  awk      [kernel.kallsyms]  [k] lock_acquire
     1.75%  awk      [kernel.kallsyms]  [k] print_name_value_kb

Reported-by: Jann Horn <jann@thejh.net>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 fs/proc/task_mmu.c | 63 ++++++++++++++++++++++--------------------------------
 1 file changed, 25 insertions(+), 38 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 187d84ef9de9..41c24c0811da 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -721,6 +721,13 @@ void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
 {
 }
 
+static void print_name_value_kb(struct seq_file *m, const char *name, unsigned long val)
+{
+	seq_puts(m, name);
+	seq_put_decimal_ull(m, 0, val);
+	seq_puts(m, " kB\n");
+}
+
 static int show_smap(struct seq_file *m, void *v, int is_pid)
 {
 	struct vm_area_struct *vma = v;
@@ -765,45 +772,25 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
 
 	show_map_vma(m, vma, is_pid);
 
-	seq_printf(m,
-		   "Size:           %8lu kB\n"
-		   "Rss:            %8lu kB\n"
-		   "Pss:            %8lu kB\n"
-		   "Shared_Clean:   %8lu kB\n"
-		   "Shared_Dirty:   %8lu kB\n"
-		   "Private_Clean:  %8lu kB\n"
-		   "Private_Dirty:  %8lu kB\n"
-		   "Referenced:     %8lu kB\n"
-		   "Anonymous:      %8lu kB\n"
-		   "AnonHugePages:  %8lu kB\n"
-		   "ShmemPmdMapped: %8lu kB\n"
-		   "Shared_Hugetlb: %8lu kB\n"
-		   "Private_Hugetlb: %7lu kB\n"
-		   "Swap:           %8lu kB\n"
-		   "SwapPss:        %8lu kB\n"
-		   "KernelPageSize: %8lu kB\n"
-		   "MMUPageSize:    %8lu kB\n"
-		   "Locked:         %8lu kB\n",
-		   (vma->vm_end - vma->vm_start) >> 10,
-		   mss.resident >> 10,
-		   (unsigned long)(mss.pss >> (10 + PSS_SHIFT)),
-		   mss.shared_clean  >> 10,
-		   mss.shared_dirty  >> 10,
-		   mss.private_clean >> 10,
-		   mss.private_dirty >> 10,
-		   mss.referenced >> 10,
-		   mss.anonymous >> 10,
-		   mss.anonymous_thp >> 10,
-		   mss.shmem_thp >> 10,
-		   mss.shared_hugetlb >> 10,
-		   mss.private_hugetlb >> 10,
-		   mss.swap >> 10,
-		   (unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)),
-		   vma_kernel_pagesize(vma) >> 10,
-		   vma_mmu_pagesize(vma) >> 10,
-		   (vma->vm_flags & VM_LOCKED) ?
+	print_name_value_kb(m, "Size:           ", (vma->vm_end - vma->vm_start) >> 10);
+	print_name_value_kb(m, "Rss:            ", mss.resident >> 10);
+	print_name_value_kb(m, "Pss:            ", (unsigned long)(mss.pss >> (10 + PSS_SHIFT)));
+	print_name_value_kb(m, "Shared_Clean:   ", mss.shared_clean  >> 10);
+	print_name_value_kb(m, "Shared_Dirty:   ", mss.shared_dirty  >> 10);
+	print_name_value_kb(m, "Private_Clean:  ", mss.private_clean >> 10);
+	print_name_value_kb(m, "Private_Dirty:  ", mss.private_dirty >> 10);
+	print_name_value_kb(m, "Referenced:     ", mss.referenced >> 10);
+	print_name_value_kb(m, "Anonymous:      ", mss.anonymous >> 10);
+	print_name_value_kb(m, "AnonHugePages:  ", mss.anonymous_thp >> 10);
+	print_name_value_kb(m, "ShmemPmdMapped: ", mss.shmem_thp >> 10);
+	print_name_value_kb(m, "Shared_Hugetlb: ", mss.shared_hugetlb >> 10);
+	print_name_value_kb(m, "Private_Hugetlb: ", mss.private_hugetlb >> 10);
+	print_name_value_kb(m, "Swap:           ", mss.swap >> 10);
+	print_name_value_kb(m, "SwapPss:        ", (unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)));
+	print_name_value_kb(m, "KernelPageSize: ", vma_kernel_pagesize(vma) >> 10);
+	print_name_value_kb(m, "MMUPageSize:    ", vma_mmu_pagesize(vma) >> 10);
+	print_name_value_kb(m, "Locked:         ", (vma->vm_flags & VM_LOCKED) ?
 			(unsigned long)(mss.pss >> (10 + PSS_SHIFT)) : 0);
-
 	arch_show_smap(m, vma);
 	show_smap_vma_flags(m, vma);
 	m_cache_vma(m, vma);
-- 
2.8.1

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1464656

FromRobert Foss <robert.foss@collabora.com>
Date2016-08-17 18:50 +0200
Message-ID<s7ccp-3AT-9@gated-at.bofh.it>
In reply to#1464570

On 2016-08-17 09:03 AM, Michal Hocko wrote:
> On Wed 17-08-16 11:31:25, Jann Horn wrote:
>> On Wed, Aug 17, 2016 at 10:22:00AM +0200, Michal Hocko wrote:
>>> On Tue 16-08-16 12:46:51, Robert Foss wrote:
>>> [...]
>>>> $ /usr/bin/time -v -p zsh -c "repeat 25 { awk '/^Rss/{rss+=\$2}
>>>> /^Pss/{pss+=\$2} END {printf \"rss:%d pss:%d\n\", rss, pss}\'
>>>> /proc/5025/smaps }"
>>>> [...]
>>>> 	Command being timed: "zsh -c repeat 25 { awk '/^Rss/{rss+=$2}
>>>> /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}\' /proc/5025/smaps
>>>> }"
>>>> 	User time (seconds): 0.37
>>>> 	System time (seconds): 0.45
>>>> 	Percent of CPU this job got: 92%
>>>> 	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.89
>>>
>>> This is really unexpected. Where is the user time spent? Anyway, rather
>>> than measuring some random processes I've tried to measure something
>>> resembling the worst case. So I've created a simple program to mmap as
>>> much as possible:
>>>
>>> #include <sys/mman.h>
>>> #include <sys/types.h>
>>> #include <unistd.h>
>>> #include <stdio.h>
>>> int main()
>>> {
>>> 	while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
>>> 		;
>>>
>>> 	printf("pid:%d\n", getpid());
>>> 	pause();
>>> 	return 0;
>>> }
>>
>> Ah, nice, that's a reasonable test program. :)
>>
>>
>>> So with a reasonable user space the parsing is really not all that time
>>> consuming wrt. smaps handling. That being said I am still very skeptical
>>> about a dedicated proc file which accomplishes what userspace can done
>>> in a trivial way.
>>
>> Now, since your numbers showed that all the time is spent in the kernel,
>> also create this test program to just read that file over and over again:
>>
>> $ cat justreadloop.c
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>> #include <sched.h>
>> #include <unistd.h>
>> #include <err.h>
>> #include <stdio.h>
>>
>> char buf[1000000];
>>
>> int main(int argc, char **argv) {
>>   printf("pid:%d\n", getpid());
>>   while (1) {
>>     int fd = open(argv[1], O_RDONLY);
>>     if (fd < 0) continue;
>>     if (read(fd, buf, sizeof(buf)) < 0)
>>       err(1, "read");
>>     close(fd);
>>   }
>> }
>> $ gcc -Wall -o justreadloop justreadloop.c
>> $
>>
>> Now launch your test:
>>
>> $ ./mapstuff
>> pid:29397
>>
>> point justreadloop at it:
>>
>> $ ./justreadloop /proc/29397/smaps
>> pid:32567
>>
>> ... and then check the performance stats of justreadloop:
>>
>> # perf top -p 32567
>>
>> This is what I see:
>>
>> Samples: 232K of event 'cycles:ppp', Event count (approx.): 60448424325
>> Overhead  Shared Object     Symbol
>>   30,43%  [kernel]          [k] format_decode
>>    9,12%  [kernel]          [k] number
>>    7,66%  [kernel]          [k] vsnprintf
>>    7,06%  [kernel]          [k] __lock_acquire
>>    3,23%  [kernel]          [k] lock_release
>>    2,85%  [kernel]          [k] debug_lockdep_rcu_enabled
>>    2,25%  [kernel]          [k] skip_atoi
>>    2,13%  [kernel]          [k] lock_acquire
>>    2,05%  [kernel]          [k] show_smap
>
> This is a lot! I would expect the rmap walk to consume more but it even
> doesn't show up in the top consumers.
>
>> That's at least 30.43% + 9.12% + 7.66% = 47.21% of the task's kernel
>> time spent on evaluating format strings. The new interface
>> wouldn't have to spend that much time on format strings because there
>> isn't so much text to format.
>
> well, this is true of course but I would much rather try to reduce the
> overhead of smaps file than add a new file. The following should help
> already. I've measured ~7% systime cut down. I guess there is still some
> room for improvements but I have to say I'm far from being convinced about
> a new proc file just because we suck at dumping information to the
> userspace. If this was something like /proc/<pid>/stat which is
> essentially read all the time then it would be a different question but
> is the rss, pss going to be all that often? If yes why? These are the
> questions which should be answered before we even start considering the
> implementation.

@Sonny Rao: Maybe you can comment on how often, for how many processes 
this information is needed and for which reasons this information is useful.

> ---
> From 2a6883a7278ff8979808cb8e2dbcefe5ea3bf672 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 17 Aug 2016 14:00:13 +0200
> Subject: [PATCH] proc, smaps: reduce printing overhead
>
> seq_printf (used by show_smap) can be pretty expensive when dumping a
> lot of numbers.  Say we would like to get Rss and Pss from a particular
> process.  In order to measure a pathological case let's generate as many
> mappings as possible:
>
> $ cat max_mmap.c
> int main()
> {
> 	while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
> 		;
>
> 	printf("pid:%d\n", getpid());
> 	pause();
> 	return 0;
> }
>
> $ awk '/^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}' /proc/$pid/smaps
>
> would do a trick. The whole runtime is in the kernel space which is not
> that that unexpected because smaps is not the cheapest one (we have to
> do rmap walk etc.).
>
>         Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/3050/smaps"
>         User time (seconds): 0.01
>         System time (seconds): 0.44
>         Percent of CPU this job got: 99%
>         Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.47
>
> But the perf says:
>     22.55%  awk      [kernel.kallsyms]  [k] format_decode
>     14.65%  awk      [kernel.kallsyms]  [k] vsnprintf
>      6.40%  awk      [kernel.kallsyms]  [k] number
>      2.53%  awk      [kernel.kallsyms]  [k] shmem_mapping
>      2.53%  awk      [kernel.kallsyms]  [k] show_smap
>      1.81%  awk      [kernel.kallsyms]  [k] lock_acquire
>
> we are spending most of the time actually generating the output which is
> quite lame. Let's replace seq_printf by seq_puts and seq_put_decimal_ull.
> This will give us:
>         Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/3067/smaps"
>         User time (seconds): 0.00
>         System time (seconds): 0.41
>         Percent of CPU this job got: 99%
>         Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.42
>
> which will give us ~7% improvement. Perf says:
>     28.87%  awk      [kernel.kallsyms]  [k] seq_puts
>      5.30%  awk      [kernel.kallsyms]  [k] vsnprintf
>      4.54%  awk      [kernel.kallsyms]  [k] format_decode
>      3.73%  awk      [kernel.kallsyms]  [k] show_smap
>      2.56%  awk      [kernel.kallsyms]  [k] shmem_mapping
>      1.92%  awk      [kernel.kallsyms]  [k] number
>      1.80%  awk      [kernel.kallsyms]  [k] lock_acquire
>      1.75%  awk      [kernel.kallsyms]  [k] print_name_value_kb
>
> Reported-by: Jann Horn <jann@thejh.net>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  fs/proc/task_mmu.c | 63 ++++++++++++++++++++++--------------------------------
>  1 file changed, 25 insertions(+), 38 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 187d84ef9de9..41c24c0811da 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -721,6 +721,13 @@ void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
>  {
>  }
>
> +static void print_name_value_kb(struct seq_file *m, const char *name, unsigned long val)
> +{
> +	seq_puts(m, name);
> +	seq_put_decimal_ull(m, 0, val);
> +	seq_puts(m, " kB\n");
> +}
> +
>  static int show_smap(struct seq_file *m, void *v, int is_pid)
>  {
>  	struct vm_area_struct *vma = v;
> @@ -765,45 +772,25 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
>
>  	show_map_vma(m, vma, is_pid);
>
> -	seq_printf(m,
> -		   "Size:           %8lu kB\n"
> -		   "Rss:            %8lu kB\n"
> -		   "Pss:            %8lu kB\n"
> -		   "Shared_Clean:   %8lu kB\n"
> -		   "Shared_Dirty:   %8lu kB\n"
> -		   "Private_Clean:  %8lu kB\n"
> -		   "Private_Dirty:  %8lu kB\n"
> -		   "Referenced:     %8lu kB\n"
> -		   "Anonymous:      %8lu kB\n"
> -		   "AnonHugePages:  %8lu kB\n"
> -		   "ShmemPmdMapped: %8lu kB\n"
> -		   "Shared_Hugetlb: %8lu kB\n"
> -		   "Private_Hugetlb: %7lu kB\n"
> -		   "Swap:           %8lu kB\n"
> -		   "SwapPss:        %8lu kB\n"
> -		   "KernelPageSize: %8lu kB\n"
> -		   "MMUPageSize:    %8lu kB\n"
> -		   "Locked:         %8lu kB\n",
> -		   (vma->vm_end - vma->vm_start) >> 10,
> -		   mss.resident >> 10,
> -		   (unsigned long)(mss.pss >> (10 + PSS_SHIFT)),
> -		   mss.shared_clean  >> 10,
> -		   mss.shared_dirty  >> 10,
> -		   mss.private_clean >> 10,
> -		   mss.private_dirty >> 10,
> -		   mss.referenced >> 10,
> -		   mss.anonymous >> 10,
> -		   mss.anonymous_thp >> 10,
> -		   mss.shmem_thp >> 10,
> -		   mss.shared_hugetlb >> 10,
> -		   mss.private_hugetlb >> 10,
> -		   mss.swap >> 10,
> -		   (unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)),
> -		   vma_kernel_pagesize(vma) >> 10,
> -		   vma_mmu_pagesize(vma) >> 10,
> -		   (vma->vm_flags & VM_LOCKED) ?
> +	print_name_value_kb(m, "Size:           ", (vma->vm_end - vma->vm_start) >> 10);
> +	print_name_value_kb(m, "Rss:            ", mss.resident >> 10);
> +	print_name_value_kb(m, "Pss:            ", (unsigned long)(mss.pss >> (10 + PSS_SHIFT)));
> +	print_name_value_kb(m, "Shared_Clean:   ", mss.shared_clean  >> 10);
> +	print_name_value_kb(m, "Shared_Dirty:   ", mss.shared_dirty  >> 10);
> +	print_name_value_kb(m, "Private_Clean:  ", mss.private_clean >> 10);
> +	print_name_value_kb(m, "Private_Dirty:  ", mss.private_dirty >> 10);
> +	print_name_value_kb(m, "Referenced:     ", mss.referenced >> 10);
> +	print_name_value_kb(m, "Anonymous:      ", mss.anonymous >> 10);
> +	print_name_value_kb(m, "AnonHugePages:  ", mss.anonymous_thp >> 10);
> +	print_name_value_kb(m, "ShmemPmdMapped: ", mss.shmem_thp >> 10);
> +	print_name_value_kb(m, "Shared_Hugetlb: ", mss.shared_hugetlb >> 10);
> +	print_name_value_kb(m, "Private_Hugetlb: ", mss.private_hugetlb >> 10);
> +	print_name_value_kb(m, "Swap:           ", mss.swap >> 10);
> +	print_name_value_kb(m, "SwapPss:        ", (unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)));
> +	print_name_value_kb(m, "KernelPageSize: ", vma_kernel_pagesize(vma) >> 10);
> +	print_name_value_kb(m, "MMUPageSize:    ", vma_mmu_pagesize(vma) >> 10);
> +	print_name_value_kb(m, "Locked:         ", (vma->vm_flags & VM_LOCKED) ?
>  			(unsigned long)(mss.pss >> (10 + PSS_SHIFT)) : 0);
> -
>  	arch_show_smap(m, vma);
>  	show_smap_vma_flags(m, vma);
>  	m_cache_vma(m, vma);
>

[toc] | [prev] | [next] | [standalone]


#1464720

FromSonny Rao <sonnyrao@chromium.org>
Date2016-08-17 21:10 +0200
Message-ID<s7enT-5cy-1@gated-at.bofh.it>
In reply to#1464570
On Wed, Aug 17, 2016 at 6:03 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Wed 17-08-16 11:31:25, Jann Horn wrote:
>> On Wed, Aug 17, 2016 at 10:22:00AM +0200, Michal Hocko wrote:
>> > On Tue 16-08-16 12:46:51, Robert Foss wrote:
>> > [...]
>> > > $ /usr/bin/time -v -p zsh -c "repeat 25 { awk '/^Rss/{rss+=\$2}
>> > > /^Pss/{pss+=\$2} END {printf \"rss:%d pss:%d\n\", rss, pss}\'
>> > > /proc/5025/smaps }"
>> > > [...]
>> > >   Command being timed: "zsh -c repeat 25 { awk '/^Rss/{rss+=$2}
>> > > /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}\' /proc/5025/smaps
>> > > }"
>> > >   User time (seconds): 0.37
>> > >   System time (seconds): 0.45
>> > >   Percent of CPU this job got: 92%
>> > >   Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.89
>> >
>> > This is really unexpected. Where is the user time spent? Anyway, rather
>> > than measuring some random processes I've tried to measure something
>> > resembling the worst case. So I've created a simple program to mmap as
>> > much as possible:
>> >
>> > #include <sys/mman.h>
>> > #include <sys/types.h>
>> > #include <unistd.h>
>> > #include <stdio.h>
>> > int main()
>> > {
>> >     while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
>> >             ;
>> >
>> >     printf("pid:%d\n", getpid());
>> >     pause();
>> >     return 0;
>> > }
>>
>> Ah, nice, that's a reasonable test program. :)
>>
>>
>> > So with a reasonable user space the parsing is really not all that time
>> > consuming wrt. smaps handling. That being said I am still very skeptical
>> > about a dedicated proc file which accomplishes what userspace can done
>> > in a trivial way.
>>
>> Now, since your numbers showed that all the time is spent in the kernel,
>> also create this test program to just read that file over and over again:
>>
>> $ cat justreadloop.c
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>> #include <sched.h>
>> #include <unistd.h>
>> #include <err.h>
>> #include <stdio.h>
>>
>> char buf[1000000];
>>
>> int main(int argc, char **argv) {
>>   printf("pid:%d\n", getpid());
>>   while (1) {
>>     int fd = open(argv[1], O_RDONLY);
>>     if (fd < 0) continue;
>>     if (read(fd, buf, sizeof(buf)) < 0)
>>       err(1, "read");
>>     close(fd);
>>   }
>> }
>> $ gcc -Wall -o justreadloop justreadloop.c
>> $
>>
>> Now launch your test:
>>
>> $ ./mapstuff
>> pid:29397
>>
>> point justreadloop at it:
>>
>> $ ./justreadloop /proc/29397/smaps
>> pid:32567
>>
>> ... and then check the performance stats of justreadloop:
>>
>> # perf top -p 32567
>>
>> This is what I see:
>>
>> Samples: 232K of event 'cycles:ppp', Event count (approx.): 60448424325
>> Overhead  Shared Object     Symbol
>>   30,43%  [kernel]          [k] format_decode
>>    9,12%  [kernel]          [k] number
>>    7,66%  [kernel]          [k] vsnprintf
>>    7,06%  [kernel]          [k] __lock_acquire
>>    3,23%  [kernel]          [k] lock_release
>>    2,85%  [kernel]          [k] debug_lockdep_rcu_enabled
>>    2,25%  [kernel]          [k] skip_atoi
>>    2,13%  [kernel]          [k] lock_acquire
>>    2,05%  [kernel]          [k] show_smap
>
> This is a lot! I would expect the rmap walk to consume more but it even
> doesn't show up in the top consumers.
>
>> That's at least 30.43% + 9.12% + 7.66% = 47.21% of the task's kernel
>> time spent on evaluating format strings. The new interface
>> wouldn't have to spend that much time on format strings because there
>> isn't so much text to format.
>
> well, this is true of course but I would much rather try to reduce the
> overhead of smaps file than add a new file. The following should help
> already. I've measured ~7% systime cut down. I guess there is still some
> room for improvements but I have to say I'm far from being convinced about
> a new proc file just because we suck at dumping information to the
> userspace.
> If this was something like /proc/<pid>/stat which is
> essentially read all the time then it would be a different question but
> is the rss, pss going to be all that often? If yes why?

If the question is why do we need to read RSS, PSS, Private_*, Swap
and the other fields so often?

I have two use cases so far involving monitoring per-process memory
usage, and we usually need to read stats for about 25 processes.

Here's a timing example on an fairly recent ARM system 4 core RK3288
running at 1.8Ghz

localhost ~ # time cat /proc/25946/smaps > /dev/null

real    0m0.036s
user    0m0.020s
sys     0m0.020s

localhost ~ # time cat /proc/25946/totmaps > /dev/null

real    0m0.027s
user    0m0.010s
sys     0m0.010s
localhost ~ #

I'll ignore the user time for now, and we see about 20 ms of system
time with smaps and 10 ms with totmaps, with 20 similar processes it
would be 400 milliseconds of cpu time for the kernel to get this
information from smaps vs 200 milliseconds with totmaps.  Even totmaps
is still pretty slow, but much better than smaps.

Use cases:
1) Basic task monitoring -- like "top" that shows memory consumption
including PSS, Private, Swap
    1 second update means about 40% of one CPU is spent in the kernel
gathering the data with smaps

2) User space OOM handling -- we'd rather do a more graceful shutdown
than let the kernel's OOM killer activate and need to gather this
information
    and we'd like to be able to get this information to make the
decision much faster than 400ms

> These are the
> questions which should be answered before we even start considering the
> implementation.
> ---
> From 2a6883a7278ff8979808cb8e2dbcefe5ea3bf672 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 17 Aug 2016 14:00:13 +0200
> Subject: [PATCH] proc, smaps: reduce printing overhead
>
> seq_printf (used by show_smap) can be pretty expensive when dumping a
> lot of numbers.  Say we would like to get Rss and Pss from a particular
> process.  In order to measure a pathological case let's generate as many
> mappings as possible:
>
> $ cat max_mmap.c
> int main()
> {
>         while (mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_SHARED|MAP_POPULATE, -1, 0) != MAP_FAILED)
>                 ;
>
>         printf("pid:%d\n", getpid());
>         pause();
>         return 0;
> }
>
> $ awk '/^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss}' /proc/$pid/smaps
>
> would do a trick. The whole runtime is in the kernel space which is not
> that that unexpected because smaps is not the cheapest one (we have to
> do rmap walk etc.).
>
>         Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/3050/smaps"
>         User time (seconds): 0.01
>         System time (seconds): 0.44
>         Percent of CPU this job got: 99%
>         Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.47
>
> But the perf says:
>     22.55%  awk      [kernel.kallsyms]  [k] format_decode
>     14.65%  awk      [kernel.kallsyms]  [k] vsnprintf
>      6.40%  awk      [kernel.kallsyms]  [k] number
>      2.53%  awk      [kernel.kallsyms]  [k] shmem_mapping
>      2.53%  awk      [kernel.kallsyms]  [k] show_smap
>      1.81%  awk      [kernel.kallsyms]  [k] lock_acquire
>
> we are spending most of the time actually generating the output which is
> quite lame. Let's replace seq_printf by seq_puts and seq_put_decimal_ull.
> This will give us:
>         Command being timed: "awk /^Rss/{rss+=$2} /^Pss/{pss+=$2} END {printf "rss:%d pss:%d\n", rss, pss} /proc/3067/smaps"
>         User time (seconds): 0.00
>         System time (seconds): 0.41
>         Percent of CPU this job got: 99%
>         Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.42
>
> which will give us ~7% improvement. Perf says:
>     28.87%  awk      [kernel.kallsyms]  [k] seq_puts
>      5.30%  awk      [kernel.kallsyms]  [k] vsnprintf
>      4.54%  awk      [kernel.kallsyms]  [k] format_decode
>      3.73%  awk      [kernel.kallsyms]  [k] show_smap
>      2.56%  awk      [kernel.kallsyms]  [k] shmem_mapping
>      1.92%  awk      [kernel.kallsyms]  [k] number
>      1.80%  awk      [kernel.kallsyms]  [k] lock_acquire
>      1.75%  awk      [kernel.kallsyms]  [k] print_name_value_kb
>
> Reported-by: Jann Horn <jann@thejh.net>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  fs/proc/task_mmu.c | 63 ++++++++++++++++++++++--------------------------------
>  1 file changed, 25 insertions(+), 38 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 187d84ef9de9..41c24c0811da 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -721,6 +721,13 @@ void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
>  {
>  }
>
> +static void print_name_value_kb(struct seq_file *m, const char *name, unsigned long val)
> +{
> +       seq_puts(m, name);
> +       seq_put_decimal_ull(m, 0, val);
> +       seq_puts(m, " kB\n");
> +}
> +
>  static int show_smap(struct seq_file *m, void *v, int is_pid)
>  {
>         struct vm_area_struct *vma = v;
> @@ -765,45 +772,25 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
>
>         show_map_vma(m, vma, is_pid);
>
> -       seq_printf(m,
> -                  "Size:           %8lu kB\n"
> -                  "Rss:            %8lu kB\n"
> -                  "Pss:            %8lu kB\n"
> -                  "Shared_Clean:   %8lu kB\n"
> -                  "Shared_Dirty:   %8lu kB\n"
> -                  "Private_Clean:  %8lu kB\n"
> -                  "Private_Dirty:  %8lu kB\n"
> -                  "Referenced:     %8lu kB\n"
> -                  "Anonymous:      %8lu kB\n"
> -                  "AnonHugePages:  %8lu kB\n"
> -                  "ShmemPmdMapped: %8lu kB\n"
> -                  "Shared_Hugetlb: %8lu kB\n"
> -                  "Private_Hugetlb: %7lu kB\n"
> -                  "Swap:           %8lu kB\n"
> -                  "SwapPss:        %8lu kB\n"
> -                  "KernelPageSize: %8lu kB\n"
> -                  "MMUPageSize:    %8lu kB\n"
> -                  "Locked:         %8lu kB\n",
> -                  (vma->vm_end - vma->vm_start) >> 10,
> -                  mss.resident >> 10,
> -                  (unsigned long)(mss.pss >> (10 + PSS_SHIFT)),
> -                  mss.shared_clean  >> 10,
> -                  mss.shared_dirty  >> 10,
> -                  mss.private_clean >> 10,
> -                  mss.private_dirty >> 10,
> -                  mss.referenced >> 10,
> -                  mss.anonymous >> 10,
> -                  mss.anonymous_thp >> 10,
> -                  mss.shmem_thp >> 10,
> -                  mss.shared_hugetlb >> 10,
> -                  mss.private_hugetlb >> 10,
> -                  mss.swap >> 10,
> -                  (unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)),
> -                  vma_kernel_pagesize(vma) >> 10,
> -                  vma_mmu_pagesize(vma) >> 10,
> -                  (vma->vm_flags & VM_LOCKED) ?
> +       print_name_value_kb(m, "Size:           ", (vma->vm_end - vma->vm_start) >> 10);
> +       print_name_value_kb(m, "Rss:            ", mss.resident >> 10);
> +       print_name_value_kb(m, "Pss:            ", (unsigned long)(mss.pss >> (10 + PSS_SHIFT)));
> +       print_name_value_kb(m, "Shared_Clean:   ", mss.shared_clean  >> 10);
> +       print_name_value_kb(m, "Shared_Dirty:   ", mss.shared_dirty  >> 10);
> +       print_name_value_kb(m, "Private_Clean:  ", mss.private_clean >> 10);
> +       print_name_value_kb(m, "Private_Dirty:  ", mss.private_dirty >> 10);
> +       print_name_value_kb(m, "Referenced:     ", mss.referenced >> 10);
> +       print_name_value_kb(m, "Anonymous:      ", mss.anonymous >> 10);
> +       print_name_value_kb(m, "AnonHugePages:  ", mss.anonymous_thp >> 10);
> +       print_name_value_kb(m, "ShmemPmdMapped: ", mss.shmem_thp >> 10);
> +       print_name_value_kb(m, "Shared_Hugetlb: ", mss.shared_hugetlb >> 10);
> +       print_name_value_kb(m, "Private_Hugetlb: ", mss.private_hugetlb >> 10);
> +       print_name_value_kb(m, "Swap:           ", mss.swap >> 10);
> +       print_name_value_kb(m, "SwapPss:        ", (unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)));
> +       print_name_value_kb(m, "KernelPageSize: ", vma_kernel_pagesize(vma) >> 10);
> +       print_name_value_kb(m, "MMUPageSize:    ", vma_mmu_pagesize(vma) >> 10);
> +       print_name_value_kb(m, "Locked:         ", (vma->vm_flags & VM_LOCKED) ?
>                         (unsigned long)(mss.pss >> (10 + PSS_SHIFT)) : 0);
> -
>         arch_show_smap(m, vma);
>         show_smap_vma_flags(m, vma);
>         m_cache_vma(m, vma);
> --
> 2.8.1
>
> --
> Michal Hocko
> SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1464982

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-18 09:50 +0200
Message-ID<s7qfo-55N-23@gated-at.bofh.it>
In reply to#1464720
On Wed 17-08-16 11:57:56, Sonny Rao wrote:
> On Wed, Aug 17, 2016 at 6:03 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Wed 17-08-16 11:31:25, Jann Horn wrote:
[...]
> >> That's at least 30.43% + 9.12% + 7.66% = 47.21% of the task's kernel
> >> time spent on evaluating format strings. The new interface
> >> wouldn't have to spend that much time on format strings because there
> >> isn't so much text to format.
> >
> > well, this is true of course but I would much rather try to reduce the
> > overhead of smaps file than add a new file. The following should help
> > already. I've measured ~7% systime cut down. I guess there is still some
> > room for improvements but I have to say I'm far from being convinced about
> > a new proc file just because we suck at dumping information to the
> > userspace.
> > If this was something like /proc/<pid>/stat which is
> > essentially read all the time then it would be a different question but
> > is the rss, pss going to be all that often? If yes why?
> 
> If the question is why do we need to read RSS, PSS, Private_*, Swap
> and the other fields so often?
> 
> I have two use cases so far involving monitoring per-process memory
> usage, and we usually need to read stats for about 25 processes.
> 
> Here's a timing example on an fairly recent ARM system 4 core RK3288
> running at 1.8Ghz
> 
> localhost ~ # time cat /proc/25946/smaps > /dev/null
> 
> real    0m0.036s
> user    0m0.020s
> sys     0m0.020s
> 
> localhost ~ # time cat /proc/25946/totmaps > /dev/null
> 
> real    0m0.027s
> user    0m0.010s
> sys     0m0.010s
> localhost ~ #
> 
> I'll ignore the user time for now, and we see about 20 ms of system
> time with smaps and 10 ms with totmaps, with 20 similar processes it
> would be 400 milliseconds of cpu time for the kernel to get this
> information from smaps vs 200 milliseconds with totmaps.  Even totmaps
> is still pretty slow, but much better than smaps.
> 
> Use cases:
> 1) Basic task monitoring -- like "top" that shows memory consumption
> including PSS, Private, Swap
>     1 second update means about 40% of one CPU is spent in the kernel
> gathering the data with smaps

I would argue that even 20% is way too much for such a monitoring. What
is the value to do it so often tha 20 vs 40ms really matters?

> 2) User space OOM handling -- we'd rather do a more graceful shutdown
> than let the kernel's OOM killer activate and need to gather this
> information and we'd like to be able to get this information to make
> the decision much faster than 400ms

Global OOM handling in userspace is really dubious if you ask me. I
understand you want something better than SIGKILL and in fact this is
already possible with memory cgroup controller (btw. memcg will give
you a cheap access to rss, amount of shared, swapped out memory as
well). Anyway if you are getting close to the OOM your system will most
probably be really busy and chances are that also reading your new file
will take much more time. I am also not quite sure how is pss useful for
oom decisions.

Don't take me wrong, /proc/<pid>/totmaps might be suitable for your
specific usecase but so far I haven't heard any sound argument for it to
be generally usable. It is true that smaps is unnecessarily costly but
at least I can see some room for improvements. A simple patch I've
posted cut the formatting overhead by 7%. Maybe we can do more.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1465723

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-19 03:10 +0200
Message-ID<s7GtR-7go-75@gated-at.bofh.it>
In reply to#1464982
On Thu 18-08-16 10:47:57, Sonny Rao wrote:
> On Thu, Aug 18, 2016 at 12:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Wed 17-08-16 11:57:56, Sonny Rao wrote:
[...]
> >> 2) User space OOM handling -- we'd rather do a more graceful shutdown
> >> than let the kernel's OOM killer activate and need to gather this
> >> information and we'd like to be able to get this information to make
> >> the decision much faster than 400ms
> >
> > Global OOM handling in userspace is really dubious if you ask me. I
> > understand you want something better than SIGKILL and in fact this is
> > already possible with memory cgroup controller (btw. memcg will give
> > you a cheap access to rss, amount of shared, swapped out memory as
> > well). Anyway if you are getting close to the OOM your system will most
> > probably be really busy and chances are that also reading your new file
> > will take much more time. I am also not quite sure how is pss useful for
> > oom decisions.
> 
> I mentioned it before, but based on experience RSS just isn't good
> enough -- there's too much sharing going on in our use case to make
> the correct decision based on RSS.  If RSS were good enough, simply
> put, this patch wouldn't exist.

But that doesn't answer my question, I am afraid. So how exactly do you
use pss for oom decisions?

> So even with memcg I think we'd have the same problem?

memcg will give you instant anon, shared counters for all processes in
the memcg.

> > Don't take me wrong, /proc/<pid>/totmaps might be suitable for your
> > specific usecase but so far I haven't heard any sound argument for it to
> > be generally usable. It is true that smaps is unnecessarily costly but
> > at least I can see some room for improvements. A simple patch I've
> > posted cut the formatting overhead by 7%. Maybe we can do more.
> 
> It seems like a general problem that if you want these values the
> existing kernel interface can be very expensive, so it would be
> generally usable by any application which wants a per process PSS,
> private data, dirty data or swap value.

yes this is really unfortunate. And if at all possible we should address
that. Precise values require the expensive rmap walk. We can introduce
some caching to help that. But so far it seems the biggest overhead is
to simply format the output and that should be addressed before any new
proc file is added.

> I mentioned two use cases, but I guess I don't understand the comment
> about why it's not usable by other use cases.

I might be wrong here but a use of pss is quite limited and I do not
remember anybody asking for large optimizations in that area. I still do
not understand your use cases properly so I am quite skeptical about a
general usefulness of a new file.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1465870

FromRobert Foss <robert.foss@collabora.com>
Date2016-08-19 04:00 +0200
Message-ID<s7Hge-7z4-35@gated-at.bofh.it>
In reply to#1465723

On 2016-08-18 02:01 PM, Michal Hocko wrote:
> On Thu 18-08-16 10:47:57, Sonny Rao wrote:
>> On Thu, Aug 18, 2016 at 12:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
>>> On Wed 17-08-16 11:57:56, Sonny Rao wrote:
> [...]
>>>> 2) User space OOM handling -- we'd rather do a more graceful shutdown
>>>> than let the kernel's OOM killer activate and need to gather this
>>>> information and we'd like to be able to get this information to make
>>>> the decision much faster than 400ms
>>>
>>> Global OOM handling in userspace is really dubious if you ask me. I
>>> understand you want something better than SIGKILL and in fact this is
>>> already possible with memory cgroup controller (btw. memcg will give
>>> you a cheap access to rss, amount of shared, swapped out memory as
>>> well). Anyway if you are getting close to the OOM your system will most
>>> probably be really busy and chances are that also reading your new file
>>> will take much more time. I am also not quite sure how is pss useful for
>>> oom decisions.
>>
>> I mentioned it before, but based on experience RSS just isn't good
>> enough -- there's too much sharing going on in our use case to make
>> the correct decision based on RSS.  If RSS were good enough, simply
>> put, this patch wouldn't exist.
>
> But that doesn't answer my question, I am afraid. So how exactly do you
> use pss for oom decisions?
>
>> So even with memcg I think we'd have the same problem?
>
> memcg will give you instant anon, shared counters for all processes in
> the memcg.

Is it technically feasible to add instant pss support to memcg?

@Sonny Rao: Would using cgroups be acceptable for chromiumos?

>
>>> Don't take me wrong, /proc/<pid>/totmaps might be suitable for your
>>> specific usecase but so far I haven't heard any sound argument for it to
>>> be generally usable. It is true that smaps is unnecessarily costly but
>>> at least I can see some room for improvements. A simple patch I've
>>> posted cut the formatting overhead by 7%. Maybe we can do more.
>>
>> It seems like a general problem that if you want these values the
>> existing kernel interface can be very expensive, so it would be
>> generally usable by any application which wants a per process PSS,
>> private data, dirty data or swap value.
>
> yes this is really unfortunate. And if at all possible we should address
> that. Precise values require the expensive rmap walk. We can introduce
> some caching to help that. But so far it seems the biggest overhead is
> to simply format the output and that should be addressed before any new
> proc file is added.
>
>> I mentioned two use cases, but I guess I don't understand the comment
>> about why it's not usable by other use cases.
>
> I might be wrong here but a use of pss is quite limited and I do not
> remember anybody asking for large optimizations in that area. I still do
> not understand your use cases properly so I am quite skeptical about a
> general usefulness of a new file.
>

[toc] | [prev] | [next] | [standalone]


#1466046

FromSonny Rao <sonnyrao@chromium.org>
Date2016-08-19 08:30 +0200
Message-ID<s7Ltv-202-5@gated-at.bofh.it>
In reply to#1465870
On Thu, Aug 18, 2016 at 2:05 PM, Robert Foss <robert.foss@collabora.com> wrote:
>
>
> On 2016-08-18 02:01 PM, Michal Hocko wrote:
>>
>> On Thu 18-08-16 10:47:57, Sonny Rao wrote:
>>>
>>> On Thu, Aug 18, 2016 at 12:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
>>>>
>>>> On Wed 17-08-16 11:57:56, Sonny Rao wrote:
>>
>> [...]
>>>>>
>>>>> 2) User space OOM handling -- we'd rather do a more graceful shutdown
>>>>> than let the kernel's OOM killer activate and need to gather this
>>>>> information and we'd like to be able to get this information to make
>>>>> the decision much faster than 400ms
>>>>
>>>>
>>>> Global OOM handling in userspace is really dubious if you ask me. I
>>>> understand you want something better than SIGKILL and in fact this is
>>>> already possible with memory cgroup controller (btw. memcg will give
>>>> you a cheap access to rss, amount of shared, swapped out memory as
>>>> well). Anyway if you are getting close to the OOM your system will most
>>>> probably be really busy and chances are that also reading your new file
>>>> will take much more time. I am also not quite sure how is pss useful for
>>>> oom decisions.
>>>
>>>
>>> I mentioned it before, but based on experience RSS just isn't good
>>> enough -- there's too much sharing going on in our use case to make
>>> the correct decision based on RSS.  If RSS were good enough, simply
>>> put, this patch wouldn't exist.
>>
>>
>> But that doesn't answer my question, I am afraid. So how exactly do you
>> use pss for oom decisions?
>>
>>> So even with memcg I think we'd have the same problem?
>>
>>
>> memcg will give you instant anon, shared counters for all processes in
>> the memcg.
>
>
> Is it technically feasible to add instant pss support to memcg?
>
> @Sonny Rao: Would using cgroups be acceptable for chromiumos?

It's possible, though I think we'd end up putting each renderer in
it's own cgroup to get the PSS stat, so it seems a bit like overkill.
I think memcg also has some overhead that we'd need to quantify but I
could be mistaken about this.

>
>
>>
>>>> Don't take me wrong, /proc/<pid>/totmaps might be suitable for your
>>>> specific usecase but so far I haven't heard any sound argument for it to
>>>> be generally usable. It is true that smaps is unnecessarily costly but
>>>> at least I can see some room for improvements. A simple patch I've
>>>> posted cut the formatting overhead by 7%. Maybe we can do more.
>>>
>>>
>>> It seems like a general problem that if you want these values the
>>> existing kernel interface can be very expensive, so it would be
>>> generally usable by any application which wants a per process PSS,
>>> private data, dirty data or swap value.
>>
>>
>> yes this is really unfortunate. And if at all possible we should address
>> that. Precise values require the expensive rmap walk. We can introduce
>> some caching to help that. But so far it seems the biggest overhead is
>> to simply format the output and that should be addressed before any new
>> proc file is added.
>>
>>> I mentioned two use cases, but I guess I don't understand the comment
>>> about why it's not usable by other use cases.
>>
>>
>> I might be wrong here but a use of pss is quite limited and I do not
>> remember anybody asking for large optimizations in that area. I still do
>> not understand your use cases properly so I am quite skeptical about a
>> general usefulness of a new file.
>>
>

[toc] | [prev] | [next] | [standalone]


#1465907

FromMinchan Kim <minchan@kernel.org>
Date2016-08-19 04:30 +0200
Message-ID<s7HJf-84B-7@gated-at.bofh.it>
In reply to#1465723
Hi Michal,

On Thu, Aug 18, 2016 at 08:01:04PM +0200, Michal Hocko wrote:
> On Thu 18-08-16 10:47:57, Sonny Rao wrote:
> > On Thu, Aug 18, 2016 at 12:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Wed 17-08-16 11:57:56, Sonny Rao wrote:
> [...]
> > >> 2) User space OOM handling -- we'd rather do a more graceful shutdown
> > >> than let the kernel's OOM killer activate and need to gather this
> > >> information and we'd like to be able to get this information to make
> > >> the decision much faster than 400ms
> > >
> > > Global OOM handling in userspace is really dubious if you ask me. I
> > > understand you want something better than SIGKILL and in fact this is
> > > already possible with memory cgroup controller (btw. memcg will give
> > > you a cheap access to rss, amount of shared, swapped out memory as
> > > well). Anyway if you are getting close to the OOM your system will most
> > > probably be really busy and chances are that also reading your new file
> > > will take much more time. I am also not quite sure how is pss useful for
> > > oom decisions.
> > 
> > I mentioned it before, but based on experience RSS just isn't good
> > enough -- there's too much sharing going on in our use case to make
> > the correct decision based on RSS.  If RSS were good enough, simply
> > put, this patch wouldn't exist.
> 
> But that doesn't answer my question, I am afraid. So how exactly do you
> use pss for oom decisions?

My case is not for OOM decision but I agree it would be great if we can get
*fast* smap summary information.

PSS is really great tool to figure out how processes consume memory
more exactly rather than RSS. We have been used it for monitoring
of memory for per-process. Although it is not used for OOM decision,
it would be great if it is speed up because we don't want to spend
many CPU time for just monitoring.

For our usecase, we don't need AnonHugePages, ShmemPmdMapped, Shared_Hugetlb,
Private_Hugetlb, KernelPageSize, MMUPageSize because we never enable THP and
hugetlb. Additionally, Locked can be known via vma flags so we don't need it,
either. Even, we don't need address range for just monitoring when we don't
investigate in detail.

Although they are not severe overhead, why does it emit the useless
information? Even bloat day by day. :( With that, userspace tools should
spend more time to parse which is pointless.

Having said that, I'm not fan of creating new stat knob for that, either.
How about appending summary information in the end of smap?
So, monitoring users can just open the file and lseek to the (end - 1) and
read the summary only.

Thanks.

[toc] | [prev] | [next] | [standalone]


#1466061

FromSonny Rao <sonnyrao@chromium.org>
Date2016-08-19 08:50 +0200
Message-ID<s7LMR-27O-15@gated-at.bofh.it>
In reply to#1465907
On Thu, Aug 18, 2016 at 7:26 PM, Minchan Kim <minchan@kernel.org> wrote:
> Hi Michal,
>
> On Thu, Aug 18, 2016 at 08:01:04PM +0200, Michal Hocko wrote:
>> On Thu 18-08-16 10:47:57, Sonny Rao wrote:
>> > On Thu, Aug 18, 2016 at 12:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > > On Wed 17-08-16 11:57:56, Sonny Rao wrote:
>> [...]
>> > >> 2) User space OOM handling -- we'd rather do a more graceful shutdown
>> > >> than let the kernel's OOM killer activate and need to gather this
>> > >> information and we'd like to be able to get this information to make
>> > >> the decision much faster than 400ms
>> > >
>> > > Global OOM handling in userspace is really dubious if you ask me. I
>> > > understand you want something better than SIGKILL and in fact this is
>> > > already possible with memory cgroup controller (btw. memcg will give
>> > > you a cheap access to rss, amount of shared, swapped out memory as
>> > > well). Anyway if you are getting close to the OOM your system will most
>> > > probably be really busy and chances are that also reading your new file
>> > > will take much more time. I am also not quite sure how is pss useful for
>> > > oom decisions.
>> >
>> > I mentioned it before, but based on experience RSS just isn't good
>> > enough -- there's too much sharing going on in our use case to make
>> > the correct decision based on RSS.  If RSS were good enough, simply
>> > put, this patch wouldn't exist.
>>
>> But that doesn't answer my question, I am afraid. So how exactly do you
>> use pss for oom decisions?
>
> My case is not for OOM decision but I agree it would be great if we can get
> *fast* smap summary information.
>
> PSS is really great tool to figure out how processes consume memory
> more exactly rather than RSS. We have been used it for monitoring
> of memory for per-process. Although it is not used for OOM decision,
> it would be great if it is speed up because we don't want to spend
> many CPU time for just monitoring.
>
> For our usecase, we don't need AnonHugePages, ShmemPmdMapped, Shared_Hugetlb,
> Private_Hugetlb, KernelPageSize, MMUPageSize because we never enable THP and
> hugetlb. Additionally, Locked can be known via vma flags so we don't need it,
> either. Even, we don't need address range for just monitoring when we don't
> investigate in detail.
>
> Although they are not severe overhead, why does it emit the useless
> information? Even bloat day by day. :( With that, userspace tools should
> spend more time to parse which is pointless.
>
> Having said that, I'm not fan of creating new stat knob for that, either.
> How about appending summary information in the end of smap?
> So, monitoring users can just open the file and lseek to the (end - 1) and
> read the summary only.
>

That would work fine for us as long as it's fast -- i.e. we don't
still have to do all the expensive per-VMA format conversion in the
kernel.

> Thanks.

[toc] | [prev] | [next] | [standalone]


#1466245

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-19 11:10 +0200
Message-ID<s7NYm-3GZ-23@gated-at.bofh.it>
In reply to#1465907
On Fri 19-08-16 11:26:34, Minchan Kim wrote:
> Hi Michal,
> 
> On Thu, Aug 18, 2016 at 08:01:04PM +0200, Michal Hocko wrote:
> > On Thu 18-08-16 10:47:57, Sonny Rao wrote:
> > > On Thu, Aug 18, 2016 at 12:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > > On Wed 17-08-16 11:57:56, Sonny Rao wrote:
> > [...]
> > > >> 2) User space OOM handling -- we'd rather do a more graceful shutdown
> > > >> than let the kernel's OOM killer activate and need to gather this
> > > >> information and we'd like to be able to get this information to make
> > > >> the decision much faster than 400ms
> > > >
> > > > Global OOM handling in userspace is really dubious if you ask me. I
> > > > understand you want something better than SIGKILL and in fact this is
> > > > already possible with memory cgroup controller (btw. memcg will give
> > > > you a cheap access to rss, amount of shared, swapped out memory as
> > > > well). Anyway if you are getting close to the OOM your system will most
> > > > probably be really busy and chances are that also reading your new file
> > > > will take much more time. I am also not quite sure how is pss useful for
> > > > oom decisions.
> > > 
> > > I mentioned it before, but based on experience RSS just isn't good
> > > enough -- there's too much sharing going on in our use case to make
> > > the correct decision based on RSS.  If RSS were good enough, simply
> > > put, this patch wouldn't exist.
> > 
> > But that doesn't answer my question, I am afraid. So how exactly do you
> > use pss for oom decisions?
> 
> My case is not for OOM decision but I agree it would be great if we can get
> *fast* smap summary information.
> 
> PSS is really great tool to figure out how processes consume memory
> more exactly rather than RSS. We have been used it for monitoring
> of memory for per-process. Although it is not used for OOM decision,
> it would be great if it is speed up because we don't want to spend
> many CPU time for just monitoring.
> 
> For our usecase, we don't need AnonHugePages, ShmemPmdMapped, Shared_Hugetlb,
> Private_Hugetlb, KernelPageSize, MMUPageSize because we never enable THP and
> hugetlb. Additionally, Locked can be known via vma flags so we don't need it,
> either. Even, we don't need address range for just monitoring when we don't
> investigate in detail.
> 
> Although they are not severe overhead, why does it emit the useless
> information? Even bloat day by day. :( With that, userspace tools should
> spend more time to parse which is pointless.

So far it doesn't really seem that the parsing is the biggest problem.
The major cycles killer is the output formatting and that doesn't sound
like a problem we are not able to address. And I would even argue that
we want to address it in a generic way as much as possible.

> Having said that, I'm not fan of creating new stat knob for that, either.
> How about appending summary information in the end of smap?
> So, monitoring users can just open the file and lseek to the (end - 1) and
> read the summary only.

That might confuse existing parsers. Besides that we already have
/proc/<pid>/statm which gives cumulative numbers already. I am not sure
how often it is used and whether the pte walk is too expensive for
existing users but that should be explored and evaluated before a new
file is created.

The /proc became a dump of everything people found interesting just
because we were to easy to allow those additions. Do not repeat those
mistakes, please!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web