Re: [PATCH v4 1/5] mm/page_idle: Add per-pid idle page tracking using virtual indexing

On Tue, Aug 06, 2019 at 10:56:05AM +0200, Michal Hocko wrote:

On Mon 05-08-19 13:04:47, Joel Fernandes (Google) wrote:

quoted

The page_idle tracking feature currently requires looking up the pagemap
for a process followed by interacting with /sys/kernel/mm/page_idle.
Looking up PFN from pagemap in Android devices is not supported by
unprivileged process and requires SYS_ADMIN and gives 0 for the PFN.

This patch adds support to directly interact with page_idle tracking at
the PID level by introducing a /proc/<pid>/page_idle file.  It follows
the exact same semantics as the global /sys/kernel/mm/page_idle, but now
looking up PFN through pagemap is not needed since the interface uses
virtual frame numbers, and at the same time also does not require
SYS_ADMIN.

In Android, we are using this for the heap profiler (heapprofd) which
profiles and pin points code paths which allocates and leaves memory
idle for long periods of time. This method solves the security issue
with userspace learning the PFN, and while at it is also shown to yield
better results than the pagemap lookup, the theory being that the window
where the address space can change is reduced by eliminating the
intermediate pagemap look up stage. In virtual address indexing, the
process's mmap_sem is held for the duration of the access.

As already mentioned in one of the previous versions. The interface
seems sane and the usecase as well. So I do not really have high level
objections.

That is great to know.

From a quick look at the patch I would just object to pulling swap idle
tracking into this patch because it makes the review harder and it is
essentially a dead code until a later patch. I am also not sure whether
that is really necessary and it really begs for an explicit
justification.

Ok I will split it out, and also expand on the need for it a bit more.

I will try to go through the patch more carefully later as time allows.

Thanks a lot.

quoted

Signed-off-by: Joel Fernandes (Google) <redacted>

-- 
Michal Hocko
SUSE Labs

 - Joel