Re: [PACTH v1] mm, proc: Implement /proc/<pid>/totmaps

From: Robert Foss
Date: Fri Aug 12 2016 - 12:28:29 EST




On 2016-08-10 02:05 PM, Jann Horn wrote:
On Wed, Aug 10, 2016 at 10:45:51AM -0700, Sonny Rao wrote:
On Wed, Aug 10, 2016 at 10:37 AM, Jann Horn <jann@xxxxxxxxx> wrote:
On Wed, Aug 10, 2016 at 10:23:53AM -0700, Sonny Rao wrote:
On Tue, Aug 9, 2016 at 2:01 PM, Robert Foss <robert.foss@xxxxxxxxxxxxx> wrote:


On 2016-08-09 03:24 PM, Jann Horn wrote:

On Tue, Aug 09, 2016 at 12:05:43PM -0400, robert.foss@xxxxxxxxxxxxx wrote:

From: Sonny Rao <sonnyrao@xxxxxxxxxxxx>

This is based on earlier work by Thiago Goncales. It implements a new
per process proc file which summarizes the contents of the smaps file
but doesn't display any addresses. It gives more detailed information
than statm like the PSS (proprotional set size). It differs from the
original implementation in that it doesn't use the full blown set of
seq operations, uses a different termination condition, and doesn't
displayed "Locked" as that was broken on the original implemenation.

This new proc file provides information faster than parsing the
potentially
huge smaps file.

Signed-off-by: Sonny Rao <sonnyrao@xxxxxxxxxxxx>

Tested-by: Robert Foss <robert.foss@xxxxxxxxxxxxx>
Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxxxxx>



+static int totmaps_proc_show(struct seq_file *m, void *data)
+{
+ struct proc_maps_private *priv = m->private;
+ struct mm_struct *mm;
+ struct vm_area_struct *vma;
+ struct mem_size_stats *mss_sum = priv->mss;
+
+ /* reference to priv->task already taken */
+ /* but need to get the mm here because */
+ /* task could be in the process of exiting */


Can you please elaborate on this? My understanding here is that you
intend for the caller to be able to repeatedly read the same totmaps
file with pread() and still see updated information after the target
process has called execve() and be able to detect process death
(instead of simply seeing stale values). Is that accurate?

I would prefer it if you could grab a reference to the mm_struct
directly at open time.


Sonny, do you know more about the above comment?

I think right now the file gets re-opened every time, but the mode
where the file is opened once and repeatedly read is interesting
because it avoids having to open the file again and again.

I guess you could end up with a wierd situation where you don't read
the entire contents of the file in open call to read() and you might
get inconsistent data across the different statistics?

If the file is read in two chunks, totmaps_proc_show is only called
once. The patch specifies seq_read as read handler. Have a look at its
definition. As long as you don't read from the same seq file in
parallel or seek around in it, simple sequential reads will not
re-invoke the show() method for data that has already been formatted.
For partially consumed data, the kernel buffers the rest until someone
reads it or seeks to another offset.

Ok that's good. If the consumer were using pread() though, would that
look like a seek?

Only if the consumer uses pread() with an offset that is not the same as
the end offset of the previous read.

So if you tried to use the same file from multiple threads in parallel,
you might still have issues, but as long as you don't do that, it should
be fine.

I guess it might make sense to document this behavior somewhere - maybe
the proc.5 manpage?


I'll add a note about limitations for parallel read. The overall documentation for this feature should live in the proc.5 manpage as well?