From: Suren Baghdasaryan <surenb@google.com>
To: Minchan Kim <minchan@kernel.org>
Cc: kbuild test robot <lkp@intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
kbuild-all@lists.01.org,
Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [PATCH v6 2/7] mm: introduce external memory hinting API
Date: Fri, 28 Feb 2020 14:14:56 -0800 [thread overview]
Message-ID: <CAJuCfpEJCJR-N9bKBPMVqC3NgEXJYF3RG46cG9Y_3MEXq4F1tQ@mail.gmail.com> (raw)
In-Reply-To: <20200220212135.GB226145@google.com>
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>
WARNING: multiple messages have this Message-ID (diff)
From: Suren Baghdasaryan <surenb@google.com>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH v6 2/7] mm: introduce external memory hinting API
Date: Fri, 28 Feb 2020 14:14:56 -0800 [thread overview]
Message-ID: <CAJuCfpEJCJR-N9bKBPMVqC3NgEXJYF3RG46cG9Y_3MEXq4F1tQ@mail.gmail.com> (raw)
In-Reply-To: <20200220212135.GB226145@google.com>
[-- Attachment #1: Type: text/plain, Size: 29544 bytes --]
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(a)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(a)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(a)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>
next prev parent reply other threads:[~2020-02-28 22:15 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-19 1:44 [PATCH v6 0/7] introduce memory hinting API for external process Minchan Kim
2020-02-19 1:44 ` [PATCH v6 1/7] mm: pass task and mm to do_madvise Minchan Kim
2020-02-28 22:15 ` Suren Baghdasaryan
2020-02-28 22:15 ` Suren Baghdasaryan
2020-02-19 1:44 ` [PATCH v6 2/7] mm: introduce external memory hinting API Minchan Kim
2020-02-20 19:13 ` kbuild test robot
2020-02-20 19:13 ` kbuild test robot
2020-02-20 21:15 ` Minchan Kim
2020-02-20 21:15 ` Minchan Kim
2020-02-20 21:21 ` Minchan Kim
2020-02-20 21:21 ` Minchan Kim
2020-02-28 22:14 ` Suren Baghdasaryan [this message]
2020-02-28 22:14 ` Suren Baghdasaryan
2020-03-02 19:18 ` Minchan Kim
2020-03-02 19:18 ` Minchan Kim
2020-02-20 20:48 ` kbuild test robot
2020-02-20 20:48 ` kbuild test robot
2020-02-19 1:44 ` [PATCH v6 3/7] mm: check fatal signal pending of target process Minchan Kim
2020-02-28 22:20 ` Suren Baghdasaryan
2020-02-28 22:20 ` Suren Baghdasaryan
2020-02-19 1:44 ` [PATCH v6 4/7] pid: move pidfd_get_pid function to pid.c Minchan Kim
2020-02-28 22:22 ` Suren Baghdasaryan
2020-02-28 22:22 ` Suren Baghdasaryan
2020-02-19 1:44 ` [PATCH v6 5/7] mm: support both pid and pidfd for process_madvise Minchan Kim
2020-02-28 22:41 ` Suren Baghdasaryan
2020-02-28 22:41 ` Suren Baghdasaryan
2020-03-02 19:23 ` Minchan Kim
2020-03-02 19:38 ` Suren Baghdasaryan
2020-03-02 19:38 ` Suren Baghdasaryan
2020-02-19 1:44 ` [PATCH v6 6/7] mm/madvise: employ mmget_still_valid for write lock Minchan Kim
2020-02-28 23:19 ` Suren Baghdasaryan
2020-02-28 23:19 ` Suren Baghdasaryan
2020-03-02 7:33 ` Oleksandr Natalenko
2020-03-02 16:32 ` Suren Baghdasaryan
2020-03-02 16:32 ` Suren Baghdasaryan
2020-02-19 1:44 ` [PATCH v6 7/7] mm/madvise: allow KSM hints for remote API Minchan Kim
2020-02-19 20:01 ` [PATCH v6 0/7] introduce memory hinting API for external process Andrew Morton
2020-02-19 21:05 ` Suren Baghdasaryan
2020-02-19 21:05 ` Suren Baghdasaryan
2020-02-19 22:32 ` Minchan Kim
2020-02-19 22:51 ` Brian Geffon
2020-02-19 22:51 ` Brian Geffon
2020-02-20 9:16 ` SeongJae Park
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJuCfpEJCJR-N9bKBPMVqC3NgEXJYF3RG46cG9Y_3MEXq4F1tQ@mail.gmail.com \
--to=surenb@google.com \
--cc=akpm@linux-foundation.org \
--cc=kbuild-all@lists.01.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=minchan@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.