| Message ID | 20190520035254.57579-8-minchan@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce memory hinting API for external process | expand |
[cc linux-api] On Mon 20-05-19 12:52:54, Minchan Kim wrote: > System could have much faster swap device like zRAM. In that case, swapping > is extremely cheaper than file-IO on the low-end storage. > In this configuration, userspace could handle different strategy for each > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > while it keeps file-backed pages in inactive LRU by MADV_COOL because > file IO is more expensive in this case so want to keep them in memory > until memory pressure happens. > > To support such strategy easier, this patch introduces > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > that /proc/<pid>/clear_refs already has supported same filters. > They are filters could be Ored with other existing hints using top two bits > of (int behavior). madvise operates on top of ranges and it is quite trivial to do the filtering from the userspace so why do we need any additional filtering? > Once either of them is set, the hint could affect only the interested vma > either anonymous or file-backed. > > With that, user could call a process_madvise syscall simply with a entire > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > MADV_FILE_FILTER so there is no need to call the syscall range by range. OK, so here is the reason you want that. The immediate question is why cannot the monitor do the filtering from the userspace. Slightly more work, all right, but less of an API to expose and that itself is a strong argument against. > * from v1r2 > * use consistent check with clear_refs to identify anon/file vma - surenb > > * from v1r1 > * use naming "filter" for new madvise option - dancol > > Signed-off-by: Minchan Kim <minchan@kernel.org> > --- > include/uapi/asm-generic/mman-common.h | 5 +++++ > mm/madvise.c | 14 ++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h > index b8e230de84a6..be59a1b90284 100644 > --- a/include/uapi/asm-generic/mman-common.h > +++ b/include/uapi/asm-generic/mman-common.h > @@ -66,6 +66,11 @@ > #define MADV_WIPEONFORK 18 /* Zero memory on fork, child only */ > #define MADV_KEEPONFORK 19 /* Undo MADV_WIPEONFORK */ > > +#define MADV_BEHAVIOR_MASK (~(MADV_ANONYMOUS_FILTER|MADV_FILE_FILTER)) > + > +#define MADV_ANONYMOUS_FILTER (1<<31) /* works for only anonymous vma */ > +#define MADV_FILE_FILTER (1<<30) /* works for only file-backed vma */ > + > /* compatibility flags */ > #define MAP_FILE 0 > > diff --git a/mm/madvise.c b/mm/madvise.c > index f4f569dac2bd..116131243540 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -1002,7 +1002,15 @@ static int madvise_core(struct task_struct *tsk, unsigned long start, > int write; > size_t len; > struct blk_plug plug; > + bool anon_only, file_only; > > + anon_only = behavior & MADV_ANONYMOUS_FILTER; > + file_only = behavior & MADV_FILE_FILTER; > + > + if (anon_only && file_only) > + return error; > + > + behavior = behavior & MADV_BEHAVIOR_MASK; > if (!madvise_behavior_valid(behavior)) > return error; > > @@ -1067,12 +1075,18 @@ static int madvise_core(struct task_struct *tsk, unsigned long start, > if (end < tmp) > tmp = end; > > + if (anon_only && vma->vm_file) > + goto next; > + if (file_only && !vma->vm_file) > + goto next; > + > /* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */ > error = madvise_vma(tsk, vma, &prev, start, tmp, > behavior, &pages); > if (error) > goto out; > *nr_pages += pages; > +next: > start = tmp; > if (prev && start < prev->vm_end) > start = prev->vm_end; > -- > 2.21.0.1020.gf2820cf01a-goog >
On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > [cc linux-api] > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > System could have much faster swap device like zRAM. In that case, swapping > > is extremely cheaper than file-IO on the low-end storage. > > In this configuration, userspace could handle different strategy for each > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > file IO is more expensive in this case so want to keep them in memory > > until memory pressure happens. > > > > To support such strategy easier, this patch introduces > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > that /proc/<pid>/clear_refs already has supported same filters. > > They are filters could be Ored with other existing hints using top two bits > > of (int behavior). > > madvise operates on top of ranges and it is quite trivial to do the > filtering from the userspace so why do we need any additional filtering? > > > Once either of them is set, the hint could affect only the interested vma > > either anonymous or file-backed. > > > > With that, user could call a process_madvise syscall simply with a entire > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > OK, so here is the reason you want that. The immediate question is why > cannot the monitor do the filtering from the userspace. Slightly more > work, all right, but less of an API to expose and that itself is a > strong argument against. What I should do if we don't have such filter option is to enumerate all of vma via /proc/<pid>/maps and then parse every ranges and inode from string, which would be painful for 2000+ vmas. > > > * from v1r2 > > * use consistent check with clear_refs to identify anon/file vma - surenb > > > > * from v1r1 > > * use naming "filter" for new madvise option - dancol > > > > Signed-off-by: Minchan Kim <minchan@kernel.org> > > --- > > include/uapi/asm-generic/mman-common.h | 5 +++++ > > mm/madvise.c | 14 ++++++++++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h > > index b8e230de84a6..be59a1b90284 100644 > > --- a/include/uapi/asm-generic/mman-common.h > > +++ b/include/uapi/asm-generic/mman-common.h > > @@ -66,6 +66,11 @@ > > #define MADV_WIPEONFORK 18 /* Zero memory on fork, child only */ > > #define MADV_KEEPONFORK 19 /* Undo MADV_WIPEONFORK */ > > > > +#define MADV_BEHAVIOR_MASK (~(MADV_ANONYMOUS_FILTER|MADV_FILE_FILTER)) > > + > > +#define MADV_ANONYMOUS_FILTER (1<<31) /* works for only anonymous vma */ > > +#define MADV_FILE_FILTER (1<<30) /* works for only file-backed vma */ > > + > > /* compatibility flags */ > > #define MAP_FILE 0 > > > > diff --git a/mm/madvise.c b/mm/madvise.c > > index f4f569dac2bd..116131243540 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -1002,7 +1002,15 @@ static int madvise_core(struct task_struct *tsk, unsigned long start, > > int write; > > size_t len; > > struct blk_plug plug; > > + bool anon_only, file_only; > > > > + anon_only = behavior & MADV_ANONYMOUS_FILTER; > > + file_only = behavior & MADV_FILE_FILTER; > > + > > + if (anon_only && file_only) > > + return error; > > + > > + behavior = behavior & MADV_BEHAVIOR_MASK; > > if (!madvise_behavior_valid(behavior)) > > return error; > > > > @@ -1067,12 +1075,18 @@ static int madvise_core(struct task_struct *tsk, unsigned long start, > > if (end < tmp) > > tmp = end; > > > > + if (anon_only && vma->vm_file) > > + goto next; > > + if (file_only && !vma->vm_file) > > + goto next; > > + > > /* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */ > > error = madvise_vma(tsk, vma, &prev, start, tmp, > > behavior, &pages); > > if (error) > > goto out; > > *nr_pages += pages; > > +next: > > start = tmp; > > if (prev && start < prev->vm_end) > > start = prev->vm_end; > > -- > > 2.21.0.1020.gf2820cf01a-goog > > > > -- > Michal Hocko > SUSE Labs
On Tue 21-05-19 11:55:33, Minchan Kim wrote: > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > [cc linux-api] > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > System could have much faster swap device like zRAM. In that case, swapping > > > is extremely cheaper than file-IO on the low-end storage. > > > In this configuration, userspace could handle different strategy for each > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > file IO is more expensive in this case so want to keep them in memory > > > until memory pressure happens. > > > > > > To support such strategy easier, this patch introduces > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > that /proc/<pid>/clear_refs already has supported same filters. > > > They are filters could be Ored with other existing hints using top two bits > > > of (int behavior). > > > > madvise operates on top of ranges and it is quite trivial to do the > > filtering from the userspace so why do we need any additional filtering? > > > > > Once either of them is set, the hint could affect only the interested vma > > > either anonymous or file-backed. > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > OK, so here is the reason you want that. The immediate question is why > > cannot the monitor do the filtering from the userspace. Slightly more > > work, all right, but less of an API to expose and that itself is a > > strong argument against. > > What I should do if we don't have such filter option is to enumerate all of > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > which would be painful for 2000+ vmas. Painful is not an argument to add a new user API. If the existing API suits the purpose then it should be used. If it is not usable, we can think of a different way.
On Tue, May 21, 2019 at 11:55:33AM +0900, Minchan Kim wrote: > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > [cc linux-api] > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > System could have much faster swap device like zRAM. In that case, swapping > > > is extremely cheaper than file-IO on the low-end storage. > > > In this configuration, userspace could handle different strategy for each > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > file IO is more expensive in this case so want to keep them in memory > > > until memory pressure happens. > > > > > > To support such strategy easier, this patch introduces > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > that /proc/<pid>/clear_refs already has supported same filters. > > > They are filters could be Ored with other existing hints using top two bits > > > of (int behavior). > > > > madvise operates on top of ranges and it is quite trivial to do the > > filtering from the userspace so why do we need any additional filtering? > > > > > Once either of them is set, the hint could affect only the interested vma > > > either anonymous or file-backed. > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > OK, so here is the reason you want that. The immediate question is why > > cannot the monitor do the filtering from the userspace. Slightly more > > work, all right, but less of an API to expose and that itself is a > > strong argument against. > > What I should do if we don't have such filter option is to enumerate all of > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > which would be painful for 2000+ vmas. Just out of curiosity, how do you get to 2000+ distinct memory regions in the address space of a mobile app? I'm assuming these aren't files, but rather anon objects with poor grouping. Is that from guard pages between individual heap allocations or something?
On Tue, May 21, 2019 at 11:33:10AM -0400, Johannes Weiner wrote: > On Tue, May 21, 2019 at 11:55:33AM +0900, Minchan Kim wrote: > > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > > [cc linux-api] > > > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > > System could have much faster swap device like zRAM. In that case, swapping > > > > is extremely cheaper than file-IO on the low-end storage. > > > > In this configuration, userspace could handle different strategy for each > > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > > file IO is more expensive in this case so want to keep them in memory > > > > until memory pressure happens. > > > > > > > > To support such strategy easier, this patch introduces > > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > > that /proc/<pid>/clear_refs already has supported same filters. > > > > They are filters could be Ored with other existing hints using top two bits > > > > of (int behavior). > > > > > > madvise operates on top of ranges and it is quite trivial to do the > > > filtering from the userspace so why do we need any additional filtering? > > > > > > > Once either of them is set, the hint could affect only the interested vma > > > > either anonymous or file-backed. > > > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > > > OK, so here is the reason you want that. The immediate question is why > > > cannot the monitor do the filtering from the userspace. Slightly more > > > work, all right, but less of an API to expose and that itself is a > > > strong argument against. > > > > What I should do if we don't have such filter option is to enumerate all of > > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > > which would be painful for 2000+ vmas. > > Just out of curiosity, how do you get to 2000+ distinct memory regions > in the address space of a mobile app? I'm assuming these aren't files, > but rather anon objects with poor grouping. Is that from guard pages > between individual heap allocations or something? Android uses preload library model to speed up app launch so it loads all of library in advance on zygote and forks new app based on it.
On Tue, May 21, 2019 at 08:26:28AM +0200, Michal Hocko wrote: > On Tue 21-05-19 11:55:33, Minchan Kim wrote: > > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > > [cc linux-api] > > > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > > System could have much faster swap device like zRAM. In that case, swapping > > > > is extremely cheaper than file-IO on the low-end storage. > > > > In this configuration, userspace could handle different strategy for each > > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > > file IO is more expensive in this case so want to keep them in memory > > > > until memory pressure happens. > > > > > > > > To support such strategy easier, this patch introduces > > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > > that /proc/<pid>/clear_refs already has supported same filters. > > > > They are filters could be Ored with other existing hints using top two bits > > > > of (int behavior). > > > > > > madvise operates on top of ranges and it is quite trivial to do the > > > filtering from the userspace so why do we need any additional filtering? > > > > > > > Once either of them is set, the hint could affect only the interested vma > > > > either anonymous or file-backed. > > > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > > > OK, so here is the reason you want that. The immediate question is why > > > cannot the monitor do the filtering from the userspace. Slightly more > > > work, all right, but less of an API to expose and that itself is a > > > strong argument against. > > > > What I should do if we don't have such filter option is to enumerate all of > > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > > which would be painful for 2000+ vmas. > > Painful is not an argument to add a new user API. If the existing API > suits the purpose then it should be used. If it is not usable, we can > think of a different way. I measured 1568 vma parsing overhead of /proc/<pid>/maps in ARM64 modern mobile CPU. It takes 60ms and 185ms on big cores depending on cpu governor. It's never trivial.
On Mon 27-05-19 16:58:11, Minchan Kim wrote: > On Tue, May 21, 2019 at 08:26:28AM +0200, Michal Hocko wrote: > > On Tue 21-05-19 11:55:33, Minchan Kim wrote: > > > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > > > [cc linux-api] > > > > > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > > > System could have much faster swap device like zRAM. In that case, swapping > > > > > is extremely cheaper than file-IO on the low-end storage. > > > > > In this configuration, userspace could handle different strategy for each > > > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > > > file IO is more expensive in this case so want to keep them in memory > > > > > until memory pressure happens. > > > > > > > > > > To support such strategy easier, this patch introduces > > > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > > > that /proc/<pid>/clear_refs already has supported same filters. > > > > > They are filters could be Ored with other existing hints using top two bits > > > > > of (int behavior). > > > > > > > > madvise operates on top of ranges and it is quite trivial to do the > > > > filtering from the userspace so why do we need any additional filtering? > > > > > > > > > Once either of them is set, the hint could affect only the interested vma > > > > > either anonymous or file-backed. > > > > > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > > > > > OK, so here is the reason you want that. The immediate question is why > > > > cannot the monitor do the filtering from the userspace. Slightly more > > > > work, all right, but less of an API to expose and that itself is a > > > > strong argument against. > > > > > > What I should do if we don't have such filter option is to enumerate all of > > > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > > > which would be painful for 2000+ vmas. > > > > Painful is not an argument to add a new user API. If the existing API > > suits the purpose then it should be used. If it is not usable, we can > > think of a different way. > > I measured 1568 vma parsing overhead of /proc/<pid>/maps in ARM64 modern > mobile CPU. It takes 60ms and 185ms on big cores depending on cpu governor. > It's never trivial. This is not the only option. Have you tried to simply use /proc/<pid>/map_files interface? This will provide you with all the file backed mappings.
On Mon, May 27, 2019 at 02:44:11PM +0200, Michal Hocko wrote: > On Mon 27-05-19 16:58:11, Minchan Kim wrote: > > On Tue, May 21, 2019 at 08:26:28AM +0200, Michal Hocko wrote: > > > On Tue 21-05-19 11:55:33, Minchan Kim wrote: > > > > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > > > > [cc linux-api] > > > > > > > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > > > > System could have much faster swap device like zRAM. In that case, swapping > > > > > > is extremely cheaper than file-IO on the low-end storage. > > > > > > In this configuration, userspace could handle different strategy for each > > > > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > > > > file IO is more expensive in this case so want to keep them in memory > > > > > > until memory pressure happens. > > > > > > > > > > > > To support such strategy easier, this patch introduces > > > > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > > > > that /proc/<pid>/clear_refs already has supported same filters. > > > > > > They are filters could be Ored with other existing hints using top two bits > > > > > > of (int behavior). > > > > > > > > > > madvise operates on top of ranges and it is quite trivial to do the > > > > > filtering from the userspace so why do we need any additional filtering? > > > > > > > > > > > Once either of them is set, the hint could affect only the interested vma > > > > > > either anonymous or file-backed. > > > > > > > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > > > > > > > OK, so here is the reason you want that. The immediate question is why > > > > > cannot the monitor do the filtering from the userspace. Slightly more > > > > > work, all right, but less of an API to expose and that itself is a > > > > > strong argument against. > > > > > > > > What I should do if we don't have such filter option is to enumerate all of > > > > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > > > > which would be painful for 2000+ vmas. > > > > > > Painful is not an argument to add a new user API. If the existing API > > > suits the purpose then it should be used. If it is not usable, we can > > > think of a different way. > > > > I measured 1568 vma parsing overhead of /proc/<pid>/maps in ARM64 modern > > mobile CPU. It takes 60ms and 185ms on big cores depending on cpu governor. > > It's never trivial. > > This is not the only option. Have you tried to simply use > /proc/<pid>/map_files interface? This will provide you with all the file > backed mappings. I compared maps vs. map_files with 3036 file-backed vma. Test scenario is to dump all of vmas of the process and parse address ranges. For map_files, it's easy to parse each address range because directory name itself is range. However, in case of maps, I need to parse each range line by line so need to scan all of lines. (maps cover additional non-file-backed vmas so nr_vma is a little bigger) performance mode: map_files: nr_vma 3036 usec 13387 maps : nr_vma 3078 usec 12923 powersave mode: map_files: nr_vma 3036 usec 52614 maps : nr_vma 3078 usec 41089 map_files is slower than maps if we dump all of vmas. I guess directory operation needs much more jobs(e.g., dentry lookup, instantiation) compared to maps.
On Tue 28-05-19 12:26:32, Minchan Kim wrote: > On Mon, May 27, 2019 at 02:44:11PM +0200, Michal Hocko wrote: > > On Mon 27-05-19 16:58:11, Minchan Kim wrote: > > > On Tue, May 21, 2019 at 08:26:28AM +0200, Michal Hocko wrote: > > > > On Tue 21-05-19 11:55:33, Minchan Kim wrote: > > > > > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > > > > > [cc linux-api] > > > > > > > > > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > > > > > System could have much faster swap device like zRAM. In that case, swapping > > > > > > > is extremely cheaper than file-IO on the low-end storage. > > > > > > > In this configuration, userspace could handle different strategy for each > > > > > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > > > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > > > > > file IO is more expensive in this case so want to keep them in memory > > > > > > > until memory pressure happens. > > > > > > > > > > > > > > To support such strategy easier, this patch introduces > > > > > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > > > > > that /proc/<pid>/clear_refs already has supported same filters. > > > > > > > They are filters could be Ored with other existing hints using top two bits > > > > > > > of (int behavior). > > > > > > > > > > > > madvise operates on top of ranges and it is quite trivial to do the > > > > > > filtering from the userspace so why do we need any additional filtering? > > > > > > > > > > > > > Once either of them is set, the hint could affect only the interested vma > > > > > > > either anonymous or file-backed. > > > > > > > > > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > > > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > > > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > > > > > > > > > OK, so here is the reason you want that. The immediate question is why > > > > > > cannot the monitor do the filtering from the userspace. Slightly more > > > > > > work, all right, but less of an API to expose and that itself is a > > > > > > strong argument against. > > > > > > > > > > What I should do if we don't have such filter option is to enumerate all of > > > > > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > > > > > which would be painful for 2000+ vmas. > > > > > > > > Painful is not an argument to add a new user API. If the existing API > > > > suits the purpose then it should be used. If it is not usable, we can > > > > think of a different way. > > > > > > I measured 1568 vma parsing overhead of /proc/<pid>/maps in ARM64 modern > > > mobile CPU. It takes 60ms and 185ms on big cores depending on cpu governor. > > > It's never trivial. > > > > This is not the only option. Have you tried to simply use > > /proc/<pid>/map_files interface? This will provide you with all the file > > backed mappings. > > I compared maps vs. map_files with 3036 file-backed vma. > Test scenario is to dump all of vmas of the process and parse address > ranges. > For map_files, it's easy to parse each address range because directory name > itself is range. However, in case of maps, I need to parse each range > line by line so need to scan all of lines. > > (maps cover additional non-file-backed vmas so nr_vma is a little bigger) > > performance mode: > map_files: nr_vma 3036 usec 13387 > maps : nr_vma 3078 usec 12923 > > powersave mode: > > map_files: nr_vma 3036 usec 52614 > maps : nr_vma 3078 usec 41089 > > map_files is slower than maps if we dump all of vmas. I guess directory > operation needs much more jobs(e.g., dentry lookup, instantiation) > compared to maps. OK, that is somehow surprising. I am still not convinced the filter is a good idea though. The primary reason is that it encourages using madvise on a wide range without having a clue what the range contains. E.g. the full address range and rely the right thing will happen. Do we really want madvise to operate in that mode? Btw. if we went with the per vma fd approach then you would get this feature automatically because map_files would refer to file backed mappings while map_anon could refer only to anonymous mappings.
On Tue, May 28, 2019 at 08:29:47AM +0200, Michal Hocko wrote: > On Tue 28-05-19 12:26:32, Minchan Kim wrote: > > On Mon, May 27, 2019 at 02:44:11PM +0200, Michal Hocko wrote: > > > On Mon 27-05-19 16:58:11, Minchan Kim wrote: > > > > On Tue, May 21, 2019 at 08:26:28AM +0200, Michal Hocko wrote: > > > > > On Tue 21-05-19 11:55:33, Minchan Kim wrote: > > > > > > On Mon, May 20, 2019 at 11:28:01AM +0200, Michal Hocko wrote: > > > > > > > [cc linux-api] > > > > > > > > > > > > > > On Mon 20-05-19 12:52:54, Minchan Kim wrote: > > > > > > > > System could have much faster swap device like zRAM. In that case, swapping > > > > > > > > is extremely cheaper than file-IO on the low-end storage. > > > > > > > > In this configuration, userspace could handle different strategy for each > > > > > > > > kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD > > > > > > > > while it keeps file-backed pages in inactive LRU by MADV_COOL because > > > > > > > > file IO is more expensive in this case so want to keep them in memory > > > > > > > > until memory pressure happens. > > > > > > > > > > > > > > > > To support such strategy easier, this patch introduces > > > > > > > > MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like > > > > > > > > that /proc/<pid>/clear_refs already has supported same filters. > > > > > > > > They are filters could be Ored with other existing hints using top two bits > > > > > > > > of (int behavior). > > > > > > > > > > > > > > madvise operates on top of ranges and it is quite trivial to do the > > > > > > > filtering from the userspace so why do we need any additional filtering? > > > > > > > > > > > > > > > Once either of them is set, the hint could affect only the interested vma > > > > > > > > either anonymous or file-backed. > > > > > > > > > > > > > > > > With that, user could call a process_madvise syscall simply with a entire > > > > > > > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > > > > > > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > > > > > > > > > > > > OK, so here is the reason you want that. The immediate question is why > > > > > > > cannot the monitor do the filtering from the userspace. Slightly more > > > > > > > work, all right, but less of an API to expose and that itself is a > > > > > > > strong argument against. > > > > > > > > > > > > What I should do if we don't have such filter option is to enumerate all of > > > > > > vma via /proc/<pid>/maps and then parse every ranges and inode from string, > > > > > > which would be painful for 2000+ vmas. > > > > > > > > > > Painful is not an argument to add a new user API. If the existing API > > > > > suits the purpose then it should be used. If it is not usable, we can > > > > > think of a different way. > > > > > > > > I measured 1568 vma parsing overhead of /proc/<pid>/maps in ARM64 modern > > > > mobile CPU. It takes 60ms and 185ms on big cores depending on cpu governor. > > > > It's never trivial. > > > > > > This is not the only option. Have you tried to simply use > > > /proc/<pid>/map_files interface? This will provide you with all the file > > > backed mappings. > > > > I compared maps vs. map_files with 3036 file-backed vma. > > Test scenario is to dump all of vmas of the process and parse address > > ranges. > > For map_files, it's easy to parse each address range because directory name > > itself is range. However, in case of maps, I need to parse each range > > line by line so need to scan all of lines. > > > > (maps cover additional non-file-backed vmas so nr_vma is a little bigger) > > > > performance mode: > > map_files: nr_vma 3036 usec 13387 > > maps : nr_vma 3078 usec 12923 > > > > powersave mode: > > > > map_files: nr_vma 3036 usec 52614 > > maps : nr_vma 3078 usec 41089 > > > > map_files is slower than maps if we dump all of vmas. I guess directory > > operation needs much more jobs(e.g., dentry lookup, instantiation) > > compared to maps. > > OK, that is somehow surprising. I am still not convinced the filter is a > good idea though. The primary reason is that it encourages using madvise > on a wide range without having a clue what the range contains. E.g. the > full address range and rely the right thing will happen. Do we really > want madvise to operate in that mode? If user space daemon(e.g., activity manager service) could know a certain process is bakground and idle for a while, yeb, that would be good option. > > Btw. if we went with the per vma fd approach then you would get this > feature automatically because map_files would refer to file backed > mappings while map_anon could refer only to anonymous mappings. The reason to add such filter option is to avoid the parsing overhead so map_anon wouldn't be helpful. > > -- > Michal Hocko > SUSE Labs
On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > if we went with the per vma fd approach then you would get this > > feature automatically because map_files would refer to file backed > > mappings while map_anon could refer only to anonymous mappings. > > The reason to add such filter option is to avoid the parsing overhead > so map_anon wouldn't be helpful. Without chiming on whether the filter option is a good idea, I'd like to suggest that providing an efficient binary interfaces for pulling memory map information out of processes. Some single-system-call method for retrieving a binary snapshot of a process's address space complete with attributes (selectable, like statx?) for each VMA would reduce complexity and increase performance in a variety of areas, e.g., Android memory map debugging commands.
On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > if we went with the per vma fd approach then you would get this > > > feature automatically because map_files would refer to file backed > > > mappings while map_anon could refer only to anonymous mappings. > > > > The reason to add such filter option is to avoid the parsing overhead > > so map_anon wouldn't be helpful. > > Without chiming on whether the filter option is a good idea, I'd like > to suggest that providing an efficient binary interfaces for pulling > memory map information out of processes. Some single-system-call > method for retrieving a binary snapshot of a process's address space > complete with attributes (selectable, like statx?) for each VMA would > reduce complexity and increase performance in a variety of areas, > e.g., Android memory map debugging commands. I agree it's the best we can get *generally*. Michal, any opinion?
On Tue 28-05-19 17:49:27, Minchan Kim wrote: > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > if we went with the per vma fd approach then you would get this > > > > feature automatically because map_files would refer to file backed > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > so map_anon wouldn't be helpful. > > > > Without chiming on whether the filter option is a good idea, I'd like > > to suggest that providing an efficient binary interfaces for pulling > > memory map information out of processes. Some single-system-call > > method for retrieving a binary snapshot of a process's address space > > complete with attributes (selectable, like statx?) for each VMA would > > reduce complexity and increase performance in a variety of areas, > > e.g., Android memory map debugging commands. > > I agree it's the best we can get *generally*. > Michal, any opinion? I am not really sure this is directly related. I think the primary question that we have to sort out first is whether we want to have the remote madvise call process or vma fd based. This is an important distinction wrt. usability. I have only seen pid vs. pidfd discussions so far unfortunately. An interface to query address range information is a separate but although a related topic. We have /proc/<pid>/[s]maps for that right now and I understand it is not a general win for all usecases because it tends to be slow for some. I can see how /proc/<pid>/map_anons could provide per vma information in a binary form via a fd based interface. But I would rather not conflate those two discussions much - well except if it could give one of the approaches more justification but let's focus on the madvise part first.
On Tue, May 28, 2019 at 2:08 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > if we went with the per vma fd approach then you would get this > > > > > feature automatically because map_files would refer to file backed > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > so map_anon wouldn't be helpful. > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > to suggest that providing an efficient binary interfaces for pulling > > > memory map information out of processes. Some single-system-call > > > method for retrieving a binary snapshot of a process's address space > > > complete with attributes (selectable, like statx?) for each VMA would > > > reduce complexity and increase performance in a variety of areas, > > > e.g., Android memory map debugging commands. > > > > I agree it's the best we can get *generally*. > > Michal, any opinion? > > I am not really sure this is directly related. I think the primary > question that we have to sort out first is whether we want to have > the remote madvise call process or vma fd based. This is an important > distinction wrt. usability. I have only seen pid vs. pidfd discussions > so far unfortunately. I don't think the vma fd approach is viable. We have some processes with a *lot* of VMAs --- system_server had 4204 when I checked just now (and that's typical) --- and an FD operation per VMA would be excessive. VMAs also come and go pretty easily depending on changes in protections and various faults. It's also not entirely clear what the semantics of vma FDs should be over address space mutations, while the semantics of address ranges are well-understood. I would much prefer an interface operating on address ranges to one operating on VMA FDs, both for efficiency and for consistency with other memory management APIs. > An interface to query address range information is a separate but > although a related topic. We have /proc/<pid>/[s]maps for that right > now and I understand it is not a general win for all usecases because > it tends to be slow for some. I can see how /proc/<pid>/map_anons could > provide per vma information in a binary form via a fd based interface. > But I would rather not conflate those two discussions much - well except > if it could give one of the approaches more justification but let's > focus on the madvise part first. I don't think it's a good idea to focus on one feature in a multi-feature change when the interactions between features can be very important for overall design of the multi-feature system and the design of each feature. Here's my thinking on the high-level design: I'm imagining an address-range system that would work like this: we'd create some kind of process_vm_getinfo(2) system call [1] that would accept a statx-like attribute map and a pid/fd parameter as input and return, on output, two things: 1) an array [2] of VMA descriptors containing the requested information, and 2) a VMA configuration sequence number. We'd then have process_madvise() and other cross-process VM interfaces accept both address ranges and this sequence number; they'd succeed only if the VMA configuration sequence number is still current, i.e., the target process hasn't changed its VMA configuration (implicitly or explicitly) since the call to process_vm_getinfo(). This way, a process A that wants to perform some VM operation on process B can slurp B's VMA configuration using process_vm_getinfo(), figure out what it wants to do, and attempt to do it. If B modifies its memory map in the meantime, If A finds that its local knowledge of B's memory map has become invalid between the process_vm_getinfo() and A taking some action based on the result, A can retry [3]. While A could instead ptrace or otherwise suspend B, *then* read B's memory map (knowing B is quiescent), *then* operate on B, the optimistic approach I'm describing would be much lighter-weight in the typical case. It's also pretty simple, IMHO. If the "operate on B" step is some kind of vectorized operation over multiple address ranges, this approach also gets us all-or-nothing semantics. Or maybe the whole sequence number thing is overkill and we don't need atomicity? But if there's a concern that A shouldn't operate on B's memory without knowing what it's operating on, then the scheme I've proposed above solves this knowledge problem in a pretty lightweight way. [1] or some other interface [2] or something more complicated if we want the descriptors to contain variable-length elements, e.g., strings [3] or override the sequence number check if it's feeling bold?
On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > if we went with the per vma fd approach then you would get this > > > > > feature automatically because map_files would refer to file backed > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > so map_anon wouldn't be helpful. > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > to suggest that providing an efficient binary interfaces for pulling > > > memory map information out of processes. Some single-system-call > > > method for retrieving a binary snapshot of a process's address space > > > complete with attributes (selectable, like statx?) for each VMA would > > > reduce complexity and increase performance in a variety of areas, > > > e.g., Android memory map debugging commands. > > > > I agree it's the best we can get *generally*. > > Michal, any opinion? > > I am not really sure this is directly related. I think the primary > question that we have to sort out first is whether we want to have > the remote madvise call process or vma fd based. This is an important > distinction wrt. usability. I have only seen pid vs. pidfd discussions > so far unfortunately. With current usecase, it's per-process API with distinguishable anon/file but thought it could be easily extended later for each address range operation as userspace getting smarter with more information.
On Tue 28-05-19 02:39:03, Daniel Colascione wrote: > On Tue, May 28, 2019 at 2:08 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > if we went with the per vma fd approach then you would get this > > > > > > feature automatically because map_files would refer to file backed > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > so map_anon wouldn't be helpful. > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > to suggest that providing an efficient binary interfaces for pulling > > > > memory map information out of processes. Some single-system-call > > > > method for retrieving a binary snapshot of a process's address space > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > reduce complexity and increase performance in a variety of areas, > > > > e.g., Android memory map debugging commands. > > > > > > I agree it's the best we can get *generally*. > > > Michal, any opinion? > > > > I am not really sure this is directly related. I think the primary > > question that we have to sort out first is whether we want to have > > the remote madvise call process or vma fd based. This is an important > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > so far unfortunately. > > I don't think the vma fd approach is viable. We have some processes > with a *lot* of VMAs --- system_server had 4204 when I checked just > now (and that's typical) --- and an FD operation per VMA would be > excessive. What do you mean by excessive here? Do you expect the process to have them open all at once? > VMAs also come and go pretty easily depending on changes in > protections and various faults. Is this really too much different from /proc/<pid>/map_files? [...] > > An interface to query address range information is a separate but > > although a related topic. We have /proc/<pid>/[s]maps for that right > > now and I understand it is not a general win for all usecases because > > it tends to be slow for some. I can see how /proc/<pid>/map_anons could > > provide per vma information in a binary form via a fd based interface. > > But I would rather not conflate those two discussions much - well except > > if it could give one of the approaches more justification but let's > > focus on the madvise part first. > > I don't think it's a good idea to focus on one feature in a > multi-feature change when the interactions between features can be > very important for overall design of the multi-feature system and the > design of each feature. > > Here's my thinking on the high-level design: > > I'm imagining an address-range system that would work like this: we'd > create some kind of process_vm_getinfo(2) system call [1] that would > accept a statx-like attribute map and a pid/fd parameter as input and > return, on output, two things: 1) an array [2] of VMA descriptors > containing the requested information, and 2) a VMA configuration > sequence number. We'd then have process_madvise() and other > cross-process VM interfaces accept both address ranges and this > sequence number; they'd succeed only if the VMA configuration sequence > number is still current, i.e., the target process hasn't changed its > VMA configuration (implicitly or explicitly) since the call to > process_vm_getinfo(). The sequence number is essentially a cookie that is transparent to the userspace right? If yes, how does it differ from a fd (returned from /proc/<pid>/map_{anons,files}/range) which is a cookie itself and it can be used to revalidate when the operation is requested and fail if something has changed. Moreover we already do have a fd based madvise syscall so there shouldn't be really a large need to add a new set of syscalls. [...] > Or maybe the whole sequence number thing is overkill and we don't need > atomicity? But if there's a concern that A shouldn't operate on B's > memory without knowing what it's operating on, then the scheme I've > proposed above solves this knowledge problem in a pretty lightweight > way. This is the main question here. Do we really want to enforce an external synchronization between the two processes to make sure that they are both operating on the same range - aka protect from the range going away and being reused for a different purpose. Right now it wouldn't be fatal because both operations are non destructive but I can imagine that there will be more madvise operations to follow (including those that are destructive) because people will simply find usecases for that. This should be reflected in the proposed API.
On Tue 28-05-19 19:32:56, Minchan Kim wrote: > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > if we went with the per vma fd approach then you would get this > > > > > > feature automatically because map_files would refer to file backed > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > so map_anon wouldn't be helpful. > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > to suggest that providing an efficient binary interfaces for pulling > > > > memory map information out of processes. Some single-system-call > > > > method for retrieving a binary snapshot of a process's address space > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > reduce complexity and increase performance in a variety of areas, > > > > e.g., Android memory map debugging commands. > > > > > > I agree it's the best we can get *generally*. > > > Michal, any opinion? > > > > I am not really sure this is directly related. I think the primary > > question that we have to sort out first is whether we want to have > > the remote madvise call process or vma fd based. This is an important > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > so far unfortunately. > > With current usecase, it's per-process API with distinguishable anon/file > but thought it could be easily extended later for each address range > operation as userspace getting smarter with more information. Never design user API based on a single usecase, please. The "easily extended" part is by far not clear to me TBH. As I've already mentioned several times, the synchronization model has to be thought through carefuly before a remote process address range operation can be implemented.
On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > memory map information out of processes. Some single-system-call > > > > > method for retrieving a binary snapshot of a process's address space > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > reduce complexity and increase performance in a variety of areas, > > > > > e.g., Android memory map debugging commands. > > > > > > > > I agree it's the best we can get *generally*. > > > > Michal, any opinion? > > > > > > I am not really sure this is directly related. I think the primary > > > question that we have to sort out first is whether we want to have > > > the remote madvise call process or vma fd based. This is an important > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > so far unfortunately. > > > > With current usecase, it's per-process API with distinguishable anon/file > > but thought it could be easily extended later for each address range > > operation as userspace getting smarter with more information. > > Never design user API based on a single usecase, please. The "easily > extended" part is by far not clear to me TBH. As I've already mentioned > several times, the synchronization model has to be thought through > carefuly before a remote process address range operation can be > implemented. I agree with you that we shouldn't design API on single usecase but what you are concerning is actually not our usecase because we are resilient with the race since MADV_COLD|PAGEOUT is not destruptive. Actually, many hints are already racy in that the upcoming pattern would be different with the behavior you thought at the moment. If you are still concerning of address range synchronization, how about moving such hints to per-process level like prctl? Does it make sense to you?
On Tue, May 28, 2019 at 3:33 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 28-05-19 02:39:03, Daniel Colascione wrote: > > On Tue, May 28, 2019 at 2:08 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > memory map information out of processes. Some single-system-call > > > > > method for retrieving a binary snapshot of a process's address space > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > reduce complexity and increase performance in a variety of areas, > > > > > e.g., Android memory map debugging commands. > > > > > > > > I agree it's the best we can get *generally*. > > > > Michal, any opinion? > > > > > > I am not really sure this is directly related. I think the primary > > > question that we have to sort out first is whether we want to have > > > the remote madvise call process or vma fd based. This is an important > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > so far unfortunately. > > > > I don't think the vma fd approach is viable. We have some processes > > with a *lot* of VMAs --- system_server had 4204 when I checked just > > now (and that's typical) --- and an FD operation per VMA would be > > excessive. > > What do you mean by excessive here? Do you expect the process to have > them open all at once? Minchan's already done timing. More broadly, in an era with various speculative execution mitigations, making a system call is pretty expensive. If we have two options for remote VMA manipulation, one that requires thousands of system calls (with the count proportional to the address space size of the process) and one that requires only a few system calls no matter how large the target process is, the latter ought to start off with more points than the former under any kind of design scoring. > > VMAs also come and go pretty easily depending on changes in > > protections and various faults. > > Is this really too much different from /proc/<pid>/map_files? It's very different. See below. > > > An interface to query address range information is a separate but > > > although a related topic. We have /proc/<pid>/[s]maps for that right > > > now and I understand it is not a general win for all usecases because > > > it tends to be slow for some. I can see how /proc/<pid>/map_anons could > > > provide per vma information in a binary form via a fd based interface. > > > But I would rather not conflate those two discussions much - well except > > > if it could give one of the approaches more justification but let's > > > focus on the madvise part first. > > > > I don't think it's a good idea to focus on one feature in a > > multi-feature change when the interactions between features can be > > very important for overall design of the multi-feature system and the > > design of each feature. > > > > Here's my thinking on the high-level design: > > > > I'm imagining an address-range system that would work like this: we'd > > create some kind of process_vm_getinfo(2) system call [1] that would > > accept a statx-like attribute map and a pid/fd parameter as input and > > return, on output, two things: 1) an array [2] of VMA descriptors > > containing the requested information, and 2) a VMA configuration > > sequence number. We'd then have process_madvise() and other > > cross-process VM interfaces accept both address ranges and this > > sequence number; they'd succeed only if the VMA configuration sequence > > number is still current, i.e., the target process hasn't changed its > > VMA configuration (implicitly or explicitly) since the call to > > process_vm_getinfo(). > > The sequence number is essentially a cookie that is transparent to the > userspace right? If yes, how does it differ from a fd (returned from > /proc/<pid>/map_{anons,files}/range) which is a cookie itself and it can If you want to operate on N VMAs simultaneously under an FD-per-VMA model, you'd need to have those N FDs all open at the same time *and* add some kind of system call that accepted those N FDs and an operation to perform. The sequence number I'm proposing also applies to the whole address space, not just one VMA. Even if you did have these N FDs open all at once and supplied them all to some batch operation, you couldn't guarantee via the FD mechanism that some *new* VMA didn't appear in the address range you want to manipulate. A global sequence number would catch this case. I still think supplying a list of address ranges (like we already do for scatter-gather IO) is less error-prone, less resource-intensive, more consistent with existing practice, and equally flexible, especially if we start supporting destructive cross-process memory operations, which may be useful for things like checkpointing and optimizing process startup. Besides: process_vm_readv and process_vm_writev already work on address ranges. Why should other cross-process memory APIs use a very different model for naming memory regions? > be used to revalidate when the operation is requested and fail if > something has changed. Moreover we already do have a fd based madvise > syscall so there shouldn't be really a large need to add a new set of > syscalls. We have various system calls that provide hints for open files, but the memory operations are distinct. Modeling anonymous memory as a kind of file-backed memory for purposes of VMA manipulation would also be a departure from existing practice. Can you help me understand why you seem to favor the FD-per-VMA approach so heavily? I don't see any arguments *for* an FD-per-VMA model for remove memory manipulation and I see a lot of arguments against it. Is there some compelling advantage I'm missing? > > Or maybe the whole sequence number thing is overkill and we don't need > > atomicity? But if there's a concern that A shouldn't operate on B's > > memory without knowing what it's operating on, then the scheme I've > > proposed above solves this knowledge problem in a pretty lightweight > > way. > > This is the main question here. Do we really want to enforce an external > synchronization between the two processes to make sure that they are > both operating on the same range - aka protect from the range going away > and being reused for a different purpose. Right now it wouldn't be fatal > because both operations are non destructive but I can imagine that there > will be more madvise operations to follow (including those that are > destructive) because people will simply find usecases for that. This > should be reflected in the proposed API. A sequence number gives us this synchronization at very low cost and adds safety. It's also a general-purpose mechanism that would safeguard *any* cross-process VM operation, not just the VM operations we're discussing right now.
On Tue 28-05-19 20:12:08, Minchan Kim wrote: > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > memory map information out of processes. Some single-system-call > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > Michal, any opinion? > > > > > > > > I am not really sure this is directly related. I think the primary > > > > question that we have to sort out first is whether we want to have > > > > the remote madvise call process or vma fd based. This is an important > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > so far unfortunately. > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > but thought it could be easily extended later for each address range > > > operation as userspace getting smarter with more information. > > > > Never design user API based on a single usecase, please. The "easily > > extended" part is by far not clear to me TBH. As I've already mentioned > > several times, the synchronization model has to be thought through > > carefuly before a remote process address range operation can be > > implemented. > > I agree with you that we shouldn't design API on single usecase but what > you are concerning is actually not our usecase because we are resilient > with the race since MADV_COLD|PAGEOUT is not destruptive. > Actually, many hints are already racy in that the upcoming pattern would > be different with the behavior you thought at the moment. How come they are racy wrt address ranges? You would have to be in multithreaded environment and then the onus of synchronization is on threads. That model is quite clear. But we are talking about separate processes and some of them might be even not aware of an external entity tweaking their address space. > If you are still concerning of address range synchronization, how about > moving such hints to per-process level like prctl? > Does it make sense to you? No it doesn't. How is prctl any relevant to any address range operations.
On Tue, May 28, 2019 at 3:41 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > memory map information out of processes. Some single-system-call > > > > > method for retrieving a binary snapshot of a process's address space > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > reduce complexity and increase performance in a variety of areas, > > > > > e.g., Android memory map debugging commands. > > > > > > > > I agree it's the best we can get *generally*. > > > > Michal, any opinion? > > > > > > I am not really sure this is directly related. I think the primary > > > question that we have to sort out first is whether we want to have > > > the remote madvise call process or vma fd based. This is an important > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > so far unfortunately. > > > > With current usecase, it's per-process API with distinguishable anon/file > > but thought it could be easily extended later for each address range > > operation as userspace getting smarter with more information. > > Never design user API based on a single usecase, please. The "easily > extended" part is by far not clear to me TBH. As I've already mentioned > several times, the synchronization model has to be thought through > carefuly before a remote process address range operation can be > implemented. I don't think anyone is overfitting for a specific use case. When some process A wants to manipulate process B's memory, it's fair for A to want to know what memory it's manipulating. That's a general concern that applies to a large family of cross-process memory operations. It's less important for non-destructive hints than for some kind of destructive operation, but the same idea applies. If there's a simple way to solve this A-B information problem in a general way, it seems to be that we should apply that general solution. Likewise, an API to get an efficiently-collected snapshot of a process's address space would be immediately useful in several very different use cases, including debuggers, Android memory use reporting tools, and various kinds of metric collection. Because we're talking about mechanisms that solve several independent problems at the same time and in a general way, it doesn't sound to me like overfitting for a particular use case.
On Tue, May 28, 2019 at 4:28 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > Michal, any opinion? > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > question that we have to sort out first is whether we want to have > > > > > the remote madvise call process or vma fd based. This is an important > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > so far unfortunately. > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > but thought it could be easily extended later for each address range > > > > operation as userspace getting smarter with more information. > > > > > > Never design user API based on a single usecase, please. The "easily > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > several times, the synchronization model has to be thought through > > > carefuly before a remote process address range operation can be > > > implemented. > > > > I agree with you that we shouldn't design API on single usecase but what > > you are concerning is actually not our usecase because we are resilient > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > Actually, many hints are already racy in that the upcoming pattern would > > be different with the behavior you thought at the moment. > > How come they are racy wrt address ranges? You would have to be in > multithreaded environment and then the onus of synchronization is on > threads. That model is quite clear. But we are talking about separate > processes and some of them might be even not aware of an external entity > tweaking their address space. I don't think the difference between a thread and a process matters in this context. Threads race on address space operations all the time --- in the sense that multiple threads modify a process's address space without synchronization. The main reasons that these races hasn't been a problem are: 1) threads mostly "mind their own business" and modify different parts of the address space or use locks to ensure that they don't stop on each other (e.g., the malloc heap lock), and 2) POSIX mmap atomic-replacement semantics make certain classes of operation (like "magic ring buffer" setup) safe even in the presence of other threads stomping over an address space. The thing that's new in this discussion from a synchronization point of view isn't that the VM operation we're talking about is coming from outside the process, but that we want to do a read-decide-modify-ish thing. We want to affect (using various hints) classes of pages like "all file pages" or "all anonymous pages" or "some pages referring to graphics buffers up to 100MB" (to pick an example off the top of my head of a policy that might make sense). From a synchronization point of view, it doesn't really matter whether it's a thread within the target process or a thread outside the target process that does the address space manipulation. What's new is the inspection of the address space before performing an operation. Minchan started this thread by proposing some flags that would implement a few of the filtering policies I used as examples above. Personally, instead of providing a few pre-built policies as flags, I'd rather push the page manipulation policy to userspace as much as possible and just have the kernel provide a mechanism that *in general* makes these read-decide-modify operations efficient and robust. I still think there's way to achieve this goal very inexpensively without compromising on flexibility.
On Tue, May 28, 2019 at 01:28:40PM +0200, Michal Hocko wrote: > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > Michal, any opinion? > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > question that we have to sort out first is whether we want to have > > > > > the remote madvise call process or vma fd based. This is an important > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > so far unfortunately. > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > but thought it could be easily extended later for each address range > > > > operation as userspace getting smarter with more information. > > > > > > Never design user API based on a single usecase, please. The "easily > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > several times, the synchronization model has to be thought through > > > carefuly before a remote process address range operation can be > > > implemented. > > > > I agree with you that we shouldn't design API on single usecase but what > > you are concerning is actually not our usecase because we are resilient > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > Actually, many hints are already racy in that the upcoming pattern would > > be different with the behavior you thought at the moment. > > How come they are racy wrt address ranges? You would have to be in > multithreaded environment and then the onus of synchronization is on > threads. That model is quite clear. But we are talking about separate Think about MADV_FREE. Allocator would think the chunk is worth to mark "freeable" but soon, user of the allocator asked the chunk - ie, it's not freeable any longer once user start to use it. My point is that kinds of *hints* are always racy so any synchronization couldn't help a lot. That's why I want to restrict hints process_madvise supports as such kinds of non-destruptive one at next respin. > processes and some of them might be even not aware of an external entity > tweaking their address space. > > > If you are still concerning of address range synchronization, how about > > moving such hints to per-process level like prctl? > > Does it make sense to you? > > No it doesn't. How is prctl any relevant to any address range > operations. "whether we want to have the remote madvise call process or vma fd based." You asked the above question and I answered we are using process level hints but anon/vma filter at this moment. That's why I told you prctl to make forward progress on discussion.
On Tue 28-05-19 04:21:44, Daniel Colascione wrote: > On Tue, May 28, 2019 at 3:33 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Tue 28-05-19 02:39:03, Daniel Colascione wrote: > > > On Tue, May 28, 2019 at 2:08 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > memory map information out of processes. Some single-system-call > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > Michal, any opinion? > > > > > > > > I am not really sure this is directly related. I think the primary > > > > question that we have to sort out first is whether we want to have > > > > the remote madvise call process or vma fd based. This is an important > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > so far unfortunately. > > > > > > I don't think the vma fd approach is viable. We have some processes > > > with a *lot* of VMAs --- system_server had 4204 when I checked just > > > now (and that's typical) --- and an FD operation per VMA would be > > > excessive. > > > > What do you mean by excessive here? Do you expect the process to have > > them open all at once? > > Minchan's already done timing. More broadly, in an era with various > speculative execution mitigations, making a system call is pretty > expensive. This is a completely separate discussion. This could be argued about many other syscalls. Let's make the semantic correct first before we even start thinking about mutliplexing. It is easier to multiplex on an existing and sane interface. Btw. Minchan concluded that multiplexing is not really all that important based on his numbers http://lkml.kernel.org/r/20190527074940.GB6879@google.com [...] > > Is this really too much different from /proc/<pid>/map_files? > > It's very different. See below. > > > > > An interface to query address range information is a separate but > > > > although a related topic. We have /proc/<pid>/[s]maps for that right > > > > now and I understand it is not a general win for all usecases because > > > > it tends to be slow for some. I can see how /proc/<pid>/map_anons could > > > > provide per vma information in a binary form via a fd based interface. > > > > But I would rather not conflate those two discussions much - well except > > > > if it could give one of the approaches more justification but let's > > > > focus on the madvise part first. > > > > > > I don't think it's a good idea to focus on one feature in a > > > multi-feature change when the interactions between features can be > > > very important for overall design of the multi-feature system and the > > > design of each feature. > > > > > > Here's my thinking on the high-level design: > > > > > > I'm imagining an address-range system that would work like this: we'd > > > create some kind of process_vm_getinfo(2) system call [1] that would > > > accept a statx-like attribute map and a pid/fd parameter as input and > > > return, on output, two things: 1) an array [2] of VMA descriptors > > > containing the requested information, and 2) a VMA configuration > > > sequence number. We'd then have process_madvise() and other > > > cross-process VM interfaces accept both address ranges and this > > > sequence number; they'd succeed only if the VMA configuration sequence > > > number is still current, i.e., the target process hasn't changed its > > > VMA configuration (implicitly or explicitly) since the call to > > > process_vm_getinfo(). > > > > The sequence number is essentially a cookie that is transparent to the > > userspace right? If yes, how does it differ from a fd (returned from > > /proc/<pid>/map_{anons,files}/range) which is a cookie itself and it can > > If you want to operate on N VMAs simultaneously under an FD-per-VMA > model, you'd need to have those N FDs all open at the same time *and* > add some kind of system call that accepted those N FDs and an > operation to perform. The sequence number I'm proposing also applies > to the whole address space, not just one VMA. Even if you did have > these N FDs open all at once and supplied them all to some batch > operation, you couldn't guarantee via the FD mechanism that some *new* > VMA didn't appear in the address range you want to manipulate. A > global sequence number would catch this case. I still think supplying > a list of address ranges (like we already do for scatter-gather IO) is > less error-prone, less resource-intensive, more consistent with > existing practice, and equally flexible, especially if we start > supporting destructive cross-process memory operations, which may be > useful for things like checkpointing and optimizing process startup. I have a strong feeling you are over optimizing here. We are talking about a pro-active memory management and so far I haven't heard any usecase where all this would happen in the fast path. There are essentially two usecases I have heard so far. Age/Reclaim the whole process (with anon/fs preferency) and do the same on a particular and well specified range (e.g. a garbage collector or an inactive large image in browsert etc...). The former doesn't really care about parallel address range manipulations because it can tolerate them. The later is a completely different story. Are there any others where saving few ms matter so much? > Besides: process_vm_readv and process_vm_writev already work on > address ranges. Why should other cross-process memory APIs use a very > different model for naming memory regions? I would consider those APIs not a great example. They are racy on more levels (pid reuse and address space modification), and require a non-trivial synchronization. Do you want something similar for madvise on a non-cooperating remote application? > > be used to revalidate when the operation is requested and fail if > > something has changed. Moreover we already do have a fd based madvise > > syscall so there shouldn't be really a large need to add a new set of > > syscalls. > > We have various system calls that provide hints for open files, but > the memory operations are distinct. Modeling anonymous memory as a > kind of file-backed memory for purposes of VMA manipulation would also > be a departure from existing practice. Can you help me understand why > you seem to favor the FD-per-VMA approach so heavily? I don't see any > arguments *for* an FD-per-VMA model for remove memory manipulation and > I see a lot of arguments against it. Is there some compelling > advantage I'm missing? First and foremost it provides an easy cookie to the userspace to guarantee time-to-check-time-to-use consistency. It also naturally extend an existing fadvise interface that achieves madvise semantic on files. I am not really pushing hard for this particular API but I really do care about a programming model that would be sane. If we have a different means to achieve the same then all fine by me but so far I haven't heard any sound arguments to invent something completely new when we have established APIs to use. Exporting anonymous mappings via proc the same way we do for file mappings doesn't seem to be stepping outside of the current practice way too much. All I am trying to say here is that process_madvise(fd, start, len) is inherently racy API and we should focus on discussing whether this is a sane model. And I think it would be much better to discuss that under the respective patch which introduces that API rather than here.
On Tue, May 28, 2019 at 4:44 AM Minchan Kim <minchan@kernel.org> wrote: > > On Tue, May 28, 2019 at 01:28:40PM +0200, Michal Hocko wrote: > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > > Michal, any opinion? > > > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > > question that we have to sort out first is whether we want to have > > > > > > the remote madvise call process or vma fd based. This is an important > > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > > so far unfortunately. > > > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > > but thought it could be easily extended later for each address range > > > > > operation as userspace getting smarter with more information. > > > > > > > > Never design user API based on a single usecase, please. The "easily > > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > > several times, the synchronization model has to be thought through > > > > carefuly before a remote process address range operation can be > > > > implemented. > > > > > > I agree with you that we shouldn't design API on single usecase but what > > > you are concerning is actually not our usecase because we are resilient > > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > > Actually, many hints are already racy in that the upcoming pattern would > > > be different with the behavior you thought at the moment. > > > > How come they are racy wrt address ranges? You would have to be in > > multithreaded environment and then the onus of synchronization is on > > threads. That model is quite clear. But we are talking about separate > > Think about MADV_FREE. Allocator would think the chunk is worth to mark > "freeable" but soon, user of the allocator asked the chunk - ie, it's not > freeable any longer once user start to use it. > > My point is that kinds of *hints* are always racy so any synchronization > couldn't help a lot. That's why I want to restrict hints process_madvise > supports as such kinds of non-destruptive one at next respin. I think it's more natural for process_madvise to be a superset of regular madvise. What's the harm? There are no security implications, since anyone who could process_madvise could just ptrace anyway. I also don't think limiting the hinting to non-destructive operations guarantees safety (in a broad sense) either, since operating on the wrong memory range can still cause unexpected system performance issues even if there's no data loss. More broadly, what I want to see is a family of process_* APIs that provide a superset of the functionality that the existing intraprocess APIs provide. I think this approach is elegant and generalizes easily. I'm worried about prematurely limiting the interprocess memory APIs and creating limitations that will last a long time in order to avoid having to consider issues like VMA synchronization.
On Tue 28-05-19 04:42:47, Daniel Colascione wrote: > On Tue, May 28, 2019 at 4:28 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > > Michal, any opinion? > > > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > > question that we have to sort out first is whether we want to have > > > > > > the remote madvise call process or vma fd based. This is an important > > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > > so far unfortunately. > > > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > > but thought it could be easily extended later for each address range > > > > > operation as userspace getting smarter with more information. > > > > > > > > Never design user API based on a single usecase, please. The "easily > > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > > several times, the synchronization model has to be thought through > > > > carefuly before a remote process address range operation can be > > > > implemented. > > > > > > I agree with you that we shouldn't design API on single usecase but what > > > you are concerning is actually not our usecase because we are resilient > > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > > Actually, many hints are already racy in that the upcoming pattern would > > > be different with the behavior you thought at the moment. > > > > How come they are racy wrt address ranges? You would have to be in > > multithreaded environment and then the onus of synchronization is on > > threads. That model is quite clear. But we are talking about separate > > processes and some of them might be even not aware of an external entity > > tweaking their address space. > > I don't think the difference between a thread and a process matters in > this context. Threads race on address space operations all the time > --- in the sense that multiple threads modify a process's address > space without synchronization. I would disagree. They do have in-kernel synchronization as long as they do not use MAP_FIXED. If they do want to use MAP_FIXED then they better synchronize or the result is undefined. > The main reasons that these races > hasn't been a problem are: 1) threads mostly "mind their own business" > and modify different parts of the address space or use locks to ensure > that they don't stop on each other (e.g., the malloc heap lock), and > 2) POSIX mmap atomic-replacement semantics make certain classes of > operation (like "magic ring buffer" setup) safe even in the presence > of other threads stomping over an address space. Agreed here. [...] > From a synchronization point > of view, it doesn't really matter whether it's a thread within the > target process or a thread outside the target process that does the > address space manipulation. What's new is the inspection of the > address space before performing an operation. The fundamental difference is that if you want to achieve the same inside the process then your application is inherenly aware of the operation and use whatever synchronization is needed to achieve a consistency. As soon as you allow the same from outside you either have to have an aware target application as well or you need a mechanism to find out that your decision has been invalidated by a later unsynchronized action. > Minchan started this thread by proposing some flags that would > implement a few of the filtering policies I used as examples above. > Personally, instead of providing a few pre-built policies as flags, > I'd rather push the page manipulation policy to userspace as much as > possible and just have the kernel provide a mechanism that *in > general* makes these read-decide-modify operations efficient and > robust. I still think there's way to achieve this goal very > inexpensively without compromising on flexibility. Agreed here.
On Tue 28-05-19 20:44:36, Minchan Kim wrote: > On Tue, May 28, 2019 at 01:28:40PM +0200, Michal Hocko wrote: > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > > Michal, any opinion? > > > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > > question that we have to sort out first is whether we want to have > > > > > > the remote madvise call process or vma fd based. This is an important > > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > > so far unfortunately. > > > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > > but thought it could be easily extended later for each address range > > > > > operation as userspace getting smarter with more information. > > > > > > > > Never design user API based on a single usecase, please. The "easily > > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > > several times, the synchronization model has to be thought through > > > > carefuly before a remote process address range operation can be > > > > implemented. > > > > > > I agree with you that we shouldn't design API on single usecase but what > > > you are concerning is actually not our usecase because we are resilient > > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > > Actually, many hints are already racy in that the upcoming pattern would > > > be different with the behavior you thought at the moment. > > > > How come they are racy wrt address ranges? You would have to be in > > multithreaded environment and then the onus of synchronization is on > > threads. That model is quite clear. But we are talking about separate > > Think about MADV_FREE. Allocator would think the chunk is worth to mark > "freeable" but soon, user of the allocator asked the chunk - ie, it's not > freeable any longer once user start to use it. That is not a race in the address space, right. The underlying object hasn't changed. It has been declared as freeable and since that moment nobody can rely on the content because it might have been discarded. Or put simply, the content is undefined. It is responsibility of the madvise caller to make sure that the object is not in active use while it is marking it. > My point is that kinds of *hints* are always racy so any synchronization > couldn't help a lot. That's why I want to restrict hints process_madvise > supports as such kinds of non-destruptive one at next respin. I agree that a non-destructive operations are safer against paralel modifications because you just get a annoying and unexpected latency at worst case. But we should discuss whether this assumption is sufficient for further development. I am pretty sure once we open remote madvise people will find usecases for destructive operations or even new madvise modes we haven't heard of. What then? > > processes and some of them might be even not aware of an external entity > > tweaking their address space. > > > > > If you are still concerning of address range synchronization, how about > > > moving such hints to per-process level like prctl? > > > Does it make sense to you? > > > > No it doesn't. How is prctl any relevant to any address range > > operations. > > "whether we want to have the remote madvise call process or vma fd based." Still not following. So you want to have a prctl (one of the worst API we have along with ioctl) to tell the semantic? This sounds like a terrible idea to me.
On Tue, May 28, 2019 at 04:42:47AM -0700, Daniel Colascione wrote: > On Tue, May 28, 2019 at 4:28 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > > Michal, any opinion? > > > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > > question that we have to sort out first is whether we want to have > > > > > > the remote madvise call process or vma fd based. This is an important > > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > > so far unfortunately. > > > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > > but thought it could be easily extended later for each address range > > > > > operation as userspace getting smarter with more information. > > > > > > > > Never design user API based on a single usecase, please. The "easily > > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > > several times, the synchronization model has to be thought through > > > > carefuly before a remote process address range operation can be > > > > implemented. > > > > > > I agree with you that we shouldn't design API on single usecase but what > > > you are concerning is actually not our usecase because we are resilient > > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > > Actually, many hints are already racy in that the upcoming pattern would > > > be different with the behavior you thought at the moment. > > > > How come they are racy wrt address ranges? You would have to be in > > multithreaded environment and then the onus of synchronization is on > > threads. That model is quite clear. But we are talking about separate > > processes and some of them might be even not aware of an external entity > > tweaking their address space. > > I don't think the difference between a thread and a process matters in > this context. Threads race on address space operations all the time > --- in the sense that multiple threads modify a process's address > space without synchronization. The main reasons that these races > hasn't been a problem are: 1) threads mostly "mind their own business" > and modify different parts of the address space or use locks to ensure > that they don't stop on each other (e.g., the malloc heap lock), and > 2) POSIX mmap atomic-replacement semantics make certain classes of > operation (like "magic ring buffer" setup) safe even in the presence > of other threads stomping over an address space. > > The thing that's new in this discussion from a synchronization point > of view isn't that the VM operation we're talking about is coming from > outside the process, but that we want to do a read-decide-modify-ish > thing. We want to affect (using various hints) classes of pages like > "all file pages" or "all anonymous pages" or "some pages referring to > graphics buffers up to 100MB" (to pick an example off the top of my > head of a policy that might make sense). From a synchronization point > of view, it doesn't really matter whether it's a thread within the > target process or a thread outside the target process that does the > address space manipulation. What's new is the inspection of the > address space before performing an operation. > > Minchan started this thread by proposing some flags that would > implement a few of the filtering policies I used as examples above. > Personally, instead of providing a few pre-built policies as flags, > I'd rather push the page manipulation policy to userspace as much as > possible and just have the kernel provide a mechanism that *in > general* makes these read-decide-modify operations efficient and > robust. I still think there's way to achieve this goal very > inexpensively without compromising on flexibility. I'm looking forward to seeing the way. ;-)
On Tue, May 28, 2019 at 4:49 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 28-05-19 04:21:44, Daniel Colascione wrote: > > On Tue, May 28, 2019 at 3:33 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > > > On Tue 28-05-19 02:39:03, Daniel Colascione wrote: > > > > On Tue, May 28, 2019 at 2:08 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > Michal, any opinion? > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > question that we have to sort out first is whether we want to have > > > > > the remote madvise call process or vma fd based. This is an important > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > so far unfortunately. > > > > > > > > I don't think the vma fd approach is viable. We have some processes > > > > with a *lot* of VMAs --- system_server had 4204 when I checked just > > > > now (and that's typical) --- and an FD operation per VMA would be > > > > excessive. > > > > > > What do you mean by excessive here? Do you expect the process to have > > > them open all at once? > > > > Minchan's already done timing. More broadly, in an era with various > > speculative execution mitigations, making a system call is pretty > > expensive. > > This is a completely separate discussion. This could be argued about > many other syscalls. Yes, it can be. That's why we have scatter-gather IO system calls in the first place. > Let's make the semantic correct first before we > even start thinking about mutliplexing. It is easier to multiplex on an > existing and sane interface. I don't think of it as "multiplexing" yet, not in the fundamental unit of operation is the address range. > Btw. Minchan concluded that multiplexing is not really all that > important based on his numbers http://lkml.kernel.org/r/20190527074940.GB6879@google.com > > [...] > > > > Is this really too much different from /proc/<pid>/map_files? > > > > It's very different. See below. > > > > > > > An interface to query address range information is a separate but > > > > > although a related topic. We have /proc/<pid>/[s]maps for that right > > > > > now and I understand it is not a general win for all usecases because > > > > > it tends to be slow for some. I can see how /proc/<pid>/map_anons could > > > > > provide per vma information in a binary form via a fd based interface. > > > > > But I would rather not conflate those two discussions much - well except > > > > > if it could give one of the approaches more justification but let's > > > > > focus on the madvise part first. > > > > > > > > I don't think it's a good idea to focus on one feature in a > > > > multi-feature change when the interactions between features can be > > > > very important for overall design of the multi-feature system and the > > > > design of each feature. > > > > > > > > Here's my thinking on the high-level design: > > > > > > > > I'm imagining an address-range system that would work like this: we'd > > > > create some kind of process_vm_getinfo(2) system call [1] that would > > > > accept a statx-like attribute map and a pid/fd parameter as input and > > > > return, on output, two things: 1) an array [2] of VMA descriptors > > > > containing the requested information, and 2) a VMA configuration > > > > sequence number. We'd then have process_madvise() and other > > > > cross-process VM interfaces accept both address ranges and this > > > > sequence number; they'd succeed only if the VMA configuration sequence > > > > number is still current, i.e., the target process hasn't changed its > > > > VMA configuration (implicitly or explicitly) since the call to > > > > process_vm_getinfo(). > > > > > > The sequence number is essentially a cookie that is transparent to the > > > userspace right? If yes, how does it differ from a fd (returned from > > > /proc/<pid>/map_{anons,files}/range) which is a cookie itself and it can > > > > If you want to operate on N VMAs simultaneously under an FD-per-VMA > > model, you'd need to have those N FDs all open at the same time *and* > > add some kind of system call that accepted those N FDs and an > > operation to perform. The sequence number I'm proposing also applies > > to the whole address space, not just one VMA. Even if you did have > > these N FDs open all at once and supplied them all to some batch > > operation, you couldn't guarantee via the FD mechanism that some *new* > > VMA didn't appear in the address range you want to manipulate. A > > global sequence number would catch this case. I still think supplying > > a list of address ranges (like we already do for scatter-gather IO) is > > less error-prone, less resource-intensive, more consistent with > > existing practice, and equally flexible, especially if we start > > supporting destructive cross-process memory operations, which may be > > useful for things like checkpointing and optimizing process startup. > > I have a strong feeling you are over optimizing here. We are talking > about a pro-active memory management and so far I haven't heard any > usecase where all this would happen in the fast path. There are > essentially two usecases I have heard so far. Age/Reclaim the whole > process (with anon/fs preferency) and do the same on a particular > and well specified range (e.g. a garbage collector or an inactive large > image in browsert etc...). The former doesn't really care about parallel > address range manipulations because it can tolerate them. The later is a > completely different story. > > Are there any others where saving few ms matter so much? Saving ms matters quite a bit. We may want to perform some of this eager memory management in response to user activity, e.g., application switch, and even if that work isn't quite synchronous, every cycle the system spends on management overhead is a cycle it can't spend on rendering frames. Overhead means jank. Additionally, we're on battery-operated devices. Milliseconds of CPU overhead accumulated over a long time is a real energy sink. > > Besides: process_vm_readv and process_vm_writev already work on > > address ranges. Why should other cross-process memory APIs use a very > > different model for naming memory regions? > > I would consider those APIs not a great example. They are racy on > more levels (pid reuse and address space modification), and require a > non-trivial synchronization. Do you want something similar for madvise > on a non-cooperating remote application? > > > > be used to revalidate when the operation is requested and fail if > > > something has changed. Moreover we already do have a fd based madvise > > > syscall so there shouldn't be really a large need to add a new set of > > > syscalls. > > > > We have various system calls that provide hints for open files, but > > the memory operations are distinct. Modeling anonymous memory as a > > kind of file-backed memory for purposes of VMA manipulation would also > > be a departure from existing practice. Can you help me understand why > > you seem to favor the FD-per-VMA approach so heavily? I don't see any > > arguments *for* an FD-per-VMA model for remove memory manipulation and > > I see a lot of arguments against it. Is there some compelling > > advantage I'm missing? > > First and foremost it provides an easy cookie to the userspace to > guarantee time-to-check-time-to-use consistency. But only for one VMA at a time. > It also naturally > extend an existing fadvise interface that achieves madvise semantic on > files. There are lots of things that madvise can do that fadvise can't and that don't even really make sense for fadvise, e.g., MADV_FREE. It seems odd to me to duplicate much of the madvise interface into fadvise so that we can use file APIs to give madvise hints. It seems simpler to me to just provide a mechanism to put the madvise hints where they're needed. > I am not really pushing hard for this particular API but I really > do care about a programming model that would be sane. You've used "sane" twice so far in this message. Can you specify more precisely what you mean by that word? I agree that there needs to be some defense against TOCTOU races when doing remote memory management, but I don't think providing this robustness via a file descriptor is any more sane than alternative approaches. A file descriptor comes with a lot of other features --- e.g., SCM_RIGHTS, fstat, and a concept of owning a resource --- that aren't needed to achieve robustness. Normally, a file descriptor refers to some resource that the kernel holds as long as the file descriptor (well, the open file description or struct file) lives -- things like graphics buffers, files, and sockets. If we're using an FD *just* as a cookie and not a resource, I'd rather just expose the cookie directly. > If we have a > different means to achieve the same then all fine by me but so far I > haven't heard any sound arguments to invent something completely new > when we have established APIs to use. Doesn't the next sentence describe something profoundly new? :-) > Exporting anonymous mappings via > proc the same way we do for file mappings doesn't seem to be stepping > outside of the current practice way too much. It seems like a radical departure from existing practice to provide filesystem interfaces to anonymous memory regions, e.g., anon_vma. You've never been able to refer to those memory regions with file descriptors. All I'm suggesting is that we take the existing madvise mechanism, make it work cross-process, and make it robust against TOCTOU problems, all one step at a time. Maybe my sense of API "size" is miscalibrated, but adding a new type of FD to refer to anonymous VMA regions feels like a bigger departure and so requires stronger justification, especially if the result of the FD approach is probably something less efficient than a cookie-based one. > and we should focus on discussing whether this is a > sane model. And I think it would be much better to discuss that under > the respective patch which introduces that API rather than here. I think it's important to discuss what that API should look like. :-)
On Tue, May 28, 2019 at 4:56 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 28-05-19 04:42:47, Daniel Colascione wrote: > > On Tue, May 28, 2019 at 4:28 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > > > Michal, any opinion? > > > > > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > > > question that we have to sort out first is whether we want to have > > > > > > > the remote madvise call process or vma fd based. This is an important > > > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > > > so far unfortunately. > > > > > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > > > but thought it could be easily extended later for each address range > > > > > > operation as userspace getting smarter with more information. > > > > > > > > > > Never design user API based on a single usecase, please. The "easily > > > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > > > several times, the synchronization model has to be thought through > > > > > carefuly before a remote process address range operation can be > > > > > implemented. > > > > > > > > I agree with you that we shouldn't design API on single usecase but what > > > > you are concerning is actually not our usecase because we are resilient > > > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > > > Actually, many hints are already racy in that the upcoming pattern would > > > > be different with the behavior you thought at the moment. > > > > > > How come they are racy wrt address ranges? You would have to be in > > > multithreaded environment and then the onus of synchronization is on > > > threads. That model is quite clear. But we are talking about separate > > > processes and some of them might be even not aware of an external entity > > > tweaking their address space. > > > > I don't think the difference between a thread and a process matters in > > this context. Threads race on address space operations all the time > > --- in the sense that multiple threads modify a process's address > > space without synchronization. > > I would disagree. They do have in-kernel synchronization as long as they > do not use MAP_FIXED. If they do want to use MAP_FIXED then they better > synchronize or the result is undefined. Right. It's because the kernel hands off different regions to different non-MAP_FIXED mmap callers that it's pretty easy for threads to mind their own business, but they're all still using the same address space. > > From a synchronization point > > of view, it doesn't really matter whether it's a thread within the > > target process or a thread outside the target process that does the > > address space manipulation. What's new is the inspection of the > > address space before performing an operation. > > The fundamental difference is that if you want to achieve the same > inside the process then your application is inherenly aware of the > operation and use whatever synchronization is needed to achieve a > consistency. As soon as you allow the same from outside you either > have to have an aware target application as well or you need a mechanism > to find out that your decision has been invalidated by a later > unsynchronized action. I thought of this objection immediately after I hit send. :-) I still don't think the intra- vs inter-process difference matters. It's true that threads can synchronize with each other, but different processes can synchronize with each other too. I mean, you *could* use sem_open(3) for your heap lock and open the semaphore from two different processes. That's silly, but it'd work. The important requirement, I think, is that we need to support managing "memory-naive" uncooperative tasks (perhaps legacy ones written before cross-process memory management even became possible), and I think that the cooperative-vs-uncooperative distinction matters a lot more than the tgid of the thread doing the memory manipulation. (Although in our case, we really do need a separate tgid. :-))
On Tue, May 28, 2019 at 02:06:14PM +0200, Michal Hocko wrote: > On Tue 28-05-19 20:44:36, Minchan Kim wrote: > > On Tue, May 28, 2019 at 01:28:40PM +0200, Michal Hocko wrote: > > > On Tue 28-05-19 20:12:08, Minchan Kim wrote: > > > > On Tue, May 28, 2019 at 12:41:17PM +0200, Michal Hocko wrote: > > > > > On Tue 28-05-19 19:32:56, Minchan Kim wrote: > > > > > > On Tue, May 28, 2019 at 11:08:21AM +0200, Michal Hocko wrote: > > > > > > > On Tue 28-05-19 17:49:27, Minchan Kim wrote: > > > > > > > > On Tue, May 28, 2019 at 01:31:13AM -0700, Daniel Colascione wrote: > > > > > > > > > On Tue, May 28, 2019 at 1:14 AM Minchan Kim <minchan@kernel.org> wrote: > > > > > > > > > > if we went with the per vma fd approach then you would get this > > > > > > > > > > > feature automatically because map_files would refer to file backed > > > > > > > > > > > mappings while map_anon could refer only to anonymous mappings. > > > > > > > > > > > > > > > > > > > > The reason to add such filter option is to avoid the parsing overhead > > > > > > > > > > so map_anon wouldn't be helpful. > > > > > > > > > > > > > > > > > > Without chiming on whether the filter option is a good idea, I'd like > > > > > > > > > to suggest that providing an efficient binary interfaces for pulling > > > > > > > > > memory map information out of processes. Some single-system-call > > > > > > > > > method for retrieving a binary snapshot of a process's address space > > > > > > > > > complete with attributes (selectable, like statx?) for each VMA would > > > > > > > > > reduce complexity and increase performance in a variety of areas, > > > > > > > > > e.g., Android memory map debugging commands. > > > > > > > > > > > > > > > > I agree it's the best we can get *generally*. > > > > > > > > Michal, any opinion? > > > > > > > > > > > > > > I am not really sure this is directly related. I think the primary > > > > > > > question that we have to sort out first is whether we want to have > > > > > > > the remote madvise call process or vma fd based. This is an important > > > > > > > distinction wrt. usability. I have only seen pid vs. pidfd discussions > > > > > > > so far unfortunately. > > > > > > > > > > > > With current usecase, it's per-process API with distinguishable anon/file > > > > > > but thought it could be easily extended later for each address range > > > > > > operation as userspace getting smarter with more information. > > > > > > > > > > Never design user API based on a single usecase, please. The "easily > > > > > extended" part is by far not clear to me TBH. As I've already mentioned > > > > > several times, the synchronization model has to be thought through > > > > > carefuly before a remote process address range operation can be > > > > > implemented. > > > > > > > > I agree with you that we shouldn't design API on single usecase but what > > > > you are concerning is actually not our usecase because we are resilient > > > > with the race since MADV_COLD|PAGEOUT is not destruptive. > > > > Actually, many hints are already racy in that the upcoming pattern would > > > > be different with the behavior you thought at the moment. > > > > > > How come they are racy wrt address ranges? You would have to be in > > > multithreaded environment and then the onus of synchronization is on > > > threads. That model is quite clear. But we are talking about separate > > > > Think about MADV_FREE. Allocator would think the chunk is worth to mark > > "freeable" but soon, user of the allocator asked the chunk - ie, it's not > > freeable any longer once user start to use it. > > That is not a race in the address space, right. The underlying object > hasn't changed. It has been declared as freeable and since that moment > nobody can rely on the content because it might have been discarded. > Or put simply, the content is undefined. It is responsibility of the > madvise caller to make sure that the object is not in active use while > it is marking it. > > > My point is that kinds of *hints* are always racy so any synchronization > > couldn't help a lot. That's why I want to restrict hints process_madvise > > supports as such kinds of non-destruptive one at next respin. > > I agree that a non-destructive operations are safer against paralel > modifications because you just get a annoying and unexpected latency at > worst case. But we should discuss whether this assumption is sufficient > for further development. I am pretty sure once we open remote madvise > people will find usecases for destructive operations or even new madvise > modes we haven't heard of. What then? I support Daniel's vma seq number approach for the future plan.
On Tue 28-05-19 05:11:16, Daniel Colascione wrote: > On Tue, May 28, 2019 at 4:49 AM Michal Hocko <mhocko@kernel.org> wrote: [...] > > > We have various system calls that provide hints for open files, but > > > the memory operations are distinct. Modeling anonymous memory as a > > > kind of file-backed memory for purposes of VMA manipulation would also > > > be a departure from existing practice. Can you help me understand why > > > you seem to favor the FD-per-VMA approach so heavily? I don't see any > > > arguments *for* an FD-per-VMA model for remove memory manipulation and > > > I see a lot of arguments against it. Is there some compelling > > > advantage I'm missing? > > > > First and foremost it provides an easy cookie to the userspace to > > guarantee time-to-check-time-to-use consistency. > > But only for one VMA at a time. Which is the unit we operate on, right? > > It also naturally > > extend an existing fadvise interface that achieves madvise semantic on > > files. > > There are lots of things that madvise can do that fadvise can't and > that don't even really make sense for fadvise, e.g., MADV_FREE. It > seems odd to me to duplicate much of the madvise interface into > fadvise so that we can use file APIs to give madvise hints. It seems > simpler to me to just provide a mechanism to put the madvise hints > where they're needed. I do not see why we would duplicate. I confess I haven't tried to implement this so I might be overlooking something but it seems to me that we could simply reuse the same functionality from both APIs. > > I am not really pushing hard for this particular API but I really > > do care about a programming model that would be sane. > > You've used "sane" twice so far in this message. Can you specify more > precisely what you mean by that word? Well, I would consider a model which would prevent from unintended side effects (e.g. working on a completely different object) without a tricky synchronization sane. > I agree that there needs to be > some defense against TOCTOU races when doing remote memory management, > but I don't think providing this robustness via a file descriptor is > any more sane than alternative approaches. A file descriptor comes > with a lot of other features --- e.g., SCM_RIGHTS, fstat, and a > concept of owning a resource --- that aren't needed to achieve > robustness. > > Normally, a file descriptor refers to some resource that the kernel > holds as long as the file descriptor (well, the open file description > or struct file) lives -- things like graphics buffers, files, and > sockets. If we're using an FD *just* as a cookie and not a resource, > I'd rather just expose the cookie directly. You are absolutely right. But doesn't that apply to any other revalidation method that would be tracking VMA status as well. As I've said I am not married to this approach as long as there are better alternatives. So far we are in a discussion what should be the actual semantic of the operation and how much do we want to tolerate races. And it seems that we are diving into implementation details rather than landing with a firm decision that the current proposed API is suitable or not. > > If we have a > > different means to achieve the same then all fine by me but so far I > > haven't heard any sound arguments to invent something completely new > > when we have established APIs to use. > > Doesn't the next sentence describe something profoundly new? :-) > > > Exporting anonymous mappings via > > proc the same way we do for file mappings doesn't seem to be stepping > > outside of the current practice way too much. > > It seems like a radical departure from existing practice to provide > filesystem interfaces to anonymous memory regions, e.g., anon_vma. > You've never been able to refer to those memory regions with file > descriptors. > > All I'm suggesting is that we take the existing madvise mechanism, > make it work cross-process, and make it robust against TOCTOU > problems, all one step at a time. Maybe my sense of API "size" is > miscalibrated, but adding a new type of FD to refer to anonymous VMA > regions feels like a bigger departure and so requires stronger > justification, especially if the result of the FD approach is probably > something less efficient than a cookie-based one. Feel free to propose the way to achieve that in the respective email thread. > > and we should focus on discussing whether this is a > > sane model. And I think it would be much better to discuss that under > > the respective patch which introduces that API rather than here. > > I think it's important to discuss what that API should look like. :-) It will be fun to follow this discussion and make some sense of different parallel threads.
On Tue 28-05-19 05:18:48, Daniel Colascione wrote: [...] > The important requirement, I think, is that we need to support > managing "memory-naive" uncooperative tasks (perhaps legacy ones > written before cross-process memory management even became possible), > and I think that the cooperative-vs-uncooperative distinction matters > a lot more than the tgid of the thread doing the memory manipulation. > (Although in our case, we really do need a separate tgid. :-)) Agreed here and that requires some sort of revalidation and failure on "object has changed" in one form or another IMHO.
On Wed, May 29, 2019 at 12:36:04PM +0800, Hillf Danton wrote: > > On Mon, 20 May 2019 12:52:54 +0900 Minchan Kim wrote: > > > > With that, user could call a process_madvise syscall simply with a entire > > range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and > > MADV_FILE_FILTER so there is no need to call the syscall range by range. > > > Cool. > > Look forward to seeing the non-RFC delivery. I will drop this filter patch if userspace can parse address range fast. Daniel suggested a new interface which could get necessary information from the process with binary format so will it will remove endoce/decode overhead as well as overhead we don't need to get like directory look up. Yes, with this filter option, it's best performance for a specific usecase but sometime we need to give up *best* thing for *general* stuff. I belive it's one of the category. ;-)
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index b8e230de84a6..be59a1b90284 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -66,6 +66,11 @@ #define MADV_WIPEONFORK 18 /* Zero memory on fork, child only */ #define MADV_KEEPONFORK 19 /* Undo MADV_WIPEONFORK */ +#define MADV_BEHAVIOR_MASK (~(MADV_ANONYMOUS_FILTER|MADV_FILE_FILTER)) + +#define MADV_ANONYMOUS_FILTER (1<<31) /* works for only anonymous vma */ +#define MADV_FILE_FILTER (1<<30) /* works for only file-backed vma */ + /* compatibility flags */ #define MAP_FILE 0 diff --git a/mm/madvise.c b/mm/madvise.c index f4f569dac2bd..116131243540 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -1002,7 +1002,15 @@ static int madvise_core(struct task_struct *tsk, unsigned long start, int write; size_t len; struct blk_plug plug; + bool anon_only, file_only; + anon_only = behavior & MADV_ANONYMOUS_FILTER; + file_only = behavior & MADV_FILE_FILTER; + + if (anon_only && file_only) + return error; + + behavior = behavior & MADV_BEHAVIOR_MASK; if (!madvise_behavior_valid(behavior)) return error; @@ -1067,12 +1075,18 @@ static int madvise_core(struct task_struct *tsk, unsigned long start, if (end < tmp) tmp = end; + if (anon_only && vma->vm_file) + goto next; + if (file_only && !vma->vm_file) + goto next; + /* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */ error = madvise_vma(tsk, vma, &prev, start, tmp, behavior, &pages); if (error) goto out; *nr_pages += pages; +next: start = tmp; if (prev && start < prev->vm_end) start = prev->vm_end;
System could have much faster swap device like zRAM. In that case, swapping is extremely cheaper than file-IO on the low-end storage. In this configuration, userspace could handle different strategy for each kinds of vma. IOW, they want to reclaim anonymous pages by MADV_COLD while it keeps file-backed pages in inactive LRU by MADV_COOL because file IO is more expensive in this case so want to keep them in memory until memory pressure happens. To support such strategy easier, this patch introduces MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER options in madvise(2) like that /proc/<pid>/clear_refs already has supported same filters. They are filters could be Ored with other existing hints using top two bits of (int behavior). Once either of them is set, the hint could affect only the interested vma either anonymous or file-backed. With that, user could call a process_madvise syscall simply with a entire range(0x0 - 0xFFFFFFFFFFFFFFFF) but either of MADV_ANONYMOUS_FILTER and MADV_FILE_FILTER so there is no need to call the syscall range by range. * from v1r2 * use consistent check with clear_refs to identify anon/file vma - surenb * from v1r1 * use naming "filter" for new madvise option - dancol Signed-off-by: Minchan Kim <minchan@kernel.org> --- include/uapi/asm-generic/mman-common.h | 5 +++++ mm/madvise.c | 14 ++++++++++++++ 2 files changed, 19 insertions(+)