[PATCH v6 0/7] introduce memory hinting API for external process

[PATCH v6 0/7] introduce memory hinting API for external process

[Minchan Kim]

Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
application could give hints to kernel what memory range are preferred to be
reclaimed. However, in some platform(e.g., Android), the information
required to make the hinting decision is not known to the app.
Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
and that daemon must be able to initiate reclaim on its own without any app
involvement.

To solve the concern, this patch introduces new syscall - process_madvise(2).
Bascially, it's same with madvise(2) syscall but it has some differences.

1. It needs pidfd of target process to provide the hint
2. It supports only MADV_{COLD|PAGEOUT|MERGEABLE|UNMEREABLE} at this moment.
   Other hints in madvise will be opened when there are explicit requests from
   community to prevent unexpected bugs we couldn't support.
3. Only privileged processes can do something for other process's address
   space.

For more detail of the new API, please see "mm: introduce external memory hinting API"
description in this patchset.

* from v5 - https://lore.kernel.org/linux-mm/20200214170520.160271-1-minchan@kernel.org/
  * use null task and requestor's mm for io_madvise - Jann and Jens
  * use right commit description for moving pidfd_get_pid - Christoph

* from v4 - https://lore.kernel.org/linux-mm/20200212233946.246210-1-minchan@kernel.org/
  * pass mm down to functions, not accessing task->mm - Jann
  * clean up - Alexander
  * add Reviewed-by - Alexander, SeongJae
  * patch reordering

* from v3 - https://lore.kernel.org/linux-mm/20200128001641.5086-1-minchan@kernel.org/
  * verify task->mm aftere access_mm - Oleg
  * split some patches for easy review - Alexander
  * clean up fatal signal checking - Suren

* from v2 - https://lore.kernel.org/linux-mm/20200116235953.163318-1-minchan@kernel.org/
  * check signal callee and caller to bail out - Kirill Tkhai
  * put more clarification for justification of new API

* from v1 - https://lore.kernel.org/linux-mm/20200110213433.94739-1-minchan@kernel.org/
  * fix syscall number - SeongJae
  * use get_pid_task - Kirill Tkhai
  * extend API to support pid as well as pidfd - Kirill Tkhai


Minchan Kim (5):
  mm: pass task and mm to do_madvise
  mm: introduce external memory hinting API
  mm: check fatal signal pending of target process
  pid: move pidfd_get_pid function to pid.c
  mm: support both pid and pidfd for process_madvise

Oleksandr Natalenko (2):
  mm/madvise: employ mmget_still_valid for write lock
  mm/madvise: allow KSM hints for remote API

 arch/alpha/kernel/syscalls/syscall.tbl      |   1 +
 arch/arm/tools/syscall.tbl                  |   1 +
 arch/arm64/include/asm/unistd.h             |   2 +-
 arch/arm64/include/asm/unistd32.h           |   2 +
 arch/ia64/kernel/syscalls/syscall.tbl       |   1 +
 arch/m68k/kernel/syscalls/syscall.tbl       |   1 +
 arch/microblaze/kernel/syscalls/syscall.tbl |   1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |   1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |   1 +
 arch/parisc/kernel/syscalls/syscall.tbl     |   1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |   1 +
 arch/s390/kernel/syscalls/syscall.tbl       |   1 +
 arch/sh/kernel/syscalls/syscall.tbl         |   1 +
 arch/sparc/kernel/syscalls/syscall.tbl      |   1 +
 arch/x86/entry/syscalls/syscall_32.tbl      |   1 +
 arch/x86/entry/syscalls/syscall_64.tbl      |   1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |   1 +
 fs/io_uring.c                               |   2 +-
 include/linux/mm.h                          |   3 +-
 include/linux/pid.h                         |   1 +
 include/linux/syscalls.h                    |   3 +
 include/uapi/asm-generic/unistd.h           |   4 +-
 kernel/exit.c                               |  17 ---
 kernel/pid.c                                |  17 +++
 kernel/sys_ni.c                             |   1 +
 mm/madvise.c                                | 144 ++++++++++++++++----
 26 files changed, 167 insertions(+), 44 deletions(-)

-- 
2.25.0.265.gbab2e86ba0-goog

[PATCH v6 1/7] mm: pass task and mm to do_madvise

[Minchan Kim]

In upcoming patches, do_madvise will be called from external process
context so we shouldn't asssume "current" is always hinted process's
task_struct. Furthermore, we couldn't access mm_struct via task->mm
once it's verified by access_mm which will be introduced in next
patch[1]. And let's pass *current* and current->mm as arguments of
do_madvise so it shouldn't change existing behavior but prepare
next patch to make review easy.

Note: io_madvise pass NULL as target_task argument of do_madvise
because it couldn't know who is target.

[1] http://lore.kernel.org/r/CAG48ez27=pwm5m_N_988xT1huO7g7h6arTQL44zev6TD-h-7Tg@mail.gmail.com

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jann Horn <jannh@google.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 fs/io_uring.c      |  2 +-
 include/linux/mm.h |  3 ++-
 mm/madvise.c       | 34 +++++++++++++++++++---------------
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 63beda9bafc5..a858da2ae2f4 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2736,7 +2736,7 @@ static int io_madvise(struct io_kiocb *req, struct io_kiocb **nxt,
 	if (force_nonblock)
 		return -EAGAIN;
 
-	ret = do_madvise(ma->addr, ma->len, ma->advice);
+	ret = do_madvise(NULL, req->work.mm, ma->addr, ma->len, ma->advice);
 	if (ret < 0)
 		req_set_fail_links(req);
 	io_cqring_add_event(req, ret);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 52269e56c514..bc16c8774328 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2323,7 +2323,8 @@ extern int __do_munmap(struct mm_struct *, unsigned long, size_t,
 		       struct list_head *uf, bool downgrade);
 extern int do_munmap(struct mm_struct *, unsigned long, size_t,
 		     struct list_head *uf);
-extern int do_madvise(unsigned long start, size_t len_in, int behavior);
+extern int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
+		unsigned long start, size_t len_in, int behavior);
 
 static inline unsigned long
 do_mmap_pgoff(struct file *file, unsigned long addr,
diff --git a/mm/madvise.c b/mm/madvise.c
index 43b47d3fae02..f75c86b6c463 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -254,6 +254,7 @@ static long madvise_willneed(struct vm_area_struct *vma,
 			     struct vm_area_struct **prev,
 			     unsigned long start, unsigned long end)
 {
+	struct mm_struct *mm = vma->vm_mm;
 	struct file *file = vma->vm_file;
 	loff_t offset;
 
@@ -288,12 +289,12 @@ static long madvise_willneed(struct vm_area_struct *vma,
 	 */
 	*prev = NULL;	/* tell sys_madvise we drop mmap_sem */
 	get_file(file);
-	up_read(&current->mm->mmap_sem);
+	up_read(&mm->mmap_sem);
 	offset = (loff_t)(start - vma->vm_start)
 			+ ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
 	vfs_fadvise(file, offset, end - start, POSIX_FADV_WILLNEED);
 	fput(file);
-	down_read(&current->mm->mmap_sem);
+	down_read(&mm->mmap_sem);
 	return 0;
 }
 
@@ -676,7 +677,6 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
 	if (nr_swap) {
 		if (current->mm == mm)
 			sync_mm_rss(mm);
-
 		add_mm_counter(mm, MM_SWAPENTS, nr_swap);
 	}
 	arch_leave_lazy_mmu_mode();
@@ -756,6 +756,8 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
 				  unsigned long start, unsigned long end,
 				  int behavior)
 {
+	struct mm_struct *mm = vma->vm_mm;
+
 	*prev = vma;
 	if (!can_madv_lru_vma(vma))
 		return -EINVAL;
@@ -763,8 +765,8 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
 	if (!userfaultfd_remove(vma, start, end)) {
 		*prev = NULL; /* mmap_sem has been dropped, prev is stale */
 
-		down_read(&current->mm->mmap_sem);
-		vma = find_vma(current->mm, start);
+		down_read(&mm->mmap_sem);
+		vma = find_vma(mm, start);
 		if (!vma)
 			return -ENOMEM;
 		if (start < vma->vm_start) {
@@ -818,6 +820,7 @@ static long madvise_remove(struct vm_area_struct *vma,
 	loff_t offset;
 	int error;
 	struct file *f;
+	struct mm_struct *mm = vma->vm_mm;
 
 	*prev = NULL;	/* tell sys_madvise we drop mmap_sem */
 
@@ -845,13 +848,13 @@ static long madvise_remove(struct vm_area_struct *vma,
 	get_file(f);
 	if (userfaultfd_remove(vma, start, end)) {
 		/* mmap_sem was not released by userfaultfd_remove() */
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 	}
 	error = vfs_fallocate(f,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 				offset, end - start);
 	fput(f);
-	down_read(&current->mm->mmap_sem);
+	down_read(&mm->mmap_sem);
 	return error;
 }
 
@@ -1044,7 +1047,8 @@ madvise_behavior_valid(int behavior)
  *  -EBADF  - map exists, but area maps something that isn't a file.
  *  -EAGAIN - a kernel resource was temporarily unavailable.
  */
-int do_madvise(unsigned long start, size_t len_in, int behavior)
+int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
+		unsigned long start, size_t len_in, int behavior)
 {
 	unsigned long end, tmp;
 	struct vm_area_struct *vma, *prev;
@@ -1082,10 +1086,10 @@ int do_madvise(unsigned long start, size_t len_in, int behavior)
 
 	write = madvise_need_mmap_write(behavior);
 	if (write) {
-		if (down_write_killable(&current->mm->mmap_sem))
+		if (down_write_killable(&mm->mmap_sem))
 			return -EINTR;
 	} else {
-		down_read(&current->mm->mmap_sem);
+		down_read(&mm->mmap_sem);
 	}
 
 	/*
@@ -1093,7 +1097,7 @@ int do_madvise(unsigned long start, size_t len_in, int behavior)
 	 * ranges, just ignore them, but return -ENOMEM at the end.
 	 * - different from the way of handling in mlock etc.
 	 */
-	vma = find_vma_prev(current->mm, start, &prev);
+	vma = find_vma_prev(mm, start, &prev);
 	if (vma && start > vma->vm_start)
 		prev = vma;
 
@@ -1130,19 +1134,19 @@ int do_madvise(unsigned long start, size_t len_in, int behavior)
 		if (prev)
 			vma = prev->vm_next;
 		else	/* madvise_remove dropped mmap_sem */
-			vma = find_vma(current->mm, start);
+			vma = find_vma(mm, start);
 	}
 out:
 	blk_finish_plug(&plug);
 	if (write)
-		up_write(&current->mm->mmap_sem);
+		up_write(&mm->mmap_sem);
 	else
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 
 	return error;
 }
 
 SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 {
-	return do_madvise(start, len_in, behavior);
+	return do_madvise(current, current->mm, start, len_in, behavior);
 }
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 1/7] mm: pass task and mm to do_madvise

[Suren Baghdasaryan]

On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
>
> In upcoming patches, do_madvise will be called from external process
> context so we shouldn't asssume "current" is always hinted process's
> task_struct. Furthermore, we couldn't access mm_struct via task->mm
> once it's verified by access_mm which will be introduced in next
> patch[1]. And let's pass *current* and current->mm as arguments of
> do_madvise so it shouldn't change existing behavior but prepare
> next patch to make review easy.
>
> Note: io_madvise pass NULL as target_task argument of do_madvise
> because it couldn't know who is target.
>
> [1] http://lore.kernel.org/r/CAG48ez27=pwm5m_N_988xT1huO7g7h6arTQL44zev6TD-h-7Tg@mail.gmail.com
>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Jann Horn <jannh@google.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  fs/io_uring.c      |  2 +-
>  include/linux/mm.h |  3 ++-
>  mm/madvise.c       | 34 +++++++++++++++++++---------------
>  3 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 63beda9bafc5..a858da2ae2f4 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2736,7 +2736,7 @@ static int io_madvise(struct io_kiocb *req, struct io_kiocb **nxt,
>         if (force_nonblock)
>                 return -EAGAIN;
>
> -       ret = do_madvise(ma->addr, ma->len, ma->advice);
> +       ret = do_madvise(NULL, req->work.mm, ma->addr, ma->len, ma->advice);
>         if (ret < 0)
>                 req_set_fail_links(req);
>         io_cqring_add_event(req, ret);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 52269e56c514..bc16c8774328 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2323,7 +2323,8 @@ extern int __do_munmap(struct mm_struct *, unsigned long, size_t,
>                        struct list_head *uf, bool downgrade);
>  extern int do_munmap(struct mm_struct *, unsigned long, size_t,
>                      struct list_head *uf);
> -extern int do_madvise(unsigned long start, size_t len_in, int behavior);
> +extern int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
> +               unsigned long start, size_t len_in, int behavior);
>
>  static inline unsigned long
>  do_mmap_pgoff(struct file *file, unsigned long addr,
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 43b47d3fae02..f75c86b6c463 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -254,6 +254,7 @@ static long madvise_willneed(struct vm_area_struct *vma,
>                              struct vm_area_struct **prev,
>                              unsigned long start, unsigned long end)
>  {
> +       struct mm_struct *mm = vma->vm_mm;
>         struct file *file = vma->vm_file;
>         loff_t offset;
>
> @@ -288,12 +289,12 @@ static long madvise_willneed(struct vm_area_struct *vma,
>          */
>         *prev = NULL;   /* tell sys_madvise we drop mmap_sem */
>         get_file(file);
> -       up_read(&current->mm->mmap_sem);
> +       up_read(&mm->mmap_sem);
>         offset = (loff_t)(start - vma->vm_start)
>                         + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
>         vfs_fadvise(file, offset, end - start, POSIX_FADV_WILLNEED);
>         fput(file);
> -       down_read(&current->mm->mmap_sem);
> +       down_read(&mm->mmap_sem);
>         return 0;
>  }
>
> @@ -676,7 +677,6 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
>         if (nr_swap) {
>                 if (current->mm == mm)
>                         sync_mm_rss(mm);
> -
>                 add_mm_counter(mm, MM_SWAPENTS, nr_swap);
>         }
>         arch_leave_lazy_mmu_mode();
> @@ -756,6 +756,8 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
>                                   unsigned long start, unsigned long end,
>                                   int behavior)
>  {
> +       struct mm_struct *mm = vma->vm_mm;
> +
>         *prev = vma;
>         if (!can_madv_lru_vma(vma))
>                 return -EINVAL;
> @@ -763,8 +765,8 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
>         if (!userfaultfd_remove(vma, start, end)) {
>                 *prev = NULL; /* mmap_sem has been dropped, prev is stale */
>
> -               down_read(&current->mm->mmap_sem);
> -               vma = find_vma(current->mm, start);
> +               down_read(&mm->mmap_sem);
> +               vma = find_vma(mm, start);
>                 if (!vma)
>                         return -ENOMEM;
>                 if (start < vma->vm_start) {
> @@ -818,6 +820,7 @@ static long madvise_remove(struct vm_area_struct *vma,
>         loff_t offset;
>         int error;
>         struct file *f;
> +       struct mm_struct *mm = vma->vm_mm;
>
>         *prev = NULL;   /* tell sys_madvise we drop mmap_sem */
>
> @@ -845,13 +848,13 @@ static long madvise_remove(struct vm_area_struct *vma,
>         get_file(f);
>         if (userfaultfd_remove(vma, start, end)) {
>                 /* mmap_sem was not released by userfaultfd_remove() */
> -               up_read(&current->mm->mmap_sem);
> +               up_read(&mm->mmap_sem);
>         }
>         error = vfs_fallocate(f,
>                                 FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>                                 offset, end - start);
>         fput(f);
> -       down_read(&current->mm->mmap_sem);
> +       down_read(&mm->mmap_sem);
>         return error;
>  }
>
> @@ -1044,7 +1047,8 @@ madvise_behavior_valid(int behavior)
>   *  -EBADF  - map exists, but area maps something that isn't a file.
>   *  -EAGAIN - a kernel resource was temporarily unavailable.
>   */
> -int do_madvise(unsigned long start, size_t len_in, int behavior)
> +int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
> +               unsigned long start, size_t len_in, int behavior)
>  {
>         unsigned long end, tmp;
>         struct vm_area_struct *vma, *prev;
> @@ -1082,10 +1086,10 @@ int do_madvise(unsigned long start, size_t len_in, int behavior)
>
>         write = madvise_need_mmap_write(behavior);
>         if (write) {
> -               if (down_write_killable(&current->mm->mmap_sem))
> +               if (down_write_killable(&mm->mmap_sem))
>                         return -EINTR;
>         } else {
> -               down_read(&current->mm->mmap_sem);
> +               down_read(&mm->mmap_sem);
>         }
>
>         /*
> @@ -1093,7 +1097,7 @@ int do_madvise(unsigned long start, size_t len_in, int behavior)
>          * ranges, just ignore them, but return -ENOMEM at the end.
>          * - different from the way of handling in mlock etc.
>          */
> -       vma = find_vma_prev(current->mm, start, &prev);
> +       vma = find_vma_prev(mm, start, &prev);
>         if (vma && start > vma->vm_start)
>                 prev = vma;
>
> @@ -1130,19 +1134,19 @@ int do_madvise(unsigned long start, size_t len_in, int behavior)
>                 if (prev)
>                         vma = prev->vm_next;
>                 else    /* madvise_remove dropped mmap_sem */
> -                       vma = find_vma(current->mm, start);
> +                       vma = find_vma(mm, start);
>         }
>  out:
>         blk_finish_plug(&plug);
>         if (write)
> -               up_write(&current->mm->mmap_sem);
> +               up_write(&mm->mmap_sem);
>         else
> -               up_read(&current->mm->mmap_sem);
> +               up_read(&mm->mmap_sem);
>
>         return error;
>  }
>
>  SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
>  {
> -       return do_madvise(start, len_in, behavior);
> +       return do_madvise(current, current->mm, start, len_in, behavior);
>  }
> --
> 2.25.0.265.gbab2e86ba0-goog
>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

[PATCH v6 2/7] mm: introduce external memory hinting API

[Minchan Kim]

There is usecase that System Management Software(SMS) want to give
a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
in the case of Android, it is the ActivityManagerService.

It's similar in spirit to madvise(MADV_WONTNEED), but the information
required to make the reclaim decision is not known to the app. Instead,
it is known to the centralized userspace daemon(ActivityManagerService),
and that daemon must be able to initiate reclaim on its own without
any app involvement.

To solve the issue, this patch introduces a new syscall process_madvise(2).
It uses pidfd of an external process to give the hint.

 int process_madvise(int pidfd, void *addr, size_t length, int advise,
			unsigned long flag);

Since it could affect other process's address range, only privileged
process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
gives it the right to ptrace the process could use it successfully.
The flag argument is reserved for future use if we need to extend the
API.

I think supporting all hints madvise has/will supported/support to
process_madvise is rather risky. Because we are not sure all hints make
sense from external process and implementation for the hint may rely on
the caller being in the current context so it could be error-prone.
Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.

If someone want to add other hints, we could hear hear the usecase and
review it for each hint. It's safer for maintenance rather than
introducing a buggy syscall but hard to fix it later.

Q.1 - Why does any external entity have better knowledge?

Quote from Sandeep
"For Android, every application (including the special SystemServer) are forked
from Zygote. The reason of course is to share as many libraries and classes between
the two as possible to benefit from the preloading during boot.

After applications start, (almost) all of the APIs  end up calling into this
SystemServer process over IPC (binder) and back to the application.

In a fully running system, the SystemServer monitors every single process
periodically to calculate their PSS / RSS and also decides which process is
"important" to the user for interactivity.

So, because of how these processes start _and_ the fact that the SystemServer
is looping to monitor each process, it does tend to *know* which address
range of the application is not used / useful.

Besides, we can never rely on applications to clean things up themselves.
We've had the "hey app1, the system is low on memory, please trim your
memory usage down" notifications for a long time[1]. They rely on
applications honoring the broadcasts and very few do.

So, if we want to avoid the inevitable killing of the application and
restarting it, some way to be able to tell the OS about unimportant memory in
these applications will be useful.

- ssp

Q.2 - How to guarantee the race(i.e., object validation) between when giving a
hint from an external process and get the hint from the target process?

process_madvise operates on the target process's address space as it exists
at the instant that process_madvise is called. If the space target process
can run between the time the process_madvise process inspects the target
process address space and the time that process_madvise is actually called,
process_madvise may operate on memory regions that the calling process does
not expect. It's the responsibility of the process calling process_madvise
to close this race condition. For example, the calling process can suspend
the target process with ptrace, SIGSTOP, or the freezer cgroup so that it
doesn't have an opportunity to change its own address space before
process_madvise is called. Another option is to operate on memory regions
that the caller knows a priori will be unchanged in the target process.
Yet another option is to accept the race for certain process_madvise calls
after reasoning that mistargeting will do no harm. The suggested API itself
does not provide synchronization. It also apply other APIs like move_pages,
process_vm_write.

The race isn't really a problem though. Why is it so wrong to require
that callers do their own synchronization in some manner? Nobody objects
to write(2) merely because it's possible for two processes to open the same
file and clobber each other's writes --- instead, we tell people to use
flock or something. Think about mmap. It never guarantees newly allocated
address space is still valid when the user tries to access it because other
threads could unmap the memory right before. That's where we need
synchronization by using other API or design from userside. It shouldn't
be part of API itself. If someone needs more fine-grained synchronization
rather than process level, there were two ideas suggested - cookie[2] and
anon-fd[3]. Both are applicable via using last reserved argument of the API
but I don't think it's necessary right now since we have already ways to
prevent the race so don't want to add additional complexity with more
fine-grained optimization model.

To make the API extend, it reserved an unsigned long as last argument
so we could support it in future if someone really needs it.

Q.3 - Why doesn't ptrace work?

Injecting an madvise in the target process using ptrace would not work for us
because such injected madvise would have to be executed by the target process,
which means that process would have to be runnable and that creates the risk
of the abovementioned race and hinting a wrong VMA. Furthermore, we want to
act the hint in caller's context, not calle because calle is usually limited
in cpuset/cgroups or even freezed state so they can't act by themselves
quick enough, which causes more thrashing/kill. It doesn't work if the
target process are ptraced(e.g., strace, debugger, minidump) because  a
process can have at most one ptracer.

[1] https://developer.android.com/topic/performance/memory"
[2] process_getinfo for getting the cookie which is updated whenever
    vma of process address layout are changed - Daniel Colascione
- https://lore.kernel.org/lkml/20190520035254.57579-1-minchan@kernel.org/T/#m7694416fd179b2066a2c62b5b139b14e3894e224
[3] anonymous fd which is used for the object(i.e., address range)
    validation - Michal Hocko
- https://lore.kernel.org/lkml/20200120112722.GY18451@dhcp22.suse.cz/

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 arch/alpha/kernel/syscalls/syscall.tbl      |  1 +
 arch/arm/tools/syscall.tbl                  |  1 +
 arch/arm64/include/asm/unistd.h             |  2 +-
 arch/arm64/include/asm/unistd32.h           |  2 +
 arch/ia64/kernel/syscalls/syscall.tbl       |  1 +
 arch/m68k/kernel/syscalls/syscall.tbl       |  1 +
 arch/microblaze/kernel/syscalls/syscall.tbl |  1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |  1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |  1 +
 arch/parisc/kernel/syscalls/syscall.tbl     |  1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |  1 +
 arch/s390/kernel/syscalls/syscall.tbl       |  1 +
 arch/sh/kernel/syscalls/syscall.tbl         |  1 +
 arch/sparc/kernel/syscalls/syscall.tbl      |  1 +
 arch/x86/entry/syscalls/syscall_32.tbl      |  1 +
 arch/x86/entry/syscalls/syscall_64.tbl      |  1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |  1 +
 include/linux/syscalls.h                    |  2 +
 include/uapi/asm-generic/unistd.h           |  4 +-
 kernel/sys_ni.c                             |  1 +
 mm/madvise.c                                | 64 +++++++++++++++++++++
 21 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 36d42da7466a..c82952e6fb80 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -477,3 +477,4 @@
 # 545 reserved for clone3
 547	common	openat2				sys_openat2
 548	common	pidfd_getfd			sys_pidfd_getfd
+549	common	process_madvise			sys_process_madvise
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 4d1cf74a2caa..54c2719fec46 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -451,3 +451,4 @@
 435	common	clone3				sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 1dd22da1c3a9..75f04a1023be 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -38,7 +38,7 @@
 #define __ARM_NR_compat_set_tls		(__ARM_NR_COMPAT_BASE + 5)
 #define __ARM_NR_COMPAT_END		(__ARM_NR_COMPAT_BASE + 0x800)
 
-#define __NR_compat_syscalls		439
+#define __NR_compat_syscalls		440
 #endif
 
 #define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index c1c61635f89c..2a27be7a1f91 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -883,6 +883,8 @@ __SYSCALL(__NR_clone3, sys_clone3)
 __SYSCALL(__NR_openat2, sys_openat2)
 #define __NR_pidfd_getfd 438
 __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
+#define __NR_process_madvise 439
+__SYSCALL(__NR_process_madvise, process_madvise)
 
 /*
  * Please add new compat syscalls above this comment and update
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index 042911e670b8..9524af1c318c 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -358,3 +358,4 @@
 # 435 reserved for clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index f4f49fcb76d0..8197050c097c 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -437,3 +437,4 @@
 435	common	clone3				__sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 4c67b11f9c9e..c5b6c8afe445 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -443,3 +443,4 @@
 435	common	clone3				sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 1f9e8ad636cc..8ec8c558aa9c 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -376,3 +376,4 @@
 435	n32	clone3				__sys_clone3
 437	n32	openat2				sys_openat2
 438	n32	pidfd_getfd			sys_pidfd_getfd
+439	n32	process_madvise			sys_process_madvise
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index c0b9d802dbf6..0078f891bb92 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -352,3 +352,4 @@
 435	n64	clone3				__sys_clone3
 437	n64	openat2				sys_openat2
 438	n64	pidfd_getfd			sys_pidfd_getfd
+439	n64	process_madvise			sys_process_madvise
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 52a15f5cd130..09c3b5dc6855 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -435,3 +435,4 @@
 435	common	clone3				sys_clone3_wrapper
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 35b61bfc1b1a..97eac48c2937 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -519,3 +519,4 @@
 435	nospu	clone3				ppc_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index bd7bd3581a0f..8dc8bfd958ea 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 435  common	clone3			sys_clone3			sys_clone3
 437  common	openat2			sys_openat2			sys_openat2
 438  common	pidfd_getfd		sys_pidfd_getfd			sys_pidfd_getfd
+439  common	process_madvise		sys_process_madvise		sys_process_madvise
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c7a30fcd135f..e69d98040777 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 # 435 reserved for clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index f13615ecdecc..6f6e66dd51f9 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -483,3 +483,4 @@
 # 435 reserved for clone3
 437	common	openat2			sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index c17cb77eb150..1b2184549e27 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -442,3 +442,4 @@
 435	i386	clone3			sys_clone3			__ia32_sys_clone3
 437	i386	openat2			sys_openat2			__ia32_sys_openat2
 438	i386	pidfd_getfd		sys_pidfd_getfd			__ia32_sys_pidfd_getfd
+439	i386	process_madvise		sys_process_madvise		__ia32_sys_process_madvise
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 44d510bc9b78..82d60eb1e00d 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -359,6 +359,7 @@
 435	common	clone3			__x64_sys_clone3/ptregs
 437	common	openat2			__x64_sys_openat2
 438	common	pidfd_getfd		__x64_sys_pidfd_getfd
+439	common	process_madvise		__x64_sys_process_madvise
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 85a9ab1bc04d..165cae047770 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -408,3 +408,4 @@
 435	common	clone3				sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 1815065d52f3..e4cd2c2f8bb4 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -876,6 +876,8 @@ asmlinkage long sys_munlockall(void);
 asmlinkage long sys_mincore(unsigned long start, size_t len,
 				unsigned char __user * vec);
 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
+asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
+			size_t len, int behavior, unsigned long flags);
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
 			unsigned long prot, unsigned long pgoff,
 			unsigned long flags);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 3a3201e4618e..85d8c9376a63 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -855,9 +855,11 @@ __SYSCALL(__NR_clone3, sys_clone3)
 __SYSCALL(__NR_openat2, sys_openat2)
 #define __NR_pidfd_getfd 438
 __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
+#define __NR_pidfd_getfd 439
+__SYSCALL(__NR_process_madvise, sys_process_madvise)
 
 #undef __NR_syscalls
-#define __NR_syscalls 439
+#define __NR_syscalls 440
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 3b69a560a7ac..6c7332776e8e 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -280,6 +280,7 @@ COND_SYSCALL(mlockall);
 COND_SYSCALL(munlockall);
 COND_SYSCALL(mincore);
 COND_SYSCALL(madvise);
+COND_SYSCALL(process_madvise);
 COND_SYSCALL(remap_file_pages);
 COND_SYSCALL(mbind);
 COND_SYSCALL_COMPAT(mbind);
diff --git a/mm/madvise.c b/mm/madvise.c
index f75c86b6c463..f29155b8185d 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -17,6 +17,7 @@
 #include <linux/falloc.h>
 #include <linux/fadvise.h>
 #include <linux/sched.h>
+#include <linux/sched/mm.h>
 #include <linux/ksm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
@@ -986,6 +987,18 @@ madvise_behavior_valid(int behavior)
 	}
 }
 
+static bool
+process_madvise_behavior_valid(int behavior)
+{
+	switch (behavior) {
+	case MADV_COLD:
+	case MADV_PAGEOUT:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * The madvise(2) system call.
  *
@@ -1033,6 +1046,11 @@ madvise_behavior_valid(int behavior)
  *  MADV_DONTDUMP - the application wants to prevent pages in the given range
  *		from being included in its core dump.
  *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
+ *  MADV_COLD - the application uses the memory less so the kernel can
+ *		deactivate the memory to evict them quickly when the memory
+ *		pressure happen.
+ *  MADV_PAGEOUT - the application uses the memroy very rarely so kernel can
+ *		page out the memory instantly.
  *
  * return values:
  *  zero    - success
@@ -1150,3 +1168,49 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 {
 	return do_madvise(current, current->mm, start, len_in, behavior);
 }
+
+SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
+		size_t, len_in, int, behavior, unsigned long, flags)
+{
+	int ret;
+	struct fd f;
+	struct pid *pid;
+	struct task_struct *task;
+	struct mm_struct *mm;
+
+	if (flags != 0)
+		return -EINVAL;
+
+	if (!process_madvise_behavior_valid(behavior))
+		return -EINVAL;
+
+	f = fdget(pidfd);
+	if (!f.file)
+		return -EBADF;
+
+	pid = pidfd_pid(f.file);
+	if (IS_ERR(pid)) {
+		ret = PTR_ERR(pid);
+		goto fdput;
+	}
+
+	task = get_pid_task(pid, PIDTYPE_PID);
+	if (!task) {
+		ret = -ESRCH;
+		goto fdput;
+	}
+
+	mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
+	if (IS_ERR_OR_NULL(mm)) {
+		ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
+		goto release_task;
+	}
+
+	ret = do_madvise(task, mm, start, len_in, behavior);
+	mmput(mm);
+release_task:
+	put_task_struct(task);
+fdput:
+	fdput(f);
+	return ret;
+}
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 2/7] mm: introduce external memory hinting API

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5021 bytes --]

Hi Minchan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on m68k/for-next]
[also build test WARNING on powerpc/next s390/features linus/master v5.6-rc2 next-20200220]
[cannot apply to arm64/for-next/core tip/x86/asm arm/for-next hp-parisc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/introduce-memory-hinting-API-for-external-process/20200220-225155
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
config: nds32-randconfig-a001-20200220 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/nds32/include/uapi/asm/unistd.h:10,
                    from arch/nds32/include/asm/unistd.h:6,
                    from arch/nds32/kernel/vdso/sigreturn.S:6:
>> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
     858 | #define __NR_pidfd_getfd 439
         | 
   include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
     856 | #define __NR_pidfd_getfd 438
         | 
--
   In file included from arch/nds32/include/uapi/asm/unistd.h:10,
                    from arch/nds32/include/asm/unistd.h:6,
                    from <stdin>:2:
>> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
     858 | #define __NR_pidfd_getfd 439
         | 
   include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
     856 | #define __NR_pidfd_getfd 438
         | 
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
>> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
--
   In file included from arch/nds32/include/uapi/asm/unistd.h:10,
                    from arch/nds32/include/asm/unistd.h:6,
                    from <stdin>:2:
>> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
     858 | #define __NR_pidfd_getfd 439
         | 
   include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
     856 | #define __NR_pidfd_getfd 438
         | 
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
>> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
   16 real  2 user  4 sys  39.01% cpu 	make modules_prepare
--
   In file included from arch/nds32/include/uapi/asm/unistd.h:10,
                    from arch/nds32/include/asm/unistd.h:6,
                    from <stdin>:2:
>> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
     858 | #define __NR_pidfd_getfd 439
         | 
   include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
     856 | #define __NR_pidfd_getfd 438
         | 
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
>> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
   In file included from arch/nds32/include/uapi/asm/unistd.h:10,
                    from arch/nds32/include/asm/unistd.h:6,
                    from arch/nds32/kernel/vdso/sigreturn.S:6:
>> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
     858 | #define __NR_pidfd_getfd 439
         | 
   include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
     856 | #define __NR_pidfd_getfd 438
         | 
   In file included from arch/nds32/include/uapi/asm/unistd.h:10,
                    from arch/nds32/include/asm/unistd.h:6,
                    from arch/nds32/kernel/vdso/gettimeofday.c:11:
>> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
     858 | #define __NR_pidfd_getfd 439
         | 
   include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
     856 | #define __NR_pidfd_getfd 438
         | 
   17 real  4 user  6 sys  59.28% cpu 	make prepare

vim +/__NR_pidfd_getfd +858 include/uapi/asm-generic/unistd.h

   853	
   854	#define __NR_openat2 437
   855	__SYSCALL(__NR_openat2, sys_openat2)
   856	#define __NR_pidfd_getfd 438
   857	__SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
 > 858	#define __NR_pidfd_getfd 439
   859	__SYSCALL(__NR_process_madvise, sys_process_madvise)
   860	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24985 bytes --]

Re: [PATCH v6 2/7] mm: introduce external memory hinting API

[Minchan Kim]

On Fri, Feb 21, 2020 at 03:13:49AM +0800, kbuild test robot wrote:
> Hi Minchan,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on m68k/for-next]
> [also build test WARNING on powerpc/next s390/features linus/master v5.6-rc2 next-20200220]
> [cannot apply to arm64/for-next/core tip/x86/asm arm/for-next hp-parisc/for-next]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> 
> url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/introduce-memory-hinting-API-for-external-process/20200220-225155
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
> config: nds32-randconfig-a001-20200220 (attached as .config)
> compiler: nds32le-linux-gcc (GCC) 9.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=9.2.0 make.cross ARCH=nds32 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
>                     from arch/nds32/include/asm/unistd.h:6,
>                     from arch/nds32/kernel/vdso/sigreturn.S:6:
> >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
>      858 | #define __NR_pidfd_getfd 439
>          | 
>    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
>      856 | #define __NR_pidfd_getfd 438
>          | 
> --
>    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
>                     from arch/nds32/include/asm/unistd.h:6,
>                     from <stdin>:2:
> >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
>      858 | #define __NR_pidfd_getfd 439
>          | 
>    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
>      856 | #define __NR_pidfd_getfd 438
>          | 
>    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> --
>    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
>                     from arch/nds32/include/asm/unistd.h:6,
>                     from <stdin>:2:
> >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
>      858 | #define __NR_pidfd_getfd 439
>          | 
>    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
>      856 | #define __NR_pidfd_getfd 438
>          | 
>    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
>    16 real  2 user  4 sys  39.01% cpu 	make modules_prepare
> --
>    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
>                     from arch/nds32/include/asm/unistd.h:6,
>                     from <stdin>:2:
> >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
>      858 | #define __NR_pidfd_getfd 439
>          | 
>    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
>      856 | #define __NR_pidfd_getfd 438
>          | 
>    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
>    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
>                     from arch/nds32/include/asm/unistd.h:6,
>                     from arch/nds32/kernel/vdso/sigreturn.S:6:
> >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
>      858 | #define __NR_pidfd_getfd 439
>          | 
>    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
>      856 | #define __NR_pidfd_getfd 438
>          | 
>    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
>                     from arch/nds32/include/asm/unistd.h:6,
>                     from arch/nds32/kernel/vdso/gettimeofday.c:11:
> >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
>      858 | #define __NR_pidfd_getfd 439
>          | 
>    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
>      856 | #define __NR_pidfd_getfd 438
>          | 
>    17 real  4 user  6 sys  59.28% cpu 	make prepare
> 
> vim +/__NR_pidfd_getfd +858 include/uapi/asm-generic/unistd.h
> 
>    853	
>    854	#define __NR_openat2 437
>    855	__SYSCALL(__NR_openat2, sys_openat2)
>    856	#define __NR_pidfd_getfd 438
>    857	__SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
>  > 858	#define __NR_pidfd_getfd 439
>    859	__SYSCALL(__NR_process_madvise, sys_process_madvise)
>    860	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


Hi 0-Day,

Thanks for catching this. Here the fix goes.

From ff74a830277d880716fa0f69c80e9ec9337303d5 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan@kernel.org>
Date: Fri, 14 Feb 2020 07:42:03 -0800
Subject: [PATCH v6 2/8] mm: introduce external memory hinting API

There is usecase that System Management Software(SMS) want to give
a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
in the case of Android, it is the ActivityManagerService.

It's similar in spirit to madvise(MADV_WONTNEED), but the information
required to make the reclaim decision is not known to the app. Instead,
it is known to the centralized userspace daemon(ActivityManagerService),
and that daemon must be able to initiate reclaim on its own without
any app involvement.

To solve the issue, this patch introduces a new syscall process_madvise(2).
It uses pidfd of an external process to give the hint.

 int process_madvise(int pidfd, void *addr, size_t length, int advise,
			unsigned long flag);

Since it could affect other process's address range, only privileged
process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
gives it the right to ptrace the process could use it successfully.
The flag argument is reserved for future use if we need to extend the
API.

I think supporting all hints madvise has/will supported/support to
process_madvise is rather risky. Because we are not sure all hints make
sense from external process and implementation for the hint may rely on
the caller being in the current context so it could be error-prone.
Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.

If someone want to add other hints, we could hear hear the usecase and
review it for each hint. It's safer for maintenance rather than
introducing a buggy syscall but hard to fix it later.

Q.1 - Why does any external entity have better knowledge?

Quote from Sandeep
"For Android, every application (including the special SystemServer) are forked
from Zygote. The reason of course is to share as many libraries and classes between
the two as possible to benefit from the preloading during boot.

After applications start, (almost) all of the APIs  end up calling into this
SystemServer process over IPC (binder) and back to the application.

In a fully running system, the SystemServer monitors every single process
periodically to calculate their PSS / RSS and also decides which process is
"important" to the user for interactivity.

So, because of how these processes start _and_ the fact that the SystemServer
is looping to monitor each process, it does tend to *know* which address
range of the application is not used / useful.

Besides, we can never rely on applications to clean things up themselves.
We've had the "hey app1, the system is low on memory, please trim your
memory usage down" notifications for a long time[1]. They rely on
applications honoring the broadcasts and very few do.

So, if we want to avoid the inevitable killing of the application and
restarting it, some way to be able to tell the OS about unimportant memory in
these applications will be useful.

- ssp

Q.2 - How to guarantee the race(i.e., object validation) between when giving a
hint from an external process and get the hint from the target process?

process_madvise operates on the target process's address space as it exists
at the instant that process_madvise is called. If the space target process
can run between the time the process_madvise process inspects the target
process address space and the time that process_madvise is actually called,
process_madvise may operate on memory regions that the calling process does
not expect. It's the responsibility of the process calling process_madvise
to close this race condition. For example, the calling process can suspend
the target process with ptrace, SIGSTOP, or the freezer cgroup so that it
doesn't have an opportunity to change its own address space before
process_madvise is called. Another option is to operate on memory regions
that the caller knows a priori will be unchanged in the target process.
Yet another option is to accept the race for certain process_madvise calls
after reasoning that mistargeting will do no harm. The suggested API itself
does not provide synchronization. It also apply other APIs like move_pages,
process_vm_write.

The race isn't really a problem though. Why is it so wrong to require
that callers do their own synchronization in some manner? Nobody objects
to write(2) merely because it's possible for two processes to open the same
file and clobber each other's writes --- instead, we tell people to use
flock or something. Think about mmap. It never guarantees newly allocated
address space is still valid when the user tries to access it because other
threads could unmap the memory right before. That's where we need
synchronization by using other API or design from userside. It shouldn't
be part of API itself. If someone needs more fine-grained synchronization
rather than process level, there were two ideas suggested - cookie[2] and
anon-fd[3]. Both are applicable via using last reserved argument of the API
but I don't think it's necessary right now since we have already ways to
prevent the race so don't want to add additional complexity with more
fine-grained optimization model.

To make the API extend, it reserved an unsigned long as last argument
so we could support it in future if someone really needs it.

Q.3 - Why doesn't ptrace work?

Injecting an madvise in the target process using ptrace would not work for us
because such injected madvise would have to be executed by the target process,
which means that process would have to be runnable and that creates the risk
of the abovementioned race and hinting a wrong VMA. Furthermore, we want to
act the hint in caller's context, not calle because calle is usually limited
in cpuset/cgroups or even freezed state so they can't act by themselves
quick enough, which causes more thrashing/kill. It doesn't work if the
target process are ptraced(e.g., strace, debugger, minidump) because  a
process can have at most one ptracer.

[1] https://developer.android.com/topic/performance/memory"
[2] process_getinfo for getting the cookie which is updated whenever
    vma of process address layout are changed - Daniel Colascione
- https://lore.kernel.org/lkml/20190520035254.57579-1-minchan@kernel.org/T/#m7694416fd179b2066a2c62b5b139b14e3894e224
[3] anonymous fd which is used for the object(i.e., address range)
    validation - Michal Hocko
- https://lore.kernel.org/lkml/20200120112722.GY18451@dhcp22.suse.cz/

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 arch/alpha/kernel/syscalls/syscall.tbl      |  1 +
 arch/arm/tools/syscall.tbl                  |  1 +
 arch/arm64/include/asm/unistd.h             |  2 +-
 arch/arm64/include/asm/unistd32.h           |  2 +
 arch/ia64/kernel/syscalls/syscall.tbl       |  1 +
 arch/m68k/kernel/syscalls/syscall.tbl       |  1 +
 arch/microblaze/kernel/syscalls/syscall.tbl |  1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |  1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |  1 +
 arch/parisc/kernel/syscalls/syscall.tbl     |  1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |  1 +
 arch/s390/kernel/syscalls/syscall.tbl       |  1 +
 arch/sh/kernel/syscalls/syscall.tbl         |  1 +
 arch/sparc/kernel/syscalls/syscall.tbl      |  1 +
 arch/x86/entry/syscalls/syscall_32.tbl      |  1 +
 arch/x86/entry/syscalls/syscall_64.tbl      |  1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |  1 +
 include/linux/syscalls.h                    |  2 +
 include/uapi/asm-generic/unistd.h           |  4 +-
 kernel/sys_ni.c                             |  1 +
 mm/madvise.c                                | 64 +++++++++++++++++++++
 21 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 36d42da7466a..c82952e6fb80 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -477,3 +477,4 @@
 # 545 reserved for clone3
 547	common	openat2				sys_openat2
 548	common	pidfd_getfd			sys_pidfd_getfd
+549	common	process_madvise			sys_process_madvise
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 4d1cf74a2caa..54c2719fec46 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -451,3 +451,4 @@
 435	common	clone3				sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 1dd22da1c3a9..75f04a1023be 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -38,7 +38,7 @@
 #define __ARM_NR_compat_set_tls		(__ARM_NR_COMPAT_BASE + 5)
 #define __ARM_NR_COMPAT_END		(__ARM_NR_COMPAT_BASE + 0x800)
 
-#define __NR_compat_syscalls		439
+#define __NR_compat_syscalls		440
 #endif
 
 #define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index c1c61635f89c..2a27be7a1f91 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -883,6 +883,8 @@ __SYSCALL(__NR_clone3, sys_clone3)
 __SYSCALL(__NR_openat2, sys_openat2)
 #define __NR_pidfd_getfd 438
 __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
+#define __NR_process_madvise 439
+__SYSCALL(__NR_process_madvise, process_madvise)
 
 /*
  * Please add new compat syscalls above this comment and update
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index 042911e670b8..9524af1c318c 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -358,3 +358,4 @@
 # 435 reserved for clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index f4f49fcb76d0..8197050c097c 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -437,3 +437,4 @@
 435	common	clone3				__sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 4c67b11f9c9e..c5b6c8afe445 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -443,3 +443,4 @@
 435	common	clone3				sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 1f9e8ad636cc..8ec8c558aa9c 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -376,3 +376,4 @@
 435	n32	clone3				__sys_clone3
 437	n32	openat2				sys_openat2
 438	n32	pidfd_getfd			sys_pidfd_getfd
+439	n32	process_madvise			sys_process_madvise
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index c0b9d802dbf6..0078f891bb92 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -352,3 +352,4 @@
 435	n64	clone3				__sys_clone3
 437	n64	openat2				sys_openat2
 438	n64	pidfd_getfd			sys_pidfd_getfd
+439	n64	process_madvise			sys_process_madvise
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 52a15f5cd130..09c3b5dc6855 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -435,3 +435,4 @@
 435	common	clone3				sys_clone3_wrapper
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 35b61bfc1b1a..97eac48c2937 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -519,3 +519,4 @@
 435	nospu	clone3				ppc_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index bd7bd3581a0f..8dc8bfd958ea 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 435  common	clone3			sys_clone3			sys_clone3
 437  common	openat2			sys_openat2			sys_openat2
 438  common	pidfd_getfd		sys_pidfd_getfd			sys_pidfd_getfd
+439  common	process_madvise		sys_process_madvise		sys_process_madvise
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c7a30fcd135f..e69d98040777 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 # 435 reserved for clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index f13615ecdecc..6f6e66dd51f9 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -483,3 +483,4 @@
 # 435 reserved for clone3
 437	common	openat2			sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index c17cb77eb150..1b2184549e27 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -442,3 +442,4 @@
 435	i386	clone3			sys_clone3			__ia32_sys_clone3
 437	i386	openat2			sys_openat2			__ia32_sys_openat2
 438	i386	pidfd_getfd		sys_pidfd_getfd			__ia32_sys_pidfd_getfd
+439	i386	process_madvise		sys_process_madvise		__ia32_sys_process_madvise
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 44d510bc9b78..82d60eb1e00d 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -359,6 +359,7 @@
 435	common	clone3			__x64_sys_clone3/ptregs
 437	common	openat2			__x64_sys_openat2
 438	common	pidfd_getfd		__x64_sys_pidfd_getfd
+439	common	process_madvise		__x64_sys_process_madvise
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 85a9ab1bc04d..165cae047770 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -408,3 +408,4 @@
 435	common	clone3				sys_clone3
 437	common	openat2				sys_openat2
 438	common	pidfd_getfd			sys_pidfd_getfd
+439	common	process_madvise			sys_process_madvise
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 1815065d52f3..e4cd2c2f8bb4 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -876,6 +876,8 @@ asmlinkage long sys_munlockall(void);
 asmlinkage long sys_mincore(unsigned long start, size_t len,
 				unsigned char __user * vec);
 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
+asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
+			size_t len, int behavior, unsigned long flags);
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
 			unsigned long prot, unsigned long pgoff,
 			unsigned long flags);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 3a3201e4618e..fa289b91410e 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -855,9 +855,11 @@ __SYSCALL(__NR_clone3, sys_clone3)
 __SYSCALL(__NR_openat2, sys_openat2)
 #define __NR_pidfd_getfd 438
 __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
+#define __NR_process_madvise 439
+__SYSCALL(__NR_process_madvise, sys_process_madvise)
 
 #undef __NR_syscalls
-#define __NR_syscalls 439
+#define __NR_syscalls 440
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 3b69a560a7ac..6c7332776e8e 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -280,6 +280,7 @@ COND_SYSCALL(mlockall);
 COND_SYSCALL(munlockall);
 COND_SYSCALL(mincore);
 COND_SYSCALL(madvise);
+COND_SYSCALL(process_madvise);
 COND_SYSCALL(remap_file_pages);
 COND_SYSCALL(mbind);
 COND_SYSCALL_COMPAT(mbind);
diff --git a/mm/madvise.c b/mm/madvise.c
index f75c86b6c463..f29155b8185d 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -17,6 +17,7 @@
 #include <linux/falloc.h>
 #include <linux/fadvise.h>
 #include <linux/sched.h>
+#include <linux/sched/mm.h>
 #include <linux/ksm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
@@ -986,6 +987,18 @@ madvise_behavior_valid(int behavior)
 	}
 }
 
+static bool
+process_madvise_behavior_valid(int behavior)
+{
+	switch (behavior) {
+	case MADV_COLD:
+	case MADV_PAGEOUT:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * The madvise(2) system call.
  *
@@ -1033,6 +1046,11 @@ madvise_behavior_valid(int behavior)
  *  MADV_DONTDUMP - the application wants to prevent pages in the given range
  *		from being included in its core dump.
  *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
+ *  MADV_COLD - the application uses the memory less so the kernel can
+ *		deactivate the memory to evict them quickly when the memory
+ *		pressure happen.
+ *  MADV_PAGEOUT - the application uses the memroy very rarely so kernel can
+ *		page out the memory instantly.
  *
  * return values:
  *  zero    - success
@@ -1150,3 +1168,49 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 {
 	return do_madvise(current, current->mm, start, len_in, behavior);
 }
+
+SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
+		size_t, len_in, int, behavior, unsigned long, flags)
+{
+	int ret;
+	struct fd f;
+	struct pid *pid;
+	struct task_struct *task;
+	struct mm_struct *mm;
+
+	if (flags != 0)
+		return -EINVAL;
+
+	if (!process_madvise_behavior_valid(behavior))
+		return -EINVAL;
+
+	f = fdget(pidfd);
+	if (!f.file)
+		return -EBADF;
+
+	pid = pidfd_pid(f.file);
+	if (IS_ERR(pid)) {
+		ret = PTR_ERR(pid);
+		goto fdput;
+	}
+
+	task = get_pid_task(pid, PIDTYPE_PID);
+	if (!task) {
+		ret = -ESRCH;
+		goto fdput;
+	}
+
+	mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
+	if (IS_ERR_OR_NULL(mm)) {
+		ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
+		goto release_task;
+	}
+
+	ret = do_madvise(task, mm, start, len_in, behavior);
+	mmput(mm);
+release_task:
+	put_task_struct(task);
+fdput:
+	fdput(f);
+	return ret;
+}
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 2/7] mm: introduce external memory hinting API

[Minchan Kim]

Hi Andrew,

I submit the fix with inlining here thread but if you prefer submitting
new v7 revision with more inputs, please tell me.
I am happy to resend whole patchset. 

Thanks.

On Thu, Feb 20, 2020 at 01:15:10PM -0800, Minchan Kim wrote:
> On Fri, Feb 21, 2020 at 03:13:49AM +0800, kbuild test robot wrote:
> > Hi Minchan,
> > 
> > I love your patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on m68k/for-next]
> > [also build test WARNING on powerpc/next s390/features linus/master v5.6-rc2 next-20200220]
> > [cannot apply to arm64/for-next/core tip/x86/asm arm/for-next hp-parisc/for-next]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > 
> > url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/introduce-memory-hinting-API-for-external-process/20200220-225155
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
> > config: nds32-randconfig-a001-20200220 (attached as .config)
> > compiler: nds32le-linux-gcc (GCC) 9.2.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=9.2.0 make.cross ARCH=nds32 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> >                     from arch/nds32/include/asm/unistd.h:6,
> >                     from arch/nds32/kernel/vdso/sigreturn.S:6:
> > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> >      858 | #define __NR_pidfd_getfd 439
> >          | 
> >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> >      856 | #define __NR_pidfd_getfd 438
> >          | 
> > --
> >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> >                     from arch/nds32/include/asm/unistd.h:6,
> >                     from <stdin>:2:
> > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> >      858 | #define __NR_pidfd_getfd 439
> >          | 
> >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> >      856 | #define __NR_pidfd_getfd 438
> >          | 
> >    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> > >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> > --
> >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> >                     from arch/nds32/include/asm/unistd.h:6,
> >                     from <stdin>:2:
> > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> >      858 | #define __NR_pidfd_getfd 439
> >          | 
> >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> >      856 | #define __NR_pidfd_getfd 438
> >          | 
> >    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> > >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> >    16 real  2 user  4 sys  39.01% cpu 	make modules_prepare
> > --
> >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> >                     from arch/nds32/include/asm/unistd.h:6,
> >                     from <stdin>:2:
> > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> >      858 | #define __NR_pidfd_getfd 439
> >          | 
> >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> >      856 | #define __NR_pidfd_getfd 438
> >          | 
> >    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> > >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> >                     from arch/nds32/include/asm/unistd.h:6,
> >                     from arch/nds32/kernel/vdso/sigreturn.S:6:
> > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> >      858 | #define __NR_pidfd_getfd 439
> >          | 
> >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> >      856 | #define __NR_pidfd_getfd 438
> >          | 
> >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> >                     from arch/nds32/include/asm/unistd.h:6,
> >                     from arch/nds32/kernel/vdso/gettimeofday.c:11:
> > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> >      858 | #define __NR_pidfd_getfd 439
> >          | 
> >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> >      856 | #define __NR_pidfd_getfd 438
> >          | 
> >    17 real  4 user  6 sys  59.28% cpu 	make prepare
> > 
> > vim +/__NR_pidfd_getfd +858 include/uapi/asm-generic/unistd.h
> > 
> >    853	
> >    854	#define __NR_openat2 437
> >    855	__SYSCALL(__NR_openat2, sys_openat2)
> >    856	#define __NR_pidfd_getfd 438
> >    857	__SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
> >  > 858	#define __NR_pidfd_getfd 439
> >    859	__SYSCALL(__NR_process_madvise, sys_process_madvise)
> >    860	
> > 
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 
> 
> Hi 0-Day,
> 
> Thanks for catching this. Here the fix goes.
> 
> From ff74a830277d880716fa0f69c80e9ec9337303d5 Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan@kernel.org>
> Date: Fri, 14 Feb 2020 07:42:03 -0800
> Subject: [PATCH v6 2/8] mm: introduce external memory hinting API
> 
> There is usecase that System Management Software(SMS) want to give
> a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
> in the case of Android, it is the ActivityManagerService.
> 
> It's similar in spirit to madvise(MADV_WONTNEED), but the information
> required to make the reclaim decision is not known to the app. Instead,
> it is known to the centralized userspace daemon(ActivityManagerService),
> and that daemon must be able to initiate reclaim on its own without
> any app involvement.
> 
> To solve the issue, this patch introduces a new syscall process_madvise(2).
> It uses pidfd of an external process to give the hint.
> 
>  int process_madvise(int pidfd, void *addr, size_t length, int advise,
> 			unsigned long flag);
> 
> Since it could affect other process's address range, only privileged
> process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
> gives it the right to ptrace the process could use it successfully.
> The flag argument is reserved for future use if we need to extend the
> API.
> 
> I think supporting all hints madvise has/will supported/support to
> process_madvise is rather risky. Because we are not sure all hints make
> sense from external process and implementation for the hint may rely on
> the caller being in the current context so it could be error-prone.
> Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
> 
> If someone want to add other hints, we could hear hear the usecase and
> review it for each hint. It's safer for maintenance rather than
> introducing a buggy syscall but hard to fix it later.
> 
> Q.1 - Why does any external entity have better knowledge?
> 
> Quote from Sandeep
> "For Android, every application (including the special SystemServer) are forked
> from Zygote. The reason of course is to share as many libraries and classes between
> the two as possible to benefit from the preloading during boot.
> 
> After applications start, (almost) all of the APIs  end up calling into this
> SystemServer process over IPC (binder) and back to the application.
> 
> In a fully running system, the SystemServer monitors every single process
> periodically to calculate their PSS / RSS and also decides which process is
> "important" to the user for interactivity.
> 
> So, because of how these processes start _and_ the fact that the SystemServer
> is looping to monitor each process, it does tend to *know* which address
> range of the application is not used / useful.
> 
> Besides, we can never rely on applications to clean things up themselves.
> We've had the "hey app1, the system is low on memory, please trim your
> memory usage down" notifications for a long time[1]. They rely on
> applications honoring the broadcasts and very few do.
> 
> So, if we want to avoid the inevitable killing of the application and
> restarting it, some way to be able to tell the OS about unimportant memory in
> these applications will be useful.
> 
> - ssp
> 
> Q.2 - How to guarantee the race(i.e., object validation) between when giving a
> hint from an external process and get the hint from the target process?
> 
> process_madvise operates on the target process's address space as it exists
> at the instant that process_madvise is called. If the space target process
> can run between the time the process_madvise process inspects the target
> process address space and the time that process_madvise is actually called,
> process_madvise may operate on memory regions that the calling process does
> not expect. It's the responsibility of the process calling process_madvise
> to close this race condition. For example, the calling process can suspend
> the target process with ptrace, SIGSTOP, or the freezer cgroup so that it
> doesn't have an opportunity to change its own address space before
> process_madvise is called. Another option is to operate on memory regions
> that the caller knows a priori will be unchanged in the target process.
> Yet another option is to accept the race for certain process_madvise calls
> after reasoning that mistargeting will do no harm. The suggested API itself
> does not provide synchronization. It also apply other APIs like move_pages,
> process_vm_write.
> 
> The race isn't really a problem though. Why is it so wrong to require
> that callers do their own synchronization in some manner? Nobody objects
> to write(2) merely because it's possible for two processes to open the same
> file and clobber each other's writes --- instead, we tell people to use
> flock or something. Think about mmap. It never guarantees newly allocated
> address space is still valid when the user tries to access it because other
> threads could unmap the memory right before. That's where we need
> synchronization by using other API or design from userside. It shouldn't
> be part of API itself. If someone needs more fine-grained synchronization
> rather than process level, there were two ideas suggested - cookie[2] and
> anon-fd[3]. Both are applicable via using last reserved argument of the API
> but I don't think it's necessary right now since we have already ways to
> prevent the race so don't want to add additional complexity with more
> fine-grained optimization model.
> 
> To make the API extend, it reserved an unsigned long as last argument
> so we could support it in future if someone really needs it.
> 
> Q.3 - Why doesn't ptrace work?
> 
> Injecting an madvise in the target process using ptrace would not work for us
> because such injected madvise would have to be executed by the target process,
> which means that process would have to be runnable and that creates the risk
> of the abovementioned race and hinting a wrong VMA. Furthermore, we want to
> act the hint in caller's context, not calle because calle is usually limited
> in cpuset/cgroups or even freezed state so they can't act by themselves
> quick enough, which causes more thrashing/kill. It doesn't work if the
> target process are ptraced(e.g., strace, debugger, minidump) because  a
> process can have at most one ptracer.
> 
> [1] https://developer.android.com/topic/performance/memory"
> [2] process_getinfo for getting the cookie which is updated whenever
>     vma of process address layout are changed - Daniel Colascione
> - https://lore.kernel.org/lkml/20190520035254.57579-1-minchan@kernel.org/T/#m7694416fd179b2066a2c62b5b139b14e3894e224
> [3] anonymous fd which is used for the object(i.e., address range)
>     validation - Michal Hocko
> - https://lore.kernel.org/lkml/20200120112722.GY18451@dhcp22.suse.cz/
> 
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  arch/alpha/kernel/syscalls/syscall.tbl      |  1 +
>  arch/arm/tools/syscall.tbl                  |  1 +
>  arch/arm64/include/asm/unistd.h             |  2 +-
>  arch/arm64/include/asm/unistd32.h           |  2 +
>  arch/ia64/kernel/syscalls/syscall.tbl       |  1 +
>  arch/m68k/kernel/syscalls/syscall.tbl       |  1 +
>  arch/microblaze/kernel/syscalls/syscall.tbl |  1 +
>  arch/mips/kernel/syscalls/syscall_n32.tbl   |  1 +
>  arch/mips/kernel/syscalls/syscall_n64.tbl   |  1 +
>  arch/parisc/kernel/syscalls/syscall.tbl     |  1 +
>  arch/powerpc/kernel/syscalls/syscall.tbl    |  1 +
>  arch/s390/kernel/syscalls/syscall.tbl       |  1 +
>  arch/sh/kernel/syscalls/syscall.tbl         |  1 +
>  arch/sparc/kernel/syscalls/syscall.tbl      |  1 +
>  arch/x86/entry/syscalls/syscall_32.tbl      |  1 +
>  arch/x86/entry/syscalls/syscall_64.tbl      |  1 +
>  arch/xtensa/kernel/syscalls/syscall.tbl     |  1 +
>  include/linux/syscalls.h                    |  2 +
>  include/uapi/asm-generic/unistd.h           |  4 +-
>  kernel/sys_ni.c                             |  1 +
>  mm/madvise.c                                | 64 +++++++++++++++++++++
>  21 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> index 36d42da7466a..c82952e6fb80 100644
> --- a/arch/alpha/kernel/syscalls/syscall.tbl
> +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> @@ -477,3 +477,4 @@
>  # 545 reserved for clone3
>  547	common	openat2				sys_openat2
>  548	common	pidfd_getfd			sys_pidfd_getfd
> +549	common	process_madvise			sys_process_madvise
> diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> index 4d1cf74a2caa..54c2719fec46 100644
> --- a/arch/arm/tools/syscall.tbl
> +++ b/arch/arm/tools/syscall.tbl
> @@ -451,3 +451,4 @@
>  435	common	clone3				sys_clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
> index 1dd22da1c3a9..75f04a1023be 100644
> --- a/arch/arm64/include/asm/unistd.h
> +++ b/arch/arm64/include/asm/unistd.h
> @@ -38,7 +38,7 @@
>  #define __ARM_NR_compat_set_tls		(__ARM_NR_COMPAT_BASE + 5)
>  #define __ARM_NR_COMPAT_END		(__ARM_NR_COMPAT_BASE + 0x800)
>  
> -#define __NR_compat_syscalls		439
> +#define __NR_compat_syscalls		440
>  #endif
>  
>  #define __ARCH_WANT_SYS_CLONE
> diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
> index c1c61635f89c..2a27be7a1f91 100644
> --- a/arch/arm64/include/asm/unistd32.h
> +++ b/arch/arm64/include/asm/unistd32.h
> @@ -883,6 +883,8 @@ __SYSCALL(__NR_clone3, sys_clone3)
>  __SYSCALL(__NR_openat2, sys_openat2)
>  #define __NR_pidfd_getfd 438
>  __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
> +#define __NR_process_madvise 439
> +__SYSCALL(__NR_process_madvise, process_madvise)
>  
>  /*
>   * Please add new compat syscalls above this comment and update
> diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
> index 042911e670b8..9524af1c318c 100644
> --- a/arch/ia64/kernel/syscalls/syscall.tbl
> +++ b/arch/ia64/kernel/syscalls/syscall.tbl
> @@ -358,3 +358,4 @@
>  # 435 reserved for clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
> index f4f49fcb76d0..8197050c097c 100644
> --- a/arch/m68k/kernel/syscalls/syscall.tbl
> +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> @@ -437,3 +437,4 @@
>  435	common	clone3				__sys_clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
> index 4c67b11f9c9e..c5b6c8afe445 100644
> --- a/arch/microblaze/kernel/syscalls/syscall.tbl
> +++ b/arch/microblaze/kernel/syscalls/syscall.tbl
> @@ -443,3 +443,4 @@
>  435	common	clone3				sys_clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
> index 1f9e8ad636cc..8ec8c558aa9c 100644
> --- a/arch/mips/kernel/syscalls/syscall_n32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
> @@ -376,3 +376,4 @@
>  435	n32	clone3				__sys_clone3
>  437	n32	openat2				sys_openat2
>  438	n32	pidfd_getfd			sys_pidfd_getfd
> +439	n32	process_madvise			sys_process_madvise
> diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
> index c0b9d802dbf6..0078f891bb92 100644
> --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> @@ -352,3 +352,4 @@
>  435	n64	clone3				__sys_clone3
>  437	n64	openat2				sys_openat2
>  438	n64	pidfd_getfd			sys_pidfd_getfd
> +439	n64	process_madvise			sys_process_madvise
> diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
> index 52a15f5cd130..09c3b5dc6855 100644
> --- a/arch/parisc/kernel/syscalls/syscall.tbl
> +++ b/arch/parisc/kernel/syscalls/syscall.tbl
> @@ -435,3 +435,4 @@
>  435	common	clone3				sys_clone3_wrapper
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index 35b61bfc1b1a..97eac48c2937 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -519,3 +519,4 @@
>  435	nospu	clone3				ppc_clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> index bd7bd3581a0f..8dc8bfd958ea 100644
> --- a/arch/s390/kernel/syscalls/syscall.tbl
> +++ b/arch/s390/kernel/syscalls/syscall.tbl
> @@ -440,3 +440,4 @@
>  435  common	clone3			sys_clone3			sys_clone3
>  437  common	openat2			sys_openat2			sys_openat2
>  438  common	pidfd_getfd		sys_pidfd_getfd			sys_pidfd_getfd
> +439  common	process_madvise		sys_process_madvise		sys_process_madvise
> diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
> index c7a30fcd135f..e69d98040777 100644
> --- a/arch/sh/kernel/syscalls/syscall.tbl
> +++ b/arch/sh/kernel/syscalls/syscall.tbl
> @@ -440,3 +440,4 @@
>  # 435 reserved for clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
> index f13615ecdecc..6f6e66dd51f9 100644
> --- a/arch/sparc/kernel/syscalls/syscall.tbl
> +++ b/arch/sparc/kernel/syscalls/syscall.tbl
> @@ -483,3 +483,4 @@
>  # 435 reserved for clone3
>  437	common	openat2			sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index c17cb77eb150..1b2184549e27 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -442,3 +442,4 @@
>  435	i386	clone3			sys_clone3			__ia32_sys_clone3
>  437	i386	openat2			sys_openat2			__ia32_sys_openat2
>  438	i386	pidfd_getfd		sys_pidfd_getfd			__ia32_sys_pidfd_getfd
> +439	i386	process_madvise		sys_process_madvise		__ia32_sys_process_madvise
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 44d510bc9b78..82d60eb1e00d 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -359,6 +359,7 @@
>  435	common	clone3			__x64_sys_clone3/ptregs
>  437	common	openat2			__x64_sys_openat2
>  438	common	pidfd_getfd		__x64_sys_pidfd_getfd
> +439	common	process_madvise		__x64_sys_process_madvise
>  
>  #
>  # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
> index 85a9ab1bc04d..165cae047770 100644
> --- a/arch/xtensa/kernel/syscalls/syscall.tbl
> +++ b/arch/xtensa/kernel/syscalls/syscall.tbl
> @@ -408,3 +408,4 @@
>  435	common	clone3				sys_clone3
>  437	common	openat2				sys_openat2
>  438	common	pidfd_getfd			sys_pidfd_getfd
> +439	common	process_madvise			sys_process_madvise
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 1815065d52f3..e4cd2c2f8bb4 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -876,6 +876,8 @@ asmlinkage long sys_munlockall(void);
>  asmlinkage long sys_mincore(unsigned long start, size_t len,
>  				unsigned char __user * vec);
>  asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
> +asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
> +			size_t len, int behavior, unsigned long flags);
>  asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
>  			unsigned long prot, unsigned long pgoff,
>  			unsigned long flags);
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 3a3201e4618e..fa289b91410e 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -855,9 +855,11 @@ __SYSCALL(__NR_clone3, sys_clone3)
>  __SYSCALL(__NR_openat2, sys_openat2)
>  #define __NR_pidfd_getfd 438
>  __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
> +#define __NR_process_madvise 439
> +__SYSCALL(__NR_process_madvise, sys_process_madvise)
>  
>  #undef __NR_syscalls
> -#define __NR_syscalls 439
> +#define __NR_syscalls 440
>  
>  /*
>   * 32 bit systems traditionally used different
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 3b69a560a7ac..6c7332776e8e 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -280,6 +280,7 @@ COND_SYSCALL(mlockall);
>  COND_SYSCALL(munlockall);
>  COND_SYSCALL(mincore);
>  COND_SYSCALL(madvise);
> +COND_SYSCALL(process_madvise);
>  COND_SYSCALL(remap_file_pages);
>  COND_SYSCALL(mbind);
>  COND_SYSCALL_COMPAT(mbind);
> diff --git a/mm/madvise.c b/mm/madvise.c
> index f75c86b6c463..f29155b8185d 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -17,6 +17,7 @@
>  #include <linux/falloc.h>
>  #include <linux/fadvise.h>
>  #include <linux/sched.h>
> +#include <linux/sched/mm.h>
>  #include <linux/ksm.h>
>  #include <linux/fs.h>
>  #include <linux/file.h>
> @@ -986,6 +987,18 @@ madvise_behavior_valid(int behavior)
>  	}
>  }
>  
> +static bool
> +process_madvise_behavior_valid(int behavior)
> +{
> +	switch (behavior) {
> +	case MADV_COLD:
> +	case MADV_PAGEOUT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  /*
>   * The madvise(2) system call.
>   *
> @@ -1033,6 +1046,11 @@ madvise_behavior_valid(int behavior)
>   *  MADV_DONTDUMP - the application wants to prevent pages in the given range
>   *		from being included in its core dump.
>   *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
> + *  MADV_COLD - the application uses the memory less so the kernel can
> + *		deactivate the memory to evict them quickly when the memory
> + *		pressure happen.
> + *  MADV_PAGEOUT - the application uses the memroy very rarely so kernel can
> + *		page out the memory instantly.
>   *
>   * return values:
>   *  zero    - success
> @@ -1150,3 +1168,49 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
>  {
>  	return do_madvise(current, current->mm, start, len_in, behavior);
>  }
> +
> +SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> +		size_t, len_in, int, behavior, unsigned long, flags)
> +{
> +	int ret;
> +	struct fd f;
> +	struct pid *pid;
> +	struct task_struct *task;
> +	struct mm_struct *mm;
> +
> +	if (flags != 0)
> +		return -EINVAL;
> +
> +	if (!process_madvise_behavior_valid(behavior))
> +		return -EINVAL;
> +
> +	f = fdget(pidfd);
> +	if (!f.file)
> +		return -EBADF;
> +
> +	pid = pidfd_pid(f.file);
> +	if (IS_ERR(pid)) {
> +		ret = PTR_ERR(pid);
> +		goto fdput;
> +	}
> +
> +	task = get_pid_task(pid, PIDTYPE_PID);
> +	if (!task) {
> +		ret = -ESRCH;
> +		goto fdput;
> +	}
> +
> +	mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> +	if (IS_ERR_OR_NULL(mm)) {
> +		ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
> +		goto release_task;
> +	}
> +
> +	ret = do_madvise(task, mm, start, len_in, behavior);
> +	mmput(mm);
> +release_task:
> +	put_task_struct(task);
> +fdput:
> +	fdput(f);
> +	return ret;
> +}
> -- 
> 2.25.0.265.gbab2e86ba0-goog
> 

Re: [PATCH v6 2/7] mm: introduce external memory hinting API

[Suren Baghdasaryan]

On Thu, Feb 20, 2020 at 1:21 PM Minchan Kim <minchan@kernel.org> wrote:
>
> Hi Andrew,
>
> I submit the fix with inlining here thread but if you prefer submitting
> new v7 revision with more inputs, please tell me.
> I am happy to resend whole patchset.
>
> Thanks.
>
> On Thu, Feb 20, 2020 at 01:15:10PM -0800, Minchan Kim wrote:
> > On Fri, Feb 21, 2020 at 03:13:49AM +0800, kbuild test robot wrote:
> > > Hi Minchan,
> > >
> > > I love your patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on m68k/for-next]
> > > [also build test WARNING on powerpc/next s390/features linus/master v5.6-rc2 next-20200220]
> > > [cannot apply to arm64/for-next/core tip/x86/asm arm/for-next hp-parisc/for-next]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/introduce-memory-hinting-API-for-external-process/20200220-225155
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
> > > config: nds32-randconfig-a001-20200220 (attached as .config)
> > > compiler: nds32le-linux-gcc (GCC) 9.2.0
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         GCC_VERSION=9.2.0 make.cross ARCH=nds32
> > >
> > > If you fix the issue, kindly add following tag
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> > >                     from arch/nds32/include/asm/unistd.h:6,
> > >                     from arch/nds32/kernel/vdso/sigreturn.S:6:
> > > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> > >      858 | #define __NR_pidfd_getfd 439
> > >          |
> > >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> > >      856 | #define __NR_pidfd_getfd 438
> > >          |
> > > --
> > >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> > >                     from arch/nds32/include/asm/unistd.h:6,
> > >                     from <stdin>:2:
> > > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> > >      858 | #define __NR_pidfd_getfd 439
> > >          |
> > >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> > >      856 | #define __NR_pidfd_getfd 438
> > >          |
> > >    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> > > >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> > > --
> > >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> > >                     from arch/nds32/include/asm/unistd.h:6,
> > >                     from <stdin>:2:
> > > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> > >      858 | #define __NR_pidfd_getfd 439
> > >          |
> > >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> > >      856 | #define __NR_pidfd_getfd 438
> > >          |
> > >    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> > > >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> > >    16 real  2 user  4 sys  39.01% cpu       make modules_prepare
> > > --
> > >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> > >                     from arch/nds32/include/asm/unistd.h:6,
> > >                     from <stdin>:2:
> > > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> > >      858 | #define __NR_pidfd_getfd 439
> > >          |
> > >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> > >      856 | #define __NR_pidfd_getfd 438
> > >          |
> > >    <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> > > >> <stdin>:1520:2: warning: #warning syscall process_madvise not implemented [-Wcpp]
> > >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> > >                     from arch/nds32/include/asm/unistd.h:6,
> > >                     from arch/nds32/kernel/vdso/sigreturn.S:6:
> > > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> > >      858 | #define __NR_pidfd_getfd 439
> > >          |
> > >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> > >      856 | #define __NR_pidfd_getfd 438
> > >          |
> > >    In file included from arch/nds32/include/uapi/asm/unistd.h:10,
> > >                     from arch/nds32/include/asm/unistd.h:6,
> > >                     from arch/nds32/kernel/vdso/gettimeofday.c:11:
> > > >> include/uapi/asm-generic/unistd.h:858: warning: "__NR_pidfd_getfd" redefined
> > >      858 | #define __NR_pidfd_getfd 439
> > >          |
> > >    include/uapi/asm-generic/unistd.h:856: note: this is the location of the previous definition
> > >      856 | #define __NR_pidfd_getfd 438
> > >          |
> > >    17 real  4 user  6 sys  59.28% cpu       make prepare
> > >
> > > vim +/__NR_pidfd_getfd +858 include/uapi/asm-generic/unistd.h
> > >
> > >    853
> > >    854      #define __NR_openat2 437
> > >    855      __SYSCALL(__NR_openat2, sys_openat2)
> > >    856      #define __NR_pidfd_getfd 438
> > >    857      __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
> > >  > 858      #define __NR_pidfd_getfd 439
> > >    859      __SYSCALL(__NR_process_madvise, sys_process_madvise)
> > >    860
> > >
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> >
> >
> > Hi 0-Day,
> >
> > Thanks for catching this. Here the fix goes.
> >
> > From ff74a830277d880716fa0f69c80e9ec9337303d5 Mon Sep 17 00:00:00 2001
> > From: Minchan Kim <minchan@kernel.org>
> > Date: Fri, 14 Feb 2020 07:42:03 -0800
> > Subject: [PATCH v6 2/8] mm: introduce external memory hinting API
> >
> > There is usecase that System Management Software(SMS) want to give
> > a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
> > in the case of Android, it is the ActivityManagerService.
> >
> > It's similar in spirit to madvise(MADV_WONTNEED), but the information
> > required to make the reclaim decision is not known to the app. Instead,
> > it is known to the centralized userspace daemon(ActivityManagerService),
> > and that daemon must be able to initiate reclaim on its own without
> > any app involvement.
> >
> > To solve the issue, this patch introduces a new syscall process_madvise(2).
> > It uses pidfd of an external process to give the hint.
> >
> >  int process_madvise(int pidfd, void *addr, size_t length, int advise,
> >                       unsigned long flag);
> >
> > Since it could affect other process's address range, only privileged
> > process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
> > gives it the right to ptrace the process could use it successfully.
> > The flag argument is reserved for future use if we need to extend the
> > API.
> >
> > I think supporting all hints madvise has/will supported/support to
> > process_madvise is rather risky. Because we are not sure all hints make
> > sense from external process and implementation for the hint may rely on
> > the caller being in the current context so it could be error-prone.
> > Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
> >
> > If someone want to add other hints, we could hear hear the usecase and
> > review it for each hint. It's safer for maintenance rather than
> > introducing a buggy syscall but hard to fix it later.
> >
> > Q.1 - Why does any external entity have better knowledge?
> >
> > Quote from Sandeep
> > "For Android, every application (including the special SystemServer) are forked
> > from Zygote. The reason of course is to share as many libraries and classes between
> > the two as possible to benefit from the preloading during boot.
> >
> > After applications start, (almost) all of the APIs  end up calling into this
> > SystemServer process over IPC (binder) and back to the application.
> >
> > In a fully running system, the SystemServer monitors every single process
> > periodically to calculate their PSS / RSS and also decides which process is
> > "important" to the user for interactivity.
> >
> > So, because of how these processes start _and_ the fact that the SystemServer
> > is looping to monitor each process, it does tend to *know* which address
> > range of the application is not used / useful.
> >
> > Besides, we can never rely on applications to clean things up themselves.
> > We've had the "hey app1, the system is low on memory, please trim your
> > memory usage down" notifications for a long time[1]. They rely on
> > applications honoring the broadcasts and very few do.
> >
> > So, if we want to avoid the inevitable killing of the application and
> > restarting it, some way to be able to tell the OS about unimportant memory in
> > these applications will be useful.
> >
> > - ssp
> >
> > Q.2 - How to guarantee the race(i.e., object validation) between when giving a
> > hint from an external process and get the hint from the target process?
> >
> > process_madvise operates on the target process's address space as it exists
> > at the instant that process_madvise is called. If the space target process
> > can run between the time the process_madvise process inspects the target
> > process address space and the time that process_madvise is actually called,
> > process_madvise may operate on memory regions that the calling process does
> > not expect. It's the responsibility of the process calling process_madvise
> > to close this race condition. For example, the calling process can suspend
> > the target process with ptrace, SIGSTOP, or the freezer cgroup so that it
> > doesn't have an opportunity to change its own address space before
> > process_madvise is called. Another option is to operate on memory regions
> > that the caller knows a priori will be unchanged in the target process.
> > Yet another option is to accept the race for certain process_madvise calls
> > after reasoning that mistargeting will do no harm. The suggested API itself
> > does not provide synchronization. It also apply other APIs like move_pages,
> > process_vm_write.
> >
> > The race isn't really a problem though. Why is it so wrong to require
> > that callers do their own synchronization in some manner? Nobody objects
> > to write(2) merely because it's possible for two processes to open the same
> > file and clobber each other's writes --- instead, we tell people to use
> > flock or something. Think about mmap. It never guarantees newly allocated
> > address space is still valid when the user tries to access it because other
> > threads could unmap the memory right before. That's where we need
> > synchronization by using other API or design from userside. It shouldn't
> > be part of API itself. If someone needs more fine-grained synchronization
> > rather than process level, there were two ideas suggested - cookie[2] and
> > anon-fd[3]. Both are applicable via using last reserved argument of the API
> > but I don't think it's necessary right now since we have already ways to
> > prevent the race so don't want to add additional complexity with more
> > fine-grained optimization model.
> >
> > To make the API extend, it reserved an unsigned long as last argument
> > so we could support it in future if someone really needs it.
> >
> > Q.3 - Why doesn't ptrace work?
> >
> > Injecting an madvise in the target process using ptrace would not work for us
> > because such injected madvise would have to be executed by the target process,
> > which means that process would have to be runnable and that creates the risk
> > of the abovementioned race and hinting a wrong VMA. Furthermore, we want to
> > act the hint in caller's context, not calle because calle is usually limited
> > in cpuset/cgroups or even freezed state so they can't act by themselves
> > quick enough, which causes more thrashing/kill. It doesn't work if the
> > target process are ptraced(e.g., strace, debugger, minidump) because  a
> > process can have at most one ptracer.
> >
> > [1] https://developer.android.com/topic/performance/memory"
> > [2] process_getinfo for getting the cookie which is updated whenever
> >     vma of process address layout are changed - Daniel Colascione
> > - https://lore.kernel.org/lkml/20190520035254.57579-1-minchan@kernel.org/T/#m7694416fd179b2066a2c62b5b139b14e3894e224
> > [3] anonymous fd which is used for the object(i.e., address range)
> >     validation - Michal Hocko
> > - https://lore.kernel.org/lkml/20200120112722.GY18451@dhcp22.suse.cz/
> >
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  arch/alpha/kernel/syscalls/syscall.tbl      |  1 +
> >  arch/arm/tools/syscall.tbl                  |  1 +
> >  arch/arm64/include/asm/unistd.h             |  2 +-
> >  arch/arm64/include/asm/unistd32.h           |  2 +
> >  arch/ia64/kernel/syscalls/syscall.tbl       |  1 +
> >  arch/m68k/kernel/syscalls/syscall.tbl       |  1 +
> >  arch/microblaze/kernel/syscalls/syscall.tbl |  1 +
> >  arch/mips/kernel/syscalls/syscall_n32.tbl   |  1 +
> >  arch/mips/kernel/syscalls/syscall_n64.tbl   |  1 +
> >  arch/parisc/kernel/syscalls/syscall.tbl     |  1 +
> >  arch/powerpc/kernel/syscalls/syscall.tbl    |  1 +
> >  arch/s390/kernel/syscalls/syscall.tbl       |  1 +
> >  arch/sh/kernel/syscalls/syscall.tbl         |  1 +
> >  arch/sparc/kernel/syscalls/syscall.tbl      |  1 +
> >  arch/x86/entry/syscalls/syscall_32.tbl      |  1 +
> >  arch/x86/entry/syscalls/syscall_64.tbl      |  1 +
> >  arch/xtensa/kernel/syscalls/syscall.tbl     |  1 +
> >  include/linux/syscalls.h                    |  2 +
> >  include/uapi/asm-generic/unistd.h           |  4 +-
> >  kernel/sys_ni.c                             |  1 +
> >  mm/madvise.c                                | 64 +++++++++++++++++++++
> >  21 files changed, 88 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> > index 36d42da7466a..c82952e6fb80 100644
> > --- a/arch/alpha/kernel/syscalls/syscall.tbl
> > +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> > @@ -477,3 +477,4 @@
> >  # 545 reserved for clone3
> >  547  common  openat2                         sys_openat2
> >  548  common  pidfd_getfd                     sys_pidfd_getfd
> > +549  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> > index 4d1cf74a2caa..54c2719fec46 100644
> > --- a/arch/arm/tools/syscall.tbl
> > +++ b/arch/arm/tools/syscall.tbl
> > @@ -451,3 +451,4 @@
> >  435  common  clone3                          sys_clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
> > index 1dd22da1c3a9..75f04a1023be 100644
> > --- a/arch/arm64/include/asm/unistd.h
> > +++ b/arch/arm64/include/asm/unistd.h
> > @@ -38,7 +38,7 @@
> >  #define __ARM_NR_compat_set_tls              (__ARM_NR_COMPAT_BASE + 5)
> >  #define __ARM_NR_COMPAT_END          (__ARM_NR_COMPAT_BASE + 0x800)
> >
> > -#define __NR_compat_syscalls         439
> > +#define __NR_compat_syscalls         440
> >  #endif
> >
> >  #define __ARCH_WANT_SYS_CLONE
> > diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
> > index c1c61635f89c..2a27be7a1f91 100644
> > --- a/arch/arm64/include/asm/unistd32.h
> > +++ b/arch/arm64/include/asm/unistd32.h
> > @@ -883,6 +883,8 @@ __SYSCALL(__NR_clone3, sys_clone3)
> >  __SYSCALL(__NR_openat2, sys_openat2)
> >  #define __NR_pidfd_getfd 438
> >  __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
> > +#define __NR_process_madvise 439
> > +__SYSCALL(__NR_process_madvise, process_madvise)
> >
> >  /*
> >   * Please add new compat syscalls above this comment and update
> > diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
> > index 042911e670b8..9524af1c318c 100644
> > --- a/arch/ia64/kernel/syscalls/syscall.tbl
> > +++ b/arch/ia64/kernel/syscalls/syscall.tbl
> > @@ -358,3 +358,4 @@
> >  # 435 reserved for clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
> > index f4f49fcb76d0..8197050c097c 100644
> > --- a/arch/m68k/kernel/syscalls/syscall.tbl
> > +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> > @@ -437,3 +437,4 @@
> >  435  common  clone3                          __sys_clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
> > index 4c67b11f9c9e..c5b6c8afe445 100644
> > --- a/arch/microblaze/kernel/syscalls/syscall.tbl
> > +++ b/arch/microblaze/kernel/syscalls/syscall.tbl
> > @@ -443,3 +443,4 @@
> >  435  common  clone3                          sys_clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
> > index 1f9e8ad636cc..8ec8c558aa9c 100644
> > --- a/arch/mips/kernel/syscalls/syscall_n32.tbl
> > +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
> > @@ -376,3 +376,4 @@
> >  435  n32     clone3                          __sys_clone3
> >  437  n32     openat2                         sys_openat2
> >  438  n32     pidfd_getfd                     sys_pidfd_getfd
> > +439  n32     process_madvise                 sys_process_madvise
> > diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
> > index c0b9d802dbf6..0078f891bb92 100644
> > --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> > +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> > @@ -352,3 +352,4 @@
> >  435  n64     clone3                          __sys_clone3
> >  437  n64     openat2                         sys_openat2
> >  438  n64     pidfd_getfd                     sys_pidfd_getfd
> > +439  n64     process_madvise                 sys_process_madvise
> > diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
> > index 52a15f5cd130..09c3b5dc6855 100644
> > --- a/arch/parisc/kernel/syscalls/syscall.tbl
> > +++ b/arch/parisc/kernel/syscalls/syscall.tbl
> > @@ -435,3 +435,4 @@
> >  435  common  clone3                          sys_clone3_wrapper
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> > index 35b61bfc1b1a..97eac48c2937 100644
> > --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> > +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> > @@ -519,3 +519,4 @@
> >  435  nospu   clone3                          ppc_clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> > index bd7bd3581a0f..8dc8bfd958ea 100644
> > --- a/arch/s390/kernel/syscalls/syscall.tbl
> > +++ b/arch/s390/kernel/syscalls/syscall.tbl
> > @@ -440,3 +440,4 @@
> >  435  common  clone3                  sys_clone3                      sys_clone3
> >  437  common  openat2                 sys_openat2                     sys_openat2
> >  438  common  pidfd_getfd             sys_pidfd_getfd                 sys_pidfd_getfd
> > +439  common  process_madvise         sys_process_madvise             sys_process_madvise
> > diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
> > index c7a30fcd135f..e69d98040777 100644
> > --- a/arch/sh/kernel/syscalls/syscall.tbl
> > +++ b/arch/sh/kernel/syscalls/syscall.tbl
> > @@ -440,3 +440,4 @@
> >  # 435 reserved for clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
> > index f13615ecdecc..6f6e66dd51f9 100644
> > --- a/arch/sparc/kernel/syscalls/syscall.tbl
> > +++ b/arch/sparc/kernel/syscalls/syscall.tbl
> > @@ -483,3 +483,4 @@
> >  # 435 reserved for clone3
> >  437  common  openat2                 sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> > index c17cb77eb150..1b2184549e27 100644
> > --- a/arch/x86/entry/syscalls/syscall_32.tbl
> > +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> > @@ -442,3 +442,4 @@
> >  435  i386    clone3                  sys_clone3                      __ia32_sys_clone3
> >  437  i386    openat2                 sys_openat2                     __ia32_sys_openat2
> >  438  i386    pidfd_getfd             sys_pidfd_getfd                 __ia32_sys_pidfd_getfd
> > +439  i386    process_madvise         sys_process_madvise             __ia32_sys_process_madvise
> > diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> > index 44d510bc9b78..82d60eb1e00d 100644
> > --- a/arch/x86/entry/syscalls/syscall_64.tbl
> > +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> > @@ -359,6 +359,7 @@
> >  435  common  clone3                  __x64_sys_clone3/ptregs
> >  437  common  openat2                 __x64_sys_openat2
> >  438  common  pidfd_getfd             __x64_sys_pidfd_getfd
> > +439  common  process_madvise         __x64_sys_process_madvise
> >
> >  #
> >  # x32-specific system call numbers start at 512 to avoid cache impact
> > diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
> > index 85a9ab1bc04d..165cae047770 100644
> > --- a/arch/xtensa/kernel/syscalls/syscall.tbl
> > +++ b/arch/xtensa/kernel/syscalls/syscall.tbl
> > @@ -408,3 +408,4 @@
> >  435  common  clone3                          sys_clone3
> >  437  common  openat2                         sys_openat2
> >  438  common  pidfd_getfd                     sys_pidfd_getfd
> > +439  common  process_madvise                 sys_process_madvise
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index 1815065d52f3..e4cd2c2f8bb4 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -876,6 +876,8 @@ asmlinkage long sys_munlockall(void);
> >  asmlinkage long sys_mincore(unsigned long start, size_t len,
> >                               unsigned char __user * vec);
> >  asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
> > +asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
> > +                     size_t len, int behavior, unsigned long flags);
> >  asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
> >                       unsigned long prot, unsigned long pgoff,
> >                       unsigned long flags);
> > diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> > index 3a3201e4618e..fa289b91410e 100644
> > --- a/include/uapi/asm-generic/unistd.h
> > +++ b/include/uapi/asm-generic/unistd.h
> > @@ -855,9 +855,11 @@ __SYSCALL(__NR_clone3, sys_clone3)
> >  __SYSCALL(__NR_openat2, sys_openat2)
> >  #define __NR_pidfd_getfd 438
> >  __SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
> > +#define __NR_process_madvise 439
> > +__SYSCALL(__NR_process_madvise, sys_process_madvise)
> >
> >  #undef __NR_syscalls
> > -#define __NR_syscalls 439
> > +#define __NR_syscalls 440
> >
> >  /*
> >   * 32 bit systems traditionally used different
> > diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> > index 3b69a560a7ac..6c7332776e8e 100644
> > --- a/kernel/sys_ni.c
> > +++ b/kernel/sys_ni.c
> > @@ -280,6 +280,7 @@ COND_SYSCALL(mlockall);
> >  COND_SYSCALL(munlockall);
> >  COND_SYSCALL(mincore);
> >  COND_SYSCALL(madvise);
> > +COND_SYSCALL(process_madvise);
> >  COND_SYSCALL(remap_file_pages);
> >  COND_SYSCALL(mbind);
> >  COND_SYSCALL_COMPAT(mbind);
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index f75c86b6c463..f29155b8185d 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/falloc.h>
> >  #include <linux/fadvise.h>
> >  #include <linux/sched.h>
> > +#include <linux/sched/mm.h>
> >  #include <linux/ksm.h>
> >  #include <linux/fs.h>
> >  #include <linux/file.h>
> > @@ -986,6 +987,18 @@ madvise_behavior_valid(int behavior)
> >       }
> >  }
> >
> > +static bool
> > +process_madvise_behavior_valid(int behavior)
> > +{
> > +     switch (behavior) {
> > +     case MADV_COLD:
> > +     case MADV_PAGEOUT:
> > +             return true;
> > +     default:
> > +             return false;
> > +     }
> > +}
> > +
> >  /*/
> >   * The madvise(2) system call.
> >   *
> > @@ -1033,6 +1046,11 @@ madvise_behavior_valid(int behavior)
> >   *  MADV_DONTDUMP - the application wants to prevent pages in the given range
> >   *           from being included in its core dump.
> >   *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
> > + *  MADV_COLD - the application uses the memory less so the kernel can

"kernel can" implies that kernel might not deactivate the pages, which
IIUC is not the case. Maybe rephrase as "MADV_COLD - the application
is not expected to use this memory soon, deactivate pages in this
range so that they can be reclaimed easily if memory pressure
happens.""

> > + *           deactivate the memory to evict them quickly when the memory
> > + *           pressure happen.
> > + *  MADV_PAGEOUT - the application uses the memroy very rarely so kernel can

s/memroy/memory

> > + *           page out the memory instantly.

same nit about the usage of "kernel can". Maybe rephrase as
"MADV_PAGEOUT - the application is not expected to use this memory
soon, page out the pages in this range immediately.""

> >   *
> >   * return values:
> >   *  zero    - success
> > @@ -1150,3 +1168,49 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
> >  {
> >       return do_madvise(current, current->mm, start, len_in, behavior);
> >  }
> > +
> > +SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > +             size_t, len_in, int, behavior, unsigned long, flags)
> > +{
> > +     int ret;
> > +     struct fd f;
> > +     struct pid *pid;
> > +     struct task_struct *task;
> > +     struct mm_struct *mm;
> > +
> > +     if (flags != 0)
> > +             return -EINVAL;
> > +
> > +     if (!process_madvise_behavior_valid(behavior))
> > +             return -EINVAL;
> > +
> > +     f = fdget(pidfd);
> > +     if (!f.file)
> > +             return -EBADF;
> > +
> > +     pid = pidfd_pid(f.file);
> > +     if (IS_ERR(pid)) {
> > +             ret = PTR_ERR(pid);
> > +             goto fdput;
> > +     }
> > +
> > +     task = get_pid_task(pid, PIDTYPE_PID);
> > +     if (!task) {
> > +             ret = -ESRCH;
> > +             goto fdput;
> > +     }
> > +
> > +     mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> > +     if (IS_ERR_OR_NULL(mm)) {
> > +             ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
> > +             goto release_task;
> > +     }
> > +
> > +     ret = do_madvise(task, mm, start, len_in, behavior);
> > +     mmput(mm);
> > +release_task:
> > +     put_task_struct(task);
> > +fdput:
> > +     fdput(f);
> > +     return ret;
> > +}
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

Re: [PATCH v6 2/7] mm: introduce external memory hinting API

[Minchan Kim]

On Fri, Feb 28, 2020 at 02:14:56PM -0800, Suren Baghdasaryan wrote:

< snip >

> > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > index f75c86b6c463..f29155b8185d 100644
> > > --- a/mm/madvise.c
> > > +++ b/mm/madvise.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/falloc.h>
> > >  #include <linux/fadvise.h>
> > >  #include <linux/sched.h>
> > > +#include <linux/sched/mm.h>
> > >  #include <linux/ksm.h>
> > >  #include <linux/fs.h>
> > >  #include <linux/file.h>
> > > @@ -986,6 +987,18 @@ madvise_behavior_valid(int behavior)
> > >       }
> > >  }
> > >
> > > +static bool
> > > +process_madvise_behavior_valid(int behavior)
> > > +{
> > > +     switch (behavior) {
> > > +     case MADV_COLD:
> > > +     case MADV_PAGEOUT:
> > > +             return true;
> > > +     default:
> > > +             return false;
> > > +     }
> > > +}
> > > +
> > >  /*/
> > >   * The madvise(2) system call.
> > >   *
> > > @@ -1033,6 +1046,11 @@ madvise_behavior_valid(int behavior)
> > >   *  MADV_DONTDUMP - the application wants to prevent pages in the given range
> > >   *           from being included in its core dump.
> > >   *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
> > > + *  MADV_COLD - the application uses the memory less so the kernel can
> 
> "kernel can" implies that kernel might not deactivate the pages, which
> IIUC is not the case. Maybe rephrase as "MADV_COLD - the application
> is not expected to use this memory soon, deactivate pages in this
> range so that they can be reclaimed easily if memory pressure
> happens.""

That is much better.

> 
> > > + *           deactivate the memory to evict them quickly when the memory
> > > + *           pressure happen.
> > > + *  MADV_PAGEOUT - the application uses the memroy very rarely so kernel can
> 
> s/memroy/memory

Fixed.

> 
> > > + *           page out the memory instantly.
> 
> same nit about the usage of "kernel can". Maybe rephrase as
> "MADV_PAGEOUT - the application is not expected to use this memory
> soon, page out the pages in this range immediately.""

Yub.

> 
> > >   *
> > >   * return values:
> > >   *  zero    - success
> > > @@ -1150,3 +1168,49 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
> > >  {
> > >       return do_madvise(current, current->mm, start, len_in, behavior);
> > >  }
> > > +
> > > +SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > > +             size_t, len_in, int, behavior, unsigned long, flags)
> > > +{
> > > +     int ret;
> > > +     struct fd f;
> > > +     struct pid *pid;
> > > +     struct task_struct *task;
> > > +     struct mm_struct *mm;
> > > +
> > > +     if (flags != 0)
> > > +             return -EINVAL;
> > > +
> > > +     if (!process_madvise_behavior_valid(behavior))
> > > +             return -EINVAL;
> > > +
> > > +     f = fdget(pidfd);
> > > +     if (!f.file)
> > > +             return -EBADF;
> > > +
> > > +     pid = pidfd_pid(f.file);
> > > +     if (IS_ERR(pid)) {
> > > +             ret = PTR_ERR(pid);
> > > +             goto fdput;
> > > +     }
> > > +
> > > +     task = get_pid_task(pid, PIDTYPE_PID);
> > > +     if (!task) {
> > > +             ret = -ESRCH;
> > > +             goto fdput;
> > > +     }
> > > +
> > > +     mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> > > +     if (IS_ERR_OR_NULL(mm)) {
> > > +             ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
> > > +             goto release_task;
> > > +     }
> > > +
> > > +     ret = do_madvise(task, mm, start, len_in, behavior);
> > > +     mmput(mm);
> > > +release_task:
> > > +     put_task_struct(task);
> > > +fdput:
> > > +     fdput(f);
> > > +     return ret;
> > > +}
> > > --
> > > 2.25.0.265.gbab2e86ba0-goog
> > >
> >
> 
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>

Thanks, Suren!

Re: [PATCH v6 2/7] mm: introduce external memory hinting API

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9316 bytes --]

Hi Minchan,

I love your patch! Yet something to improve:

[auto build test ERROR on m68k/for-next]
[also build test ERROR on powerpc/next s390/features linus/master v5.6-rc2 next-20200220]
[cannot apply to arm64/for-next/core tip/x86/asm arm/for-next hp-parisc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/introduce-memory-hinting-API-for-external-process/20200220-225155
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
config: arm64-randconfig-a001-20200220 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from include/uapi/linux/unistd.h:8,
                    from include/linux/syscalls.h:81,
                    from arch/arm64/kernel/sys.c:16:
   include/uapi/asm-generic/unistd.h:858:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 439
    
   include/uapi/asm-generic/unistd.h:856:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64/kernel/sys.c:53:
   include/uapi/asm-generic/unistd.h:856:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from include/uapi/linux/unistd.h:8,
                    from include/linux/syscalls.h:81,
                    from arch/arm64/kernel/sys.c:16:
   include/uapi/asm-generic/unistd.h:858:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 439
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64/kernel/sys.c:53:
   include/uapi/asm-generic/unistd.h:858:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 439
    
   include/uapi/asm-generic/unistd.h:856:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64/kernel/sys.c:60:
   include/uapi/asm-generic/unistd.h:856:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64/kernel/sys.c:53:
   include/uapi/asm-generic/unistd.h:858:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 439
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64/kernel/sys.c:60:
   include/uapi/asm-generic/unistd.h:858:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 439
    
   include/uapi/asm-generic/unistd.h:856:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 438
    
>> include/uapi/asm-generic/unistd.h:859:11: error: '__NR_process_madvise' undeclared here (not in a function); did you mean '__NR_process_vm_writev'?
    __SYSCALL(__NR_process_madvise, sys_process_madvise)
              ^
   arch/arm64/kernel/sys.c:56:29: note: in definition of macro '__SYSCALL'
    #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
                                ^~
   include/uapi/asm-generic/unistd.h:859:11: error: array index in initializer not of integer type
    __SYSCALL(__NR_process_madvise, sys_process_madvise)
              ^
   arch/arm64/kernel/sys.c:56:29: note: in definition of macro '__SYSCALL'
    #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
                                ^~
   include/uapi/asm-generic/unistd.h:859:11: note: (near initialization for 'sys_call_table')
    __SYSCALL(__NR_process_madvise, sys_process_madvise)
              ^
   arch/arm64/kernel/sys.c:56:29: note: in definition of macro '__SYSCALL'
    #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
                                ^~
--
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from include/uapi/linux/unistd.h:8,
                    from include/linux/syscalls.h:81,
                    from arch/arm64//kernel/sys.c:16:
   include/uapi/asm-generic/unistd.h:858:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 439
    
   include/uapi/asm-generic/unistd.h:856:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64//kernel/sys.c:53:
   include/uapi/asm-generic/unistd.h:856:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from include/uapi/linux/unistd.h:8,
                    from include/linux/syscalls.h:81,
                    from arch/arm64//kernel/sys.c:16:
   include/uapi/asm-generic/unistd.h:858:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 439
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64//kernel/sys.c:53:
   include/uapi/asm-generic/unistd.h:858:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 439
    
   include/uapi/asm-generic/unistd.h:856:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64//kernel/sys.c:60:
   include/uapi/asm-generic/unistd.h:856:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 438
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64//kernel/sys.c:53:
   include/uapi/asm-generic/unistd.h:858:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 439
    
   In file included from arch/arm64/include/uapi/asm/unistd.h:24:0,
                    from arch/arm64/include/asm/unistd.h:47,
                    from arch/arm64//kernel/sys.c:60:
   include/uapi/asm-generic/unistd.h:858:0: warning: "__NR_pidfd_getfd" redefined
    #define __NR_pidfd_getfd 439
    
   include/uapi/asm-generic/unistd.h:856:0: note: this is the location of the previous definition
    #define __NR_pidfd_getfd 438
    
>> include/uapi/asm-generic/unistd.h:859:11: error: '__NR_process_madvise' undeclared here (not in a function); did you mean '__NR_process_vm_writev'?
    __SYSCALL(__NR_process_madvise, sys_process_madvise)
              ^
   arch/arm64//kernel/sys.c:56:29: note: in definition of macro '__SYSCALL'
    #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
                                ^~
   include/uapi/asm-generic/unistd.h:859:11: error: array index in initializer not of integer type
    __SYSCALL(__NR_process_madvise, sys_process_madvise)
              ^
   arch/arm64//kernel/sys.c:56:29: note: in definition of macro '__SYSCALL'
    #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
                                ^~
   include/uapi/asm-generic/unistd.h:859:11: note: (near initialization for 'sys_call_table')
    __SYSCALL(__NR_process_madvise, sys_process_madvise)
              ^
   arch/arm64//kernel/sys.c:56:29: note: in definition of macro '__SYSCALL'
    #define __SYSCALL(nr, sym) [nr] = __arm64_##sym,
                                ^~

vim +859 include/uapi/asm-generic/unistd.h

   853	
   854	#define __NR_openat2 437
   855	__SYSCALL(__NR_openat2, sys_openat2)
   856	#define __NR_pidfd_getfd 438
   857	__SYSCALL(__NR_pidfd_getfd, sys_pidfd_getfd)
   858	#define __NR_pidfd_getfd 439
 > 859	__SYSCALL(__NR_process_madvise, sys_process_madvise)
   860	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34781 bytes --]

[PATCH v6 3/7] mm: check fatal signal pending of target process

[Minchan Kim]

Bail out to prevent unnecessary CPU overhead if target process has
pending fatal signal during (MADV_COLD|MADV_PAGEOUT) operation.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/madvise.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index f29155b8185d..def1507c2030 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -36,6 +36,7 @@
 struct madvise_walk_private {
 	struct mmu_gather *tlb;
 	bool pageout;
+	struct task_struct *target_task;
 };
 
 /*
@@ -316,6 +317,10 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 	if (fatal_signal_pending(current))
 		return -EINTR;
 
+	if (private->target_task &&
+			fatal_signal_pending(private->target_task))
+		return -EINTR;
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	if (pmd_trans_huge(*pmd)) {
 		pmd_t orig_pmd;
@@ -471,12 +476,14 @@ static const struct mm_walk_ops cold_walk_ops = {
 };
 
 static void madvise_cold_page_range(struct mmu_gather *tlb,
+			     struct task_struct *task,
 			     struct vm_area_struct *vma,
 			     unsigned long addr, unsigned long end)
 {
 	struct madvise_walk_private walk_private = {
 		.pageout = false,
 		.tlb = tlb,
+		.target_task = task,
 	};
 
 	tlb_start_vma(tlb, vma);
@@ -484,7 +491,8 @@ static void madvise_cold_page_range(struct mmu_gather *tlb,
 	tlb_end_vma(tlb, vma);
 }
 
-static long madvise_cold(struct vm_area_struct *vma,
+static long madvise_cold(struct task_struct *task,
+			struct vm_area_struct *vma,
 			struct vm_area_struct **prev,
 			unsigned long start_addr, unsigned long end_addr)
 {
@@ -497,19 +505,21 @@ static long madvise_cold(struct vm_area_struct *vma,
 
 	lru_add_drain();
 	tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
-	madvise_cold_page_range(&tlb, vma, start_addr, end_addr);
+	madvise_cold_page_range(&tlb, task, vma, start_addr, end_addr);
 	tlb_finish_mmu(&tlb, start_addr, end_addr);
 
 	return 0;
 }
 
 static void madvise_pageout_page_range(struct mmu_gather *tlb,
+			     struct task_struct *task,
 			     struct vm_area_struct *vma,
 			     unsigned long addr, unsigned long end)
 {
 	struct madvise_walk_private walk_private = {
 		.pageout = true,
 		.tlb = tlb,
+		.target_task = task,
 	};
 
 	tlb_start_vma(tlb, vma);
@@ -533,7 +543,8 @@ static inline bool can_do_pageout(struct vm_area_struct *vma)
 		inode_permission(file_inode(vma->vm_file), MAY_WRITE) == 0;
 }
 
-static long madvise_pageout(struct vm_area_struct *vma,
+static long madvise_pageout(struct task_struct *task,
+			struct vm_area_struct *vma,
 			struct vm_area_struct **prev,
 			unsigned long start_addr, unsigned long end_addr)
 {
@@ -549,7 +560,7 @@ static long madvise_pageout(struct vm_area_struct *vma,
 
 	lru_add_drain();
 	tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
-	madvise_pageout_page_range(&tlb, vma, start_addr, end_addr);
+	madvise_pageout_page_range(&tlb, task, vma, start_addr, end_addr);
 	tlb_finish_mmu(&tlb, start_addr, end_addr);
 
 	return 0;
@@ -929,7 +940,8 @@ static int madvise_inject_error(int behavior,
 #endif
 
 static long
-madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
+madvise_vma(struct task_struct *task, struct vm_area_struct *vma,
+		struct vm_area_struct **prev,
 		unsigned long start, unsigned long end, int behavior)
 {
 	switch (behavior) {
@@ -938,9 +950,9 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
 	case MADV_WILLNEED:
 		return madvise_willneed(vma, prev, start, end);
 	case MADV_COLD:
-		return madvise_cold(vma, prev, start, end);
+		return madvise_cold(task, vma, prev, start, end);
 	case MADV_PAGEOUT:
-		return madvise_pageout(vma, prev, start, end);
+		return madvise_pageout(task, vma, prev, start, end);
 	case MADV_FREE:
 	case MADV_DONTNEED:
 		return madvise_dontneed_free(vma, prev, start, end, behavior);
@@ -1140,7 +1152,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
 			tmp = end;
 
 		/* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */
-		error = madvise_vma(vma, &prev, start, tmp, behavior);
+		error = madvise_vma(target_task, vma, &prev,
+					start, tmp, behavior);
 		if (error)
 			goto out;
 		start = tmp;
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 3/7] mm: check fatal signal pending of target process

[Suren Baghdasaryan]

On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
>
> Bail out to prevent unnecessary CPU overhead if target process has
> pending fatal signal during (MADV_COLD|MADV_PAGEOUT) operation.
>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  mm/madvise.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index f29155b8185d..def1507c2030 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -36,6 +36,7 @@
>  struct madvise_walk_private {
>         struct mmu_gather *tlb;
>         bool pageout;
> +       struct task_struct *target_task;
>  };
>
>  /*
> @@ -316,6 +317,10 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>         if (fatal_signal_pending(current))
>                 return -EINTR;
>
> +       if (private->target_task &&
> +                       fatal_signal_pending(private->target_task))
> +               return -EINTR;
> +
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>         if (pmd_trans_huge(*pmd)) {
>                 pmd_t orig_pmd;
> @@ -471,12 +476,14 @@ static const struct mm_walk_ops cold_walk_ops = {
>  };
>
>  static void madvise_cold_page_range(struct mmu_gather *tlb,
> +                            struct task_struct *task,
>                              struct vm_area_struct *vma,
>                              unsigned long addr, unsigned long end)
>  {
>         struct madvise_walk_private walk_private = {
>                 .pageout = false,
>                 .tlb = tlb,
> +               .target_task = task,
>         };
>
>         tlb_start_vma(tlb, vma);
> @@ -484,7 +491,8 @@ static void madvise_cold_page_range(struct mmu_gather *tlb,
>         tlb_end_vma(tlb, vma);
>  }
>
> -static long madvise_cold(struct vm_area_struct *vma,
> +static long madvise_cold(struct task_struct *task,
> +                       struct vm_area_struct *vma,
>                         struct vm_area_struct **prev,
>                         unsigned long start_addr, unsigned long end_addr)
>  {
> @@ -497,19 +505,21 @@ static long madvise_cold(struct vm_area_struct *vma,
>
>         lru_add_drain();
>         tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
> -       madvise_cold_page_range(&tlb, vma, start_addr, end_addr);
> +       madvise_cold_page_range(&tlb, task, vma, start_addr, end_addr);
>         tlb_finish_mmu(&tlb, start_addr, end_addr);
>
>         return 0;
>  }
>
>  static void madvise_pageout_page_range(struct mmu_gather *tlb,
> +                            struct task_struct *task,
>                              struct vm_area_struct *vma,
>                              unsigned long addr, unsigned long end)
>  {
>         struct madvise_walk_private walk_private = {
>                 .pageout = true,
>                 .tlb = tlb,
> +               .target_task = task,
>         };
>
>         tlb_start_vma(tlb, vma);
> @@ -533,7 +543,8 @@ static inline bool can_do_pageout(struct vm_area_struct *vma)
>                 inode_permission(file_inode(vma->vm_file), MAY_WRITE) == 0;
>  }
>
> -static long madvise_pageout(struct vm_area_struct *vma,
> +static long madvise_pageout(struct task_struct *task,
> +                       struct vm_area_struct *vma,
>                         struct vm_area_struct **prev,
>                         unsigned long start_addr, unsigned long end_addr)
>  {
> @@ -549,7 +560,7 @@ static long madvise_pageout(struct vm_area_struct *vma,
>
>         lru_add_drain();
>         tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
> -       madvise_pageout_page_range(&tlb, vma, start_addr, end_addr);
> +       madvise_pageout_page_range(&tlb, task, vma, start_addr, end_addr);
>         tlb_finish_mmu(&tlb, start_addr, end_addr);
>
>         return 0;
> @@ -929,7 +940,8 @@ static int madvise_inject_error(int behavior,
>  #endif
>
>  static long
> -madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
> +madvise_vma(struct task_struct *task, struct vm_area_struct *vma,
> +               struct vm_area_struct **prev,
>                 unsigned long start, unsigned long end, int behavior)
>  {
>         switch (behavior) {
> @@ -938,9 +950,9 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
>         case MADV_WILLNEED:
>                 return madvise_willneed(vma, prev, start, end);
>         case MADV_COLD:
> -               return madvise_cold(vma, prev, start, end);
> +               return madvise_cold(task, vma, prev, start, end);
>         case MADV_PAGEOUT:
> -               return madvise_pageout(vma, prev, start, end);
> +               return madvise_pageout(task, vma, prev, start, end);
>         case MADV_FREE:
>         case MADV_DONTNEED:
>                 return madvise_dontneed_free(vma, prev, start, end, behavior);
> @@ -1140,7 +1152,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
>                         tmp = end;
>
>                 /* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */
> -               error = madvise_vma(vma, &prev, start, tmp, behavior);
> +               error = madvise_vma(target_task, vma, &prev,
> +                                       start, tmp, behavior);
>                 if (error)
>                         goto out;
>                 start = tmp;
> --
> 2.25.0.265.gbab2e86ba0-goog
>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

[PATCH v6 4/7] pid: move pidfd_get_pid function to pid.c

[Minchan Kim]

process_madvise syscall needs pidfd_get_pid function to translate
pidfd to pid so this patch move the function to kernel/pid.c.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Christian Brauner <christian@brauner.io>
Suggested-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/pid.h |  1 +
 kernel/exit.c       | 17 -----------------
 kernel/pid.c        | 17 +++++++++++++++++
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/linux/pid.h b/include/linux/pid.h
index 998ae7d24450..993f68cb45c2 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -75,6 +75,7 @@ extern const struct file_operations pidfd_fops;
 struct file;
 
 extern struct pid *pidfd_pid(const struct file *file);
+struct pid *pidfd_get_pid(unsigned int fd);
 
 static inline struct pid *get_pid(struct pid *pid)
 {
diff --git a/kernel/exit.c b/kernel/exit.c
index 0b81b26a872a..43375f9d8bbc 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1470,23 +1470,6 @@ static long do_wait(struct wait_opts *wo)
 	return retval;
 }
 
-static struct pid *pidfd_get_pid(unsigned int fd)
-{
-	struct fd f;
-	struct pid *pid;
-
-	f = fdget(fd);
-	if (!f.file)
-		return ERR_PTR(-EBADF);
-
-	pid = pidfd_pid(f.file);
-	if (!IS_ERR(pid))
-		get_pid(pid);
-
-	fdput(f);
-	return pid;
-}
-
 static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop,
 			  int options, struct rusage *ru)
 {
diff --git a/kernel/pid.c b/kernel/pid.c
index 0f4ecb57214c..360ba480a2a9 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -496,6 +496,23 @@ struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
 	return idr_get_next(&ns->idr, &nr);
 }
 
+struct pid *pidfd_get_pid(unsigned int fd)
+{
+	struct fd f;
+	struct pid *pid;
+
+	f = fdget(fd);
+	if (!f.file)
+		return ERR_PTR(-EBADF);
+
+	pid = pidfd_pid(f.file);
+	if (!IS_ERR(pid))
+		get_pid(pid);
+
+	fdput(f);
+	return pid;
+}
+
 /**
  * pidfd_create() - Create a new pid file descriptor.
  *
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 4/7] pid: move pidfd_get_pid function to pid.c

[Suren Baghdasaryan]

On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
>
> process_madvise syscall needs pidfd_get_pid function to translate
> pidfd to pid so this patch move the function to kernel/pid.c.
>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Christian Brauner <christian@brauner.io>
> Suggested-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  include/linux/pid.h |  1 +
>  kernel/exit.c       | 17 -----------------
>  kernel/pid.c        | 17 +++++++++++++++++
>  3 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/pid.h b/include/linux/pid.h
> index 998ae7d24450..993f68cb45c2 100644
> --- a/include/linux/pid.h
> +++ b/include/linux/pid.h
> @@ -75,6 +75,7 @@ extern const struct file_operations pidfd_fops;
>  struct file;
>
>  extern struct pid *pidfd_pid(const struct file *file);
> +struct pid *pidfd_get_pid(unsigned int fd);
>
>  static inline struct pid *get_pid(struct pid *pid)
>  {
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 0b81b26a872a..43375f9d8bbc 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1470,23 +1470,6 @@ static long do_wait(struct wait_opts *wo)
>         return retval;
>  }
>
> -static struct pid *pidfd_get_pid(unsigned int fd)
> -{
> -       struct fd f;
> -       struct pid *pid;
> -
> -       f = fdget(fd);
> -       if (!f.file)
> -               return ERR_PTR(-EBADF);
> -
> -       pid = pidfd_pid(f.file);
> -       if (!IS_ERR(pid))
> -               get_pid(pid);
> -
> -       fdput(f);
> -       return pid;
> -}
> -
>  static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop,
>                           int options, struct rusage *ru)
>  {
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 0f4ecb57214c..360ba480a2a9 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -496,6 +496,23 @@ struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
>         return idr_get_next(&ns->idr, &nr);
>  }
>
> +struct pid *pidfd_get_pid(unsigned int fd)
> +{
> +       struct fd f;
> +       struct pid *pid;
> +
> +       f = fdget(fd);
> +       if (!f.file)
> +               return ERR_PTR(-EBADF);
> +
> +       pid = pidfd_pid(f.file);
> +       if (!IS_ERR(pid))
> +               get_pid(pid);
> +
> +       fdput(f);
> +       return pid;
> +}
> +
>  /**
>   * pidfd_create() - Create a new pid file descriptor.
>   *
> --
> 2.25.0.265.gbab2e86ba0-goog
>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

[PATCH v6 5/7] mm: support both pid and pidfd for process_madvise

[Minchan Kim]

There is a demand[1] to support pid as well pidfd for process_madvise
to reduce unnecessary syscall to get pidfd if the user has control of
the target process(ie, they could guarantee the process is not gone
or pid is not reused. Or, it might be okay to give a hint to wrong
process).

This patch aims for supporting both options like waitid(2). So, the
syscall is currently,

	int process_madvise(int which, pid_t pid, void *addr,
		size_t length, int advise, unsigned long flag);

@which is actually idtype_t for userspace libray and currently,
it supports P_PID and P_PIDFD.

[1]  https://lore.kernel.org/linux-mm/9d849087-3359-c4ab-fbec-859e8186c509@virtuozzo.com/

Cc: Christian Brauner <christian@brauner.io>
Suggested-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/syscalls.h |  3 ++-
 mm/madvise.c             | 34 ++++++++++++++++++++++------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e4cd2c2f8bb4..f5ada20e2943 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -876,7 +876,8 @@ asmlinkage long sys_munlockall(void);
 asmlinkage long sys_mincore(unsigned long start, size_t len,
 				unsigned char __user * vec);
 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
-asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
+
+asmlinkage long sys_process_madvise(int which, pid_t pid, unsigned long start,
 			size_t len, int behavior, unsigned long flags);
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
 			unsigned long prot, unsigned long pgoff,
diff --git a/mm/madvise.c b/mm/madvise.c
index def1507c2030..f6d9b9e66243 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1182,11 +1182,10 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 	return do_madvise(current, current->mm, start, len_in, behavior);
 }
 
-SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
+SYSCALL_DEFINE6(process_madvise, int, which, pid_t, upid, unsigned long, start,
 		size_t, len_in, int, behavior, unsigned long, flags)
 {
 	int ret;
-	struct fd f;
 	struct pid *pid;
 	struct task_struct *task;
 	struct mm_struct *mm;
@@ -1197,20 +1196,31 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
 	if (!process_madvise_behavior_valid(behavior))
 		return -EINVAL;
 
-	f = fdget(pidfd);
-	if (!f.file)
-		return -EBADF;
+	switch (which) {
+	case P_PID:
+		if (upid <= 0)
+			return -EINVAL;
+
+		pid = find_get_pid(upid);
+		if (!pid)
+			return -ESRCH;
+		break;
+	case P_PIDFD:
+		if (upid < 0)
+			return -EINVAL;
 
-	pid = pidfd_pid(f.file);
-	if (IS_ERR(pid)) {
-		ret = PTR_ERR(pid);
-		goto fdput;
+		pid = pidfd_get_pid(upid);
+		if (IS_ERR(pid))
+			return PTR_ERR(pid);
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	task = get_pid_task(pid, PIDTYPE_PID);
 	if (!task) {
 		ret = -ESRCH;
-		goto fdput;
+		goto put_pid;
 	}
 
 	mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
@@ -1223,7 +1233,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
 	mmput(mm);
 release_task:
 	put_task_struct(task);
-fdput:
-	fdput(f);
+put_pid:
+	put_pid(pid);
 	return ret;
 }
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 5/7] mm: support both pid and pidfd for process_madvise

[Suren Baghdasaryan]

On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
>
> There is a demand[1] to support pid as well pidfd for process_madvise
> to reduce unnecessary syscall to get pidfd if the user has control of
> the target process(ie, they could guarantee the process is not gone
> or pid is not reused. Or, it might be okay to give a hint to wrong
> process).

nit: When would "give a hint to wrong process" be ok? I would just
remove this part.

>
> This patch aims for supporting both options like waitid(2). So, the
> syscall is currently,
>
>         int process_madvise(int which, pid_t pid, void *addr,
>                 size_t length, int advise, unsigned long flag);
>
> @which is actually idtype_t for userspace libray and currently,
> it supports P_PID and P_PIDFD.
>
> [1]  https://lore.kernel.org/linux-mm/9d849087-3359-c4ab-fbec-859e8186c509@virtuozzo.com/
>
> Cc: Christian Brauner <christian@brauner.io>
> Suggested-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  include/linux/syscalls.h |  3 ++-
>  mm/madvise.c             | 34 ++++++++++++++++++++++------------
>  2 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e4cd2c2f8bb4..f5ada20e2943 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -876,7 +876,8 @@ asmlinkage long sys_munlockall(void);
>  asmlinkage long sys_mincore(unsigned long start, size_t len,
>                                 unsigned char __user * vec);
>  asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
> -asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
> +
> +asmlinkage long sys_process_madvise(int which, pid_t pid, unsigned long start,
>                         size_t len, int behavior, unsigned long flags);
>  asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
>                         unsigned long prot, unsigned long pgoff,
> diff --git a/mm/madvise.c b/mm/madvise.c
> index def1507c2030..f6d9b9e66243 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1182,11 +1182,10 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
>         return do_madvise(current, current->mm, start, len_in, behavior);
>  }
>
> -SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> +SYSCALL_DEFINE6(process_madvise, int, which, pid_t, upid, unsigned long, start,
>                 size_t, len_in, int, behavior, unsigned long, flags)
>  {
>         int ret;
> -       struct fd f;
>         struct pid *pid;
>         struct task_struct *task;
>         struct mm_struct *mm;
> @@ -1197,20 +1196,31 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
>         if (!process_madvise_behavior_valid(behavior))
>                 return -EINVAL;
>
> -       f = fdget(pidfd);
> -       if (!f.file)
> -               return -EBADF;
> +       switch (which) {
> +       case P_PID:
> +               if (upid <= 0)
> +                       return -EINVAL;
> +
> +               pid = find_get_pid(upid);
> +               if (!pid)
> +                       return -ESRCH;
> +               break;
> +       case P_PIDFD:
> +               if (upid < 0)
> +                       return -EINVAL;
>
> -       pid = pidfd_pid(f.file);
> -       if (IS_ERR(pid)) {
> -               ret = PTR_ERR(pid);
> -               goto fdput;
> +               pid = pidfd_get_pid(upid);
> +               if (IS_ERR(pid))
> +                       return PTR_ERR(pid);
> +               break;
> +       default:
> +               return -EINVAL;
>         }
>
>         task = get_pid_task(pid, PIDTYPE_PID);
>         if (!task) {
>                 ret = -ESRCH;
> -               goto fdput;
> +               goto put_pid;
>         }
>
>         mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> @@ -1223,7 +1233,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
>         mmput(mm);
>  release_task:
>         put_task_struct(task);
> -fdput:
> -       fdput(f);
> +put_pid:
> +       put_pid(pid);
>         return ret;
>  }
> --
> 2.25.0.265.gbab2e86ba0-goog
>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

Re: [PATCH v6 5/7] mm: support both pid and pidfd for process_madvise

[Minchan Kim]

On Fri, Feb 28, 2020 at 02:41:07PM -0800, Suren Baghdasaryan wrote:
> On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
> >
> > There is a demand[1] to support pid as well pidfd for process_madvise
> > to reduce unnecessary syscall to get pidfd if the user has control of
> > the target process(ie, they could guarantee the process is not gone
> > or pid is not reused. Or, it might be okay to give a hint to wrong
> > process).
> 
> nit: When would "give a hint to wrong process" be ok? I would just
> remove this part.

I wanted to say non destructive hints. It's already true for other
some hints because they are just best effort so it's not critical
to be failed. If you mind it, I will remove the phrase.

Thanks.

> 
> >
> > This patch aims for supporting both options like waitid(2). So, the
> > syscall is currently,
> >
> >         int process_madvise(int which, pid_t pid, void *addr,
> >                 size_t length, int advise, unsigned long flag);
> >
> > @which is actually idtype_t for userspace libray and currently,
> > it supports P_PID and P_PIDFD.
> >
> > [1]  https://lore.kernel.org/linux-mm/9d849087-3359-c4ab-fbec-859e8186c509@virtuozzo.com/
> >
> > Cc: Christian Brauner <christian@brauner.io>
> > Suggested-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  include/linux/syscalls.h |  3 ++-
> >  mm/madvise.c             | 34 ++++++++++++++++++++++------------
> >  2 files changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index e4cd2c2f8bb4..f5ada20e2943 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -876,7 +876,8 @@ asmlinkage long sys_munlockall(void);
> >  asmlinkage long sys_mincore(unsigned long start, size_t len,
> >                                 unsigned char __user * vec);
> >  asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
> > -asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
> > +
> > +asmlinkage long sys_process_madvise(int which, pid_t pid, unsigned long start,
> >                         size_t len, int behavior, unsigned long flags);
> >  asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
> >                         unsigned long prot, unsigned long pgoff,
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index def1507c2030..f6d9b9e66243 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -1182,11 +1182,10 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
> >         return do_madvise(current, current->mm, start, len_in, behavior);
> >  }
> >
> > -SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > +SYSCALL_DEFINE6(process_madvise, int, which, pid_t, upid, unsigned long, start,
> >                 size_t, len_in, int, behavior, unsigned long, flags)
> >  {
> >         int ret;
> > -       struct fd f;
> >         struct pid *pid;
> >         struct task_struct *task;
> >         struct mm_struct *mm;
> > @@ -1197,20 +1196,31 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> >         if (!process_madvise_behavior_valid(behavior))
> >                 return -EINVAL;
> >
> > -       f = fdget(pidfd);
> > -       if (!f.file)
> > -               return -EBADF;
> > +       switch (which) {
> > +       case P_PID:
> > +               if (upid <= 0)
> > +                       return -EINVAL;
> > +
> > +               pid = find_get_pid(upid);
> > +               if (!pid)
> > +                       return -ESRCH;
> > +               break;
> > +       case P_PIDFD:
> > +               if (upid < 0)
> > +                       return -EINVAL;
> >
> > -       pid = pidfd_pid(f.file);
> > -       if (IS_ERR(pid)) {
> > -               ret = PTR_ERR(pid);
> > -               goto fdput;
> > +               pid = pidfd_get_pid(upid);
> > +               if (IS_ERR(pid))
> > +                       return PTR_ERR(pid);
> > +               break;
> > +       default:
> > +               return -EINVAL;
> >         }
> >
> >         task = get_pid_task(pid, PIDTYPE_PID);
> >         if (!task) {
> >                 ret = -ESRCH;
> > -               goto fdput;
> > +               goto put_pid;
> >         }
> >
> >         mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> > @@ -1223,7 +1233,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> >         mmput(mm);
> >  release_task:
> >         put_task_struct(task);
> > -fdput:
> > -       fdput(f);
> > +put_pid:
> > +       put_pid(pid);
> >         return ret;
> >  }
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
> 
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>

Re: [PATCH v6 5/7] mm: support both pid and pidfd for process_madvise

[Suren Baghdasaryan]

On Mon, Mar 2, 2020 at 11:23 AM Minchan Kim <minchan@kernel.org> wrote:
>
> On Fri, Feb 28, 2020 at 02:41:07PM -0800, Suren Baghdasaryan wrote:
> > On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
> > >
> > > There is a demand[1] to support pid as well pidfd for process_madvise
> > > to reduce unnecessary syscall to get pidfd if the user has control of
> > > the target process(ie, they could guarantee the process is not gone
> > > or pid is not reused. Or, it might be okay to give a hint to wrong
> > > process).
> >
> > nit: When would "give a hint to wrong process" be ok? I would just
> > remove this part.
>
> I wanted to say non destructive hints. It's already true for other
> some hints because they are just best effort so it's not critical
> to be failed. If you mind it, I will remove the phrase.

Up to you, or maybe call it a "non-fatal" error? Saying that it's ok
to hint a wrong process sounds wrong to me.

>
> Thanks.
>
> >
> > >
> > > This patch aims for supporting both options like waitid(2). So, the
> > > syscall is currently,
> > >
> > >         int process_madvise(int which, pid_t pid, void *addr,
> > >                 size_t length, int advise, unsigned long flag);
> > >
> > > @which is actually idtype_t for userspace libray and currently,
> > > it supports P_PID and P_PIDFD.
> > >
> > > [1]  https://lore.kernel.org/linux-mm/9d849087-3359-c4ab-fbec-859e8186c509@virtuozzo.com/
> > >
> > > Cc: Christian Brauner <christian@brauner.io>
> > > Suggested-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> > > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > > ---
> > >  include/linux/syscalls.h |  3 ++-
> > >  mm/madvise.c             | 34 ++++++++++++++++++++++------------
> > >  2 files changed, 24 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > > index e4cd2c2f8bb4..f5ada20e2943 100644
> > > --- a/include/linux/syscalls.h
> > > +++ b/include/linux/syscalls.h
> > > @@ -876,7 +876,8 @@ asmlinkage long sys_munlockall(void);
> > >  asmlinkage long sys_mincore(unsigned long start, size_t len,
> > >                                 unsigned char __user * vec);
> > >  asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
> > > -asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
> > > +
> > > +asmlinkage long sys_process_madvise(int which, pid_t pid, unsigned long start,
> > >                         size_t len, int behavior, unsigned long flags);
> > >  asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
> > >                         unsigned long prot, unsigned long pgoff,
> > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > index def1507c2030..f6d9b9e66243 100644
> > > --- a/mm/madvise.c
> > > +++ b/mm/madvise.c
> > > @@ -1182,11 +1182,10 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
> > >         return do_madvise(current, current->mm, start, len_in, behavior);
> > >  }
> > >
> > > -SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > > +SYSCALL_DEFINE6(process_madvise, int, which, pid_t, upid, unsigned long, start,
> > >                 size_t, len_in, int, behavior, unsigned long, flags)
> > >  {
> > >         int ret;
> > > -       struct fd f;
> > >         struct pid *pid;
> > >         struct task_struct *task;
> > >         struct mm_struct *mm;
> > > @@ -1197,20 +1196,31 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > >         if (!process_madvise_behavior_valid(behavior))
> > >                 return -EINVAL;
> > >
> > > -       f = fdget(pidfd);
> > > -       if (!f.file)
> > > -               return -EBADF;
> > > +       switch (which) {
> > > +       case P_PID:
> > > +               if (upid <= 0)
> > > +                       return -EINVAL;
> > > +
> > > +               pid = find_get_pid(upid);
> > > +               if (!pid)
> > > +                       return -ESRCH;
> > > +               break;
> > > +       case P_PIDFD:
> > > +               if (upid < 0)
> > > +                       return -EINVAL;
> > >
> > > -       pid = pidfd_pid(f.file);
> > > -       if (IS_ERR(pid)) {
> > > -               ret = PTR_ERR(pid);
> > > -               goto fdput;
> > > +               pid = pidfd_get_pid(upid);
> > > +               if (IS_ERR(pid))
> > > +                       return PTR_ERR(pid);
> > > +               break;
> > > +       default:
> > > +               return -EINVAL;
> > >         }
> > >
> > >         task = get_pid_task(pid, PIDTYPE_PID);
> > >         if (!task) {
> > >                 ret = -ESRCH;
> > > -               goto fdput;
> > > +               goto put_pid;
> > >         }
> > >
> > >         mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> > > @@ -1223,7 +1233,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > >         mmput(mm);
> > >  release_task:
> > >         put_task_struct(task);
> > > -fdput:
> > > -       fdput(f);
> > > +put_pid:
> > > +       put_pid(pid);
> > >         return ret;
> > >  }
> > > --
> > > 2.25.0.265.gbab2e86ba0-goog
> > >
> >
> > Reviewed-by: Suren Baghdasaryan <surenb@google.com>

[PATCH v6 6/7] mm/madvise: employ mmget_still_valid for write lock

[Minchan Kim]

From: Oleksandr Natalenko <oleksandr@redhat.com>

Do the very same trick as we already do since 04f5866e41fb. KSM hints
will require locking mmap_sem for write since they modify vm_flags, so
for remote KSM hinting this additional check is needed.

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/madvise.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index f6d9b9e66243..c55a18fe71f9 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1118,6 +1118,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
 	if (write) {
 		if (down_write_killable(&mm->mmap_sem))
 			return -EINTR;
+		if (current->mm != mm && !mmget_still_valid(mm))
+			goto skip_mm;
 	} else {
 		down_read(&mm->mmap_sem);
 	}
@@ -1169,6 +1171,7 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
 	}
 out:
 	blk_finish_plug(&plug);
+skip_mm:
 	if (write)
 		up_write(&mm->mmap_sem);
 	else
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 6/7] mm/madvise: employ mmget_still_valid for write lock

[Suren Baghdasaryan]

On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
>
> From: Oleksandr Natalenko <oleksandr@redhat.com>
>
> Do the very same trick as we already do since 04f5866e41fb. KSM hints
> will require locking mmap_sem for write since they modify vm_flags, so
> for remote KSM hinting this additional check is needed.
>
> Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  mm/madvise.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index f6d9b9e66243..c55a18fe71f9 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1118,6 +1118,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
>         if (write) {
>                 if (down_write_killable(&mm->mmap_sem))
>                         return -EINTR;
> +               if (current->mm != mm && !mmget_still_valid(mm))

mmget_still_valid() seems pretty light-weight, so why not just use
that without checking that the mm belongs to the current process
first?

> +                       goto skip_mm;
>         } else {
>                 down_read(&mm->mmap_sem);
>         }
> @@ -1169,6 +1171,7 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
>         }
>  out:
>         blk_finish_plug(&plug);
> +skip_mm:
>         if (write)
>                 up_write(&mm->mmap_sem);
>         else
> --
> 2.25.0.265.gbab2e86ba0-goog
>

Re: [PATCH v6 6/7] mm/madvise: employ mmget_still_valid for write lock

[Oleksandr Natalenko]

Hello.

On Fri, Feb 28, 2020 at 03:19:55PM -0800, Suren Baghdasaryan wrote:
> On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
> >
> > From: Oleksandr Natalenko <oleksandr@redhat.com>
> >
> > Do the very same trick as we already do since 04f5866e41fb. KSM hints
> > will require locking mmap_sem for write since they modify vm_flags, so
> > for remote KSM hinting this additional check is needed.
> >
> > Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  mm/madvise.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index f6d9b9e66243..c55a18fe71f9 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -1118,6 +1118,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
> >         if (write) {
> >                 if (down_write_killable(&mm->mmap_sem))
> >                         return -EINTR;
> > +               if (current->mm != mm && !mmget_still_valid(mm))
> 
> mmget_still_valid() seems pretty light-weight, so why not just use
> that without checking that the mm belongs to the current process
> first?

I'd keep the checks separate to a) do not functionally change current->mm
== mm case; b) clearly separate the intention to call
mmget_still_valid() only for remote access (using mmget_still_valid()
for current->mm == mm does not make any sense here, IMO, since there's
no possibility of expecting a core dump at this point); c) ease the job for
reviewer once mmget_still_valid() is scheduled to be removed (I hope it
eventually goes away indeed).

> 
> > +                       goto skip_mm;
> >         } else {
> >                 down_read(&mm->mmap_sem);
> >         }
> > @@ -1169,6 +1171,7 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
> >         }
> >  out:
> >         blk_finish_plug(&plug);
> > +skip_mm:
> >         if (write)
> >                 up_write(&mm->mmap_sem);
> >         else
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
> 

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Principal Software Maintenance Engineer

Re: [PATCH v6 6/7] mm/madvise: employ mmget_still_valid for write lock

[Suren Baghdasaryan]

On Sun, Mar 1, 2020 at 11:33 PM Oleksandr Natalenko
<oleksandr@redhat.com> wrote:
>
> Hello.
>
> On Fri, Feb 28, 2020 at 03:19:55PM -0800, Suren Baghdasaryan wrote:
> > On Tue, Feb 18, 2020 at 5:44 PM Minchan Kim <minchan@kernel.org> wrote:
> > >
> > > From: Oleksandr Natalenko <oleksandr@redhat.com>
> > >
> > > Do the very same trick as we already do since 04f5866e41fb. KSM hints
> > > will require locking mmap_sem for write since they modify vm_flags, so
> > > for remote KSM hinting this additional check is needed.
> > >
> > > Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
> > > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > > ---
> > >  mm/madvise.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/mm/madvise.c b/mm/madvise.c
> > > index f6d9b9e66243..c55a18fe71f9 100644
> > > --- a/mm/madvise.c
> > > +++ b/mm/madvise.c
> > > @@ -1118,6 +1118,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
> > >         if (write) {
> > >                 if (down_write_killable(&mm->mmap_sem))
> > >                         return -EINTR;
> > > +               if (current->mm != mm && !mmget_still_valid(mm))
> >
> > mmget_still_valid() seems pretty light-weight, so why not just use
> > that without checking that the mm belongs to the current process
> > first?
>
> I'd keep the checks separate to a) do not functionally change current->mm
> == mm case; b) clearly separate the intention to call
> mmget_still_valid() only for remote access (using mmget_still_valid()
> for current->mm == mm does not make any sense here, IMO, since there's
> no possibility of expecting a core dump at this point); c) ease the job for
> reviewer once mmget_still_valid() is scheduled to be removed (I hope it
> eventually goes away indeed).
>

Makes sense. Thanks!

> >
> > > +                       goto skip_mm;
> > >         } else {
> > >                 down_read(&mm->mmap_sem);
> > >         }
> > > @@ -1169,6 +1171,7 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm,
> > >         }
> > >  out:
> > >         blk_finish_plug(&plug);
> > > +skip_mm:
> > >         if (write)
> > >                 up_write(&mm->mmap_sem);
> > >         else
> > > --
> > > 2.25.0.265.gbab2e86ba0-goog
> > >
> >
>
> --
>   Best regards,
>     Oleksandr Natalenko (post-factum)
>     Principal Software Maintenance Engineer
>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

[PATCH v6 7/7] mm/madvise: allow KSM hints for remote API

[Minchan Kim]

From: Oleksandr Natalenko <oleksandr@redhat.com>

It all began with the fact that KSM works only on memory that is marked
by madvise(). And the only way to get around that is to either:

  * use LD_PRELOAD; or
  * patch the kernel with something like UKSM or PKSM.

(i skip ptrace can of worms here intentionally)

To overcome this restriction, lets employ a new remote madvise API. This
can be used by some small userspace helper daemon that will do auto-KSM
job for us.

I think of two major consumers of remote KSM hints:

  * hosts, that run containers, especially similar ones and especially in
    a trusted environment, sharing the same runtime like Node.js;

  * heavy applications, that can be run in multiple instances, not
    limited to opensource ones like Firefox, but also those that cannot be
    modified since they are binary-only and, maybe, statically linked.

Speaking of statistics, more numbers can be found in the very first
submission, that is related to this one [1]. For my current setup with
two Firefox instances I get 100 to 200 MiB saved for the second instance
depending on the amount of tabs.

1 FF instance with 15 tabs:

   $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
   410

2 FF instances, second one has 12 tabs (all the tabs are different):

   $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
   592

At the very moment I do not have specific numbers for containerised
workload, but those should be comparable in case the containers share
similar/same runtime.

[1] https://lore.kernel.org/patchwork/patch/1012142/

Reviewed-by: SeongJae Park <sjpark@amazon.de>
Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/madvise.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index c55a18fe71f9..b97c7e1a5cab 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1005,6 +1005,10 @@ process_madvise_behavior_valid(int behavior)
 	switch (behavior) {
 	case MADV_COLD:
 	case MADV_PAGEOUT:
+#ifdef CONFIG_KSM
+	case MADV_MERGEABLE:
+	case MADV_UNMERGEABLE:
+#endif
 		return true;
 	default:
 		return false;
-- 
2.25.0.265.gbab2e86ba0-goog

Re: [PATCH v6 0/7] introduce memory hinting API for external process

[Andrew Morton]

On Tue, 18 Feb 2020 17:44:26 -0800 Minchan Kim <minchan@kernel.org> wrote:

> Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
> application could give hints to kernel what memory range are preferred to be
> reclaimed. However, in some platform(e.g., Android), the information
> required to make the hinting decision is not known to the app.
> Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
> and that daemon must be able to initiate reclaim on its own without any app
> involvement.
> 

This patchset doesn't seem to be getting a lot of interest from other
potential users?  It seems very specialized.  Are there or will there
ever be any users of this apart from one Android daemon?

Also, it doesn't terribly hard for ActivityManagerService to tell
another process "now run madvise with these arguments".  Please explain
why this is not practical in ActivityManagerService and also within
other potential users of this syscall.

Re: [PATCH v6 0/7] introduce memory hinting API for external process

[Suren Baghdasaryan]

On Wed, Feb 19, 2020 at 12:01 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Tue, 18 Feb 2020 17:44:26 -0800 Minchan Kim <minchan@kernel.org> wrote:
>
> > Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
> > application could give hints to kernel what memory range are preferred to be
> > reclaimed. However, in some platform(e.g., Android), the information
> > required to make the hinting decision is not known to the app.
> > Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
> > and that daemon must be able to initiate reclaim on its own without any app
> > involvement.
> >
>
> This patchset doesn't seem to be getting a lot of interest from other
> potential users?  It seems very specialized.  Are there or will there
> ever be any users of this apart from one Android daemon?

Don't know if this can be considered another user since it's still in
the Android realm of things.
I'm interested in extending process_madvise() to support MADV_DONTNEED
to expedite memory ripping of a process killed by Android Low Memory
Killer. But for that I need process_madvise() to be accepted first.
IIRC Crome team was interested in these madv hints as well at some point...

>
> Also, it doesn't terribly hard for ActivityManagerService to tell
> another process "now run madvise with these arguments".  Please explain
> why this is not practical in ActivityManagerService and also within
> other potential users of this syscall.
>
>

Re: [PATCH v6 0/7] introduce memory hinting API for external process

[Minchan Kim]

Hi Andrew,

On Wed, Feb 19, 2020 at 12:01:23PM -0800, Andrew Morton wrote:
> On Tue, 18 Feb 2020 17:44:26 -0800 Minchan Kim <minchan@kernel.org> wrote:
> 
> > Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
> > application could give hints to kernel what memory range are preferred to be
> > reclaimed. However, in some platform(e.g., Android), the information
> > required to make the hinting decision is not known to the app.
> > Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
> > and that daemon must be able to initiate reclaim on its own without any app
> > involvement.
> > 
> 
> This patchset doesn't seem to be getting a lot of interest from other
> potential users?  It seems very specialized.  Are there or will there
> ever be any users of this apart from one Android daemon?


Quote from http://lkml.kernel.org/r/20190531064313.193437-1-minchan@kernel.org

"
Brian Geffon in ChromeOS team had an experiment with process_madvise(2)
Quote form him:
"What I found is that by using process_madvise after a tab has been back
grounded for more than 45 seconds reduced the average tab switch times by
25%! This is a huge result and very obvious validation that process_madvise
hints works well for the ChromeOS use case."
"

> 
> Also, it doesn't terribly hard for ActivityManagerService to tell
> another process "now run madvise with these arguments".  Please explain
> why this is not practical in ActivityManagerService and also within
> other potential users of this syscall.

I think that's the almost a same question why ptrace doesn't work so
I summarizes the part in [2/7]:

* makes target task runnable creates memory layout change window so
 hiniting a wrong vma

* target task(e.g., background task) could live in little core with
  cpuset/group limited environment so we couldn't react quick enough,
  which causes more killing.


Thanks.

Re: [PATCH v6 0/7] introduce memory hinting API for external process

[Brian Geffon]

To expand on how ChromeOS benefits from this, we've advanced far
beyond the experimentation phase we've deployed an older version of
this idea that was procfs based on several ChromeOS kernels. These are
now rolled out to ChromeOS stable channel where we've been testing and
the results have been amazing. To elaborate on the setup, Chrome is a
multi process architecture where each tab is a separate process and
sometimes a single tab can even represent multiple processes. The
primary Chrome process has a lot of visibility into the amount of time
a user has been spending interacting with a tab (process) and using
this knowledge these hints provided to the kernel allow it to make
much better swap decisions and amortize the cost of swap over
different memory pressure levels meaning that we were better able to
reclaim memory which allow us to avoid having to discard tabs or even
worse oom.

I'd be happy to expand even more if anyone is interested.

Brian

On Wed, Feb 19, 2020 at 2:32 PM Minchan Kim <minchan@kernel.org> wrote:
>
> Hi Andrew,
>
> On Wed, Feb 19, 2020 at 12:01:23PM -0800, Andrew Morton wrote:
> > On Tue, 18 Feb 2020 17:44:26 -0800 Minchan Kim <minchan@kernel.org> wrote:
> >
> > > Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
> > > application could give hints to kernel what memory range are preferred to be
> > > reclaimed. However, in some platform(e.g., Android), the information
> > > required to make the hinting decision is not known to the app.
> > > Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
> > > and that daemon must be able to initiate reclaim on its own without any app
> > > involvement.
> > >
> >
> > This patchset doesn't seem to be getting a lot of interest from other
> > potential users?  It seems very specialized.  Are there or will there
> > ever be any users of this apart from one Android daemon?
>
>
> Quote from http://lkml.kernel.org/r/20190531064313.193437-1-minchan@kernel.org
>
> "
> Brian Geffon in ChromeOS team had an experiment with process_madvise(2)
> Quote form him:
> "What I found is that by using process_madvise after a tab has been back
> grounded for more than 45 seconds reduced the average tab switch times by
> 25%! This is a huge result and very obvious validation that process_madvise
> hints works well for the ChromeOS use case."
> "
>
> >
> > Also, it doesn't terribly hard for ActivityManagerService to tell
> > another process "now run madvise with these arguments".  Please explain
> > why this is not practical in ActivityManagerService and also within
> > other potential users of this syscall.
>
> I think that's the almost a same question why ptrace doesn't work so
> I summarizes the part in [2/7]:
>
> * makes target task runnable creates memory layout change window so
>  hiniting a wrong vma
>
> * target task(e.g., background task) could live in little core with
>   cpuset/group limited environment so we couldn't react quick enough,
>   which causes more killing.
>
>
> Thanks.

Re: Re: [PATCH v6 0/7] introduce memory hinting API for external process

[SeongJae Park]

On Wed, 19 Feb 2020 12:01:23 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 18 Feb 2020 17:44:26 -0800 Minchan Kim <minchan@kernel.org> wrote:
> 
> > Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
> > application could give hints to kernel what memory range are preferred to be
> > reclaimed. However, in some platform(e.g., Android), the information
> > required to make the hinting decision is not known to the app.
> > Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
> > and that daemon must be able to initiate reclaim on its own without any app
> > involvement.
> > 
> 
> This patchset doesn't seem to be getting a lot of interest from other
> potential users?  It seems very specialized.  Are there or will there
> ever be any users of this apart from one Android daemon?
> 
> Also, it doesn't terribly hard for ActivityManagerService to tell
> another process "now run madvise with these arguments".  Please explain
> why this is not practical in ActivityManagerService and also within
> other potential users of this syscall.

I personally have interest in and hope successful development/merge of this
patchset.

The interested usecases of 'madvise_process()' for me is optimizations of
general memory-intensive workloads having dynamic data access patterns on
hierarchical memory systems (e.g., multi-tier memory or fast storage based swap
devices).  In more detail, I'm already using a part of this patchset for my RFC
patchset implementing Data Access Monitoring-based Operation Schemes[1].  For
my specific case, I don't need new system call but only target task argument,
though.

Once in a past, before joining my current company, I tried using 'madvise()' to
optimize some scientific HPC programs.  The improvement results were clear, but
optimizing each of the workloads was challenging and time-consuming.  I believe
this new systemcall to be very helpful for such cases, either.

[1] https://lore.kernel.org/linux-mm/20200218085309.18346-1-sjpark@amazon.com/


Thanks,
SeongJae Park