Re: [PATCH v2] mm, proc: Fix region lost in /proc/self/smaps

From: Xiao Guangrong
Date: Mon Sep 12 2016 - 23:07:02 EST




On 09/13/2016 03:10 AM, Michal Hocko wrote:
On Mon 12-09-16 08:01:06, Dave Hansen wrote:
On 09/12/2016 05:54 AM, Michal Hocko wrote:
In order to fix this bug, we make 'file->version' indicate the end address
of current VMA
Doesn't this open doors to another weird cases. Say B would be partially
unmapped (tail of the VMA would get unmapped and reused for a new VMA.

In the end, this interface isn't about VMAs. It's about addresses, and
we need to make sure that the _addresses_ coming out of it are sane. In
the case that a VMA was partially unmapped, it doesn't make sense to
show the "new" VMA because we already had some output covering the
address of the "new" VMA from the old one.

OK, that is a fair point and it speaks for caching the vm_end rather
than vm_start+skip.

I am not sure we provide any guarantee when there are more read
syscalls. Hmm, even with a single read() we can get inconsistent results
from different threads without any user space synchronization.

Yeah, very true. But, I think we _can_ at least provide the following
guarantees (among others):
1. addresses don't go backwards
2. If there is something at a given vaddr during the entirety of the
life of the smaps walk, we will produce some output for it.

I guess we also want
3. no overlaps with previously printed values (assuming two subsequent
reads without seek).

the patch tries to achieve the last part as well AFAICS but I guess this
is incomplete because at least /proc/<pid>/smaps will report counters
for the full vma range while the header (aka show_map_vma) will report
shorter (non-overlapping) range. I haven't checked other files which use
m_{start,next}

You are right. Will fix both /proc/PID/smaps and /proc/PID/maps in
the next version.


Considering how this all can be tricky and how partial reads can be
confusing and even misleading I am really wondering whether we
should simply document that only full reads will provide a sensible
results.

Make sense. Will document the guarantee in
Documentation/filesystems/proc.txt

Thank you, Dave and Michal, for figuring out the right direction. :)