mm: rearrange madvise code to allow for reuse

From: Colin Cross <[email protected]>

In many userspace applications, and especially in VM based applications like Android uses heavily, there are multiple different allocators in use.
At a minimum there is libc malloc and the stack, and in many cases there
are libc malloc, the stack, direct syscalls to mmap anonymous memory, and multiple VM heaps (one for small objects, one for big objects, etc.). Each of these layers usually has its own tools to inspect its usage; malloc by compiling a debug version, the VM through heap inspection tools, and for direct syscalls there is usually no way to track them.

On Android we heavily use a set of tools that use an extended version of the logic covered in Documentation/vm/pagemap.txt to walk all pages mapped in userspace and slice their usage by process, shared (COW) vs. unique mappings, backing, etc. This can account for real physical memory usage even in cases like fork without exec (which Android uses heavily to share as many private COW pages as possible between processes), Kernel SamePage Merging, and clean zero pages. It produces a measurement of the pages that only exist in that process (USS, for unique), and a measurement of the physical memory usage of that process with the cost of shared pages being evenly split between processes that share them (PSS).

If all anonymous memory is indistinguishable then figuring out the real physical memory usage (PSS) of each heap requires either a pagemap walking tool that can understand the heap debugging of every layer, or for every layer's heap debugging tools to implement the pagemap walking logic, in which case it is hard to get a consistent view of memory across the whole system.

Tracking the information in userspace leads to all sorts of problems. It either needs to be stored inside the process, which means every process has to have an API to export its current heap information upon request, or it has to be stored externally in a filesystem that somebody needs to clean up on crashes. It needs to be readable while the process is still running, so it has to have some sort of synchronization with every layer of userspace. Efficiently tracking the ranges requires reimplementing something like the kernel vma trees, and linking to it from every layer of userspace. It requires more memory, more syscalls, more runtime cost, and more complexity to separately track regions that the kernel is already tracking.

This patch adds a field to /proc/pid/maps and /proc/pid/smaps to show a userspace-provided name for anonymous vmas. The names of named anonymous vmas are shown in /proc/pid/maps and /proc/pid/smaps as [anon:<name>].

Userspace can set the name for a region of memory by calling prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, start, len, (unsigned long)name);

Setting the name to NULL clears it. The name length limit is 80 bytes including NUL-terminator and is checked to contain only printable ascii characters (including space), except '[',']','\','$' and '`'.

The name is stored in a pointer in the shared union in vm_area_struct that points to a null terminated string. Anonymous vmas with the same name (equivalent strings) and are otherwise mergeable will be merged.

The name pointers are not shared between vmas even if they contain the same name. The name pointer is stored in a union with fields that are only used on file-backed mappings, so it does not increase memory usage.

The patch is based on the original patch developed by Colin Cross, more specifically on its latest version [1] posted upstream by Sumit Semwal. It used a userspace pointer to store vma names. In that design, name pointers could be shared between vmas. However during the last upstreaming attempt, Kees Cook raised concerns [2] about this approach and suggested to copy the name into kernel memory space, perform validity checks [3] and store as a string referenced from vm_area_struct.
One big concern is about fork() performance which would need to strdup anonymous vma names. Dave Hansen suggested experimenting with worst-case scenario of forking a process with 64k vmas having longest possible names [4]. I ran this experiment on an ARM64 Android device and recorded a worst-case regression of almost 40% when forking such a process. This regression is addressed in the followup patch which replaces the pointer to a name with a refcounted structure that allows sharing the name pointer between vmas of the same name. Instead of duplicating the string during fork() or when splitting a vma it increments the refcount.

Quoted 45 lines · On Fri, 1 Oct 2021 13:56:56 -0700 Suren Baghdasaryan <[email protected]> wrote:

From: Colin Cross <[email protected]>

In many userspace applications, and especially in VM based applications like Android uses heavily, there are multiple different allocators in use.
At a minimum there is libc malloc and the stack, and in many cases there
are libc malloc, the stack, direct syscalls to mmap anonymous memory, and multiple VM heaps (one for small objects, one for big objects, etc.). Each of these layers usually has its own tools to inspect its usage; malloc by compiling a debug version, the VM through heap inspection tools, and for direct syscalls there is usually no way to track them.

On Android we heavily use a set of tools that use an extended version of the logic covered in Documentation/vm/pagemap.txt to walk all pages mapped in userspace and slice their usage by process, shared (COW) vs. unique mappings, backing, etc. This can account for real physical memory usage even in cases like fork without exec (which Android uses heavily to share as many private COW pages as possible between processes), Kernel SamePage Merging, and clean zero pages. It produces a measurement of the pages that only exist in that process (USS, for unique), and a measurement of the physical memory usage of that process with the cost of shared pages being evenly split between processes that share them (PSS).

If all anonymous memory is indistinguishable then figuring out the real physical memory usage (PSS) of each heap requires either a pagemap walking tool that can understand the heap debugging of every layer, or for every layer's heap debugging tools to implement the pagemap walking logic, in which case it is hard to get a consistent view of memory across the whole system.

Tracking the information in userspace leads to all sorts of problems. It either needs to be stored inside the process, which means every process has to have an API to export its current heap information upon request, or it has to be stored externally in a filesystem that somebody needs to clean up on crashes. It needs to be readable while the process is still running, so it has to have some sort of synchronization with every layer of userspace. Efficiently tracking the ranges requires reimplementing something like the kernel vma trees, and linking to it from every layer of userspace. It requires more memory, more syscalls, more runtime cost, and more complexity to separately track regions that the kernel is already tracking.

This patch adds a field to /proc/pid/maps and /proc/pid/smaps to show a userspace-provided name for anonymous vmas. The names of named anonymous vmas are shown in /proc/pid/maps and /proc/pid/smaps as [anon:<name>].

Quoted 2 lines

Userspace can set the name for a region of memory by calling prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, start, len, (unsigned long)name);

So this can cause a vma to be split, if [start,len] doesn't exactly describe an existing vma? If so, is this at all useful? If not then `len' isn't needed - just pass in some address within an existing vma?

Quoted 7 lines

Setting the name to NULL clears it. The name length limit is 80 bytes including NUL-terminator and is checked to contain only printable ascii characters (including space), except '[',']','\','$' and '`'.

The name is stored in a pointer in the shared union in vm_area_struct that points to a null terminated string. Anonymous vmas with the same name (equivalent strings) and are otherwise mergeable will be merged.

So this can prevent vma merging if used incorrectly (or maliciously - can't think how)? What are the potential impacts of this?

Quoted 4 lines

The name pointers are not shared between vmas even if they contain the same name. The name pointer is stored in a union with fields that are only used on file-backed mappings, so it does not increase memory usage.

Quoted 16 lines

The patch is based on the original patch developed by Colin Cross, more specifically on its latest version [1] posted upstream by Sumit Semwal. It used a userspace pointer to store vma names. In that design, name pointers could be shared between vmas. However during the last upstreaming attempt, Kees Cook raised concerns [2] about this approach and suggested to copy the name into kernel memory space, perform validity checks [3] and store as a string referenced from vm_area_struct.
One big concern is about fork() performance which would need to strdup anonymous vma names. Dave Hansen suggested experimenting with worst-case scenario of forking a process with 64k vmas having longest possible names [4]. I ran this experiment on an ARM64 Android device and recorded a worst-case regression of almost 40% when forking such a process. This regression is addressed in the followup patch which replaces the pointer to a name with a refcounted structure that allows sharing the name pointer between vmas of the same name. Instead of duplicating the string during fork() or when splitting a vma it increments the refcount.

Generally, the patch adds a bunch of code which a lot of users won't want. Did we bust a gut to reduce this impact? Was a standalone config setting considered?

And what would be the impact of making this feature optional? Is a proliferation of formats in /proc/pid/maps going to make userspace parsers harder to develop and test?

I do think that saying "The names of named anonymous vmas are shown in /proc/pid/maps and /proc/pid/smaps as [anon:<name>]." is a bit thin. Please provide sample output so we can consider these things better.

What are the risks that existing parsers will be broken by such changes?

Quoted 48 lines · Andrew Morton

Quoted 45 lines · On Fri, 1 Oct 2021 13:56:56 -0700 Suren Baghdasaryan <[email protected]> wrote:

From: Colin Cross <[email protected]>

In many userspace applications, and especially in VM based applications like Android uses heavily, there are multiple different allocators in use.
At a minimum there is libc malloc and the stack, and in many cases there
are libc malloc, the stack, direct syscalls to mmap anonymous memory, and multiple VM heaps (one for small objects, one for big objects, etc.). Each of these layers usually has its own tools to inspect its usage; malloc by compiling a debug version, the VM through heap inspection tools, and for direct syscalls there is usually no way to track them.

On Android we heavily use a set of tools that use an extended version of the logic covered in Documentation/vm/pagemap.txt to walk all pages mapped in userspace and slice their usage by process, shared (COW) vs. unique mappings, backing, etc. This can account for real physical memory usage even in cases like fork without exec (which Android uses heavily to share as many private COW pages as possible between processes), Kernel SamePage Merging, and clean zero pages. It produces a measurement of the pages that only exist in that process (USS, for unique), and a measurement of the physical memory usage of that process with the cost of shared pages being evenly split between processes that share them (PSS).

If all anonymous memory is indistinguishable then figuring out the real physical memory usage (PSS) of each heap requires either a pagemap walking tool that can understand the heap debugging of every layer, or for every layer's heap debugging tools to implement the pagemap walking logic, in which case it is hard to get a consistent view of memory across the whole system.

Tracking the information in userspace leads to all sorts of problems. It either needs to be stored inside the process, which means every process has to have an API to export its current heap information upon request, or it has to be stored externally in a filesystem that somebody needs to clean up on crashes. It needs to be readable while the process is still running, so it has to have some sort of synchronization with every layer of userspace. Efficiently tracking the ranges requires reimplementing something like the kernel vma trees, and linking to it from every layer of userspace. It requires more memory, more syscalls, more runtime cost, and more complexity to separately track regions that the kernel is already tracking.

This patch adds a field to /proc/pid/maps and /proc/pid/smaps to show a userspace-provided name for anonymous vmas. The names of named anonymous vmas are shown in /proc/pid/maps and /proc/pid/smaps as [anon:<name>].

Quoted 6 lines

Quoted 2 lines

Userspace can set the name for a region of memory by calling prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, start, len, (unsigned long)name);

So this can cause a vma to be split, if [start,len] doesn't exactly describe an existing vma? If so, is this at all useful? If not then `len' isn't needed - just pass in some address within an existing vma?

Technically one could mmap a large chunk of memory and then assign different names to its parts to use for different purposes, which would cause the vma to split. I don't think Android uses it that way but I'll have to double-check. I think one advantage of doing this could be to minimize the number of mmap syscalls.

Quoted 10 lines

Quoted 7 lines

Setting the name to NULL clears it. The name length limit is 80 bytes including NUL-terminator and is checked to contain only printable ascii characters (including space), except '[',']','\','$' and '`'.

The name is stored in a pointer in the shared union in vm_area_struct that points to a null terminated string. Anonymous vmas with the same name (equivalent strings) and are otherwise mergeable will be merged.

So this can prevent vma merging if used incorrectly (or maliciously - can't think how)? What are the potential impacts of this?

Potential impact would be that otherwise mergeable vmas would not be merged due to the name difference. This is a known downside of naming an anon vma which I documented in my manual pages description as "Note that assigning an attribute to a virtual memory area might prevent it from being merged with adjacent virtual memory areas due to the difference in that attribute's value.". In Android we see an increase in the number of VMAs due to this feature but it was not significant. I'll try to dig up the numbers or will rerun the test to get them. If a process maliciously wants to increase the number of vmas in the system it could generate lots of vmas with different names in its address space, however this can be done even without this feature by mapping vmas while toggling a protection bit. Something like this:

int prot = PROT_WRITE;

for (i = 0; i < count; i++) {
    mmap(NULL, size_bytes, prot, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
    prot = (prot ^ PROT_READ) & (PROT_READ | PROT_WRITE);
}

Quoted 21 lines

Quoted 4 lines

The name pointers are not shared between vmas even if they contain the same name. The name pointer is stored in a union with fields that are only used on file-backed mappings, so it does not increase memory usage.

Quoted 16 lines

The patch is based on the original patch developed by Colin Cross, more specifically on its latest version [1] posted upstream by Sumit Semwal. It used a userspace pointer to store vma names. In that design, name pointers could be shared between vmas. However during the last upstreaming attempt, Kees Cook raised concerns [2] about this approach and suggested to copy the name into kernel memory space, perform validity checks [3] and store as a string referenced from vm_area_struct.
One big concern is about fork() performance which would need to strdup anonymous vma names. Dave Hansen suggested experimenting with worst-case scenario of forking a process with 64k vmas having longest possible names [4]. I ran this experiment on an ARM64 Android device and recorded a worst-case regression of almost 40% when forking such a process. This regression is addressed in the followup patch which replaces the pointer to a name with a refcounted structure that allows sharing the name pointer between vmas of the same name. Instead of duplicating the string during fork() or when splitting a vma it increments the refcount.

Quoted 3 lines

Generally, the patch adds a bunch of code which a lot of users won't want. Did we bust a gut to reduce this impact? Was a standalone config setting considered?

I didn't consider a standalone config for this feature because when not used it has no memory impact at runtime. As for the image size, I built Linus' ToT with and without this patchset with allmodconfig and the sizes are:
Without the patchset:

$ size vmlinux
   text    data     bss     dec     hex filename
40763556 58424519 29016228 128204303 7a43e0f vmlinux

With the patchset:

$ size vmlinux
   text    data     bss     dec     hex filename
40765068 58424671 29016228 128205967 7a4448f vmlinux

The increase seems quite small, so I'm not sure if it warrants a separate config option.

Quoted 4 lines

And what would be the impact of making this feature optional? Is a proliferation of formats in /proc/pid/maps going to make userspace parsers harder to develop and test?

I'm guessing having one format is simpler and therefore preferable?

Quoted 3 lines

I do think that saying "The names of named anonymous vmas are shown in /proc/pid/maps and /proc/pid/smaps as [anon:<name>]." is a bit thin. Please provide sample output so we can consider these things better.

Sure. Here is a sample /proc/$pid/maps output (partial):

6ffacb6000-6ffacd6000 r--s 00000000 00:10 361
/dev/__properties__/properties_serial
6ffacd6000-6ffacd9000 rw-p 00000000 00:00 0
[anon:System property context nodes]
6ffacd9000-6ffaceb000 r--s 00000000 00:10 79
/dev/__properties__/property_info
6ffaceb000-6ffad4f000 r--p 00000000 00:00 0
[anon:linker_alloc]
6ffad4f000-6ffad51000 rw-p 00000000 00:00 0
[anon:bionic_alloc_small_objects]
6ffad51000-6ffad52000 r--p 00000000 00:00 0
[anon:atexit handlers]
6ffad52000-6ffbc2c000 ---p 00000000 00:00 0 6ffbc2c000-6ffbc2e000 rw-p 00000000 00:00 0 6ffbc2e000-6ffbd52000 ---p 00000000 00:00 0 6ffbd52000-6ffbd53000 ---p 00000000 00:00 0 6ffbd53000-6ffbd5b000 rw-p 00000000 00:00 0
[anon:thread signal stack]
6ffbd5b000-6ffbd5c000 rw-p 00000000 00:00 0
[anon:arc4random data]
6ffbd5c000-6ffbd5d000 r--p 0000d000 07:90 59
/apex/com.android.art/javalib/arm64/boot-okhttp.art
6ffbd5d000-6ffbd5e000 r--p 00014000 07:90 56
/apex/com.android.art/javalib/arm64/boot-core-libart.art
6ffbd5e000-6ffbd5f000 rw-p 00000000 00:00 0
[anon:arc4random data]
6ffbd5f000-6ffbd61000 r--p 00000000 00:00 0 [vvar]
6ffbd61000-6ffbd62000 r-xp 00000000 00:00 0 [vdso]

and sample /proc/$pid/smaps output (partial):

6ffad4f000-6ffad51000 rw-p 00000000 00:00 0
[anon:bionic_alloc_small_objects]

Size:                  8 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:                   0 kB
Pss:                   0 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:         0 kB
Referenced:            0 kB
Anonymous:             0 kB
LazyFree:              0 kB
AnonHugePages:         0 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:         0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  8 kB
SwapPss:               8 kB
Locked:                0 kB
THPeligible:    0
VmFlags: rd wr mr mw me ac

Quoted 2 lines

What are the risks that existing parsers will be broken by such changes?

That I can't really answer. It would depend on how the parser is written. The implementation follows the same pattern as [stack], [vdso] and other non-filebacked sections are named, however if a parser is written so that it does not ignore an unknown entry then it would fail to parse [anon:...] name if some process decides to name its anonymous vmas.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

The name pointers are not shared between vmas even if they contain the same name. The name pointer is stored in a union with fields that are only used on file-backed mappings, so it does not increase memory usage.

The patch is based on the original patch developed by Colin Cross, more specifically on its latest version [1] posted upstream by Sumit Semwal. It used a userspace pointer to store vma names. In that design, name pointers could be shared between vmas. However during the last upstreaming attempt, Kees Cook raised concerns [2] about this approach and suggested to copy the name into kernel memory space, perform validity checks [3] and store as a string referenced from vm_area_struct.
One big concern is about fork() performance which would need to strdup anonymous vma names. Dave Hansen suggested experimenting with worst-case scenario of forking a process with 64k vmas having longest possible names [4]. I ran this experiment on an ARM64 Android device and recorded a worst-case regression of almost 40% when forking such a process. This regression is addressed in the followup patch which replaces the pointer to a name with a refcounted structure that allows sharing the name pointer between vmas of the same name. Instead of duplicating the string during fork() or when splitting a vma it increments the refcount.

Quoted 7 lines

Quoted 3 lines

Generally, the patch adds a bunch of code which a lot of users won't want. Did we bust a gut to reduce this impact? Was a standalone config setting considered?

I didn't consider a standalone config for this feature because when not used it has no memory impact at runtime. As for the image size, I built Linus' ToT with and without this patchset with allmodconfig and

Quoted 11 lines

the sizes are:
Without the patchset:

$ size vmlinux
   text    data     bss     dec     hex filename
40763556 58424519 29016228 128204303 7a43e0f vmlinux

With the patchset:

$ size vmlinux
   text    data     bss     dec     hex filename
40765068 58424671 29016228 128205967 7a4448f vmlinux

Quoted 2 lines

The increase seems quite small, so I'm not sure if it warrants a separate config option.

Andrew, do you still think we need a separate CONFIG option? I fixed the build issue when CONFIG_ADVISE_SYSCALLS=n and would like to post the update but if you want to have a separate config then I can post that together with the fix. Please let me know.

Quoted 25 lines · On Mon, 4 Oct 2021 09:21:42 -0700 Suren Baghdasaryan <[email protected]> wrote:

Quoted 25 lines

Quoted 24 lines

Quoted 4 lines

The name pointers are not shared between vmas even if they contain the same name. The name pointer is stored in a union with fields that are only used on file-backed mappings, so it does not increase memory usage.

Quoted 16 lines

The patch is based on the original patch developed by Colin Cross, more specifically on its latest version [1] posted upstream by Sumit Semwal. It used a userspace pointer to store vma names. In that design, name pointers could be shared between vmas. However during the last upstreaming attempt, Kees Cook raised concerns [2] about this approach and suggested to copy the name into kernel memory space, perform validity checks [3] and store as a string referenced from vm_area_struct.
One big concern is about fork() performance which would need to strdup anonymous vma names. Dave Hansen suggested experimenting with worst-case scenario of forking a process with 64k vmas having longest possible names [4]. I ran this experiment on an ARM64 Android device and recorded a worst-case regression of almost 40% when forking such a process. This regression is addressed in the followup patch which replaces the pointer to a name with a refcounted structure that allows sharing the name pointer between vmas of the same name. Instead of duplicating the string during fork() or when splitting a vma it increments the refcount.

Generally, the patch adds a bunch of code which a lot of users won't want. Did we bust a gut to reduce this impact? Was a standalone config setting considered?

Quoted 3 lines

Quoted 3 lines

I didn't consider a standalone config for this feature because when not used it has no memory impact at runtime. As for the image size, I built Linus' ToT with and without this patchset with allmodconfig and

allnoconfig would be more interesting. People who want small kernels won't be using allmodconfig!

Quoted 15 lines

Quoted 14 lines

the sizes are:
Without the patchset:

$ size vmlinux
   text    data     bss     dec     hex filename
40763556 58424519 29016228 128204303 7a43e0f vmlinux

With the patchset:

$ size vmlinux
   text    data     bss     dec     hex filename
40765068 58424671 29016228 128205967 7a4448f vmlinux

The increase seems quite small, so I'm not sure if it warrants a separate config option.

Quoted 4 lines

Andrew, do you still think we need a separate CONFIG option? I fixed the build issue when CONFIG_ADVISE_SYSCALLS=n and would like to post the update but if you want to have a separate config then I can post that together with the fix. Please let me know.

I don't see much downside to the standalone option. More complexity for developers/testers, I guess. But such is life?

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Hi Suren,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.15-rc3 next-20210922]
[cannot apply to hnaz-mm/master kees/for-next/pstore]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Suren-Baghdasaryan/mm-rearrange-madvise-code-to-allow-for-reuse/20211002-045737
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 02d5e016800d082058b3d3b7c3ede136cdc6ddcb
config: i386-tinyconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/a0b92f268403ba8840f5cbe366ab77f9383860ae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Suren-Baghdasaryan/mm-rearrange-madvise-code-to-allow-for-reuse/20211002-045737 git checkout a0b92f268403ba8840f5cbe366ab77f9383860ae
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: kernel/fork.o: in function `vm_area_dup':
fork.c:(.text+0x6e5): undefined reference to `dup_vma_anon_name'
ld: kernel/fork.o: in function `vm_area_free':
fork.c:(.text+0x707): undefined reference to `free_vma_anon_name'
ld: kernel/fork.o: in function `dup_mmap':
fork.c:(.text+0xefb): undefined reference to `free_vma_anon_name'
ld: mm/mlock.o: in function `mlock_fixup':

Quoted 1 line

Quoted 1 line

mlock.c:(.text+0x806): undefined reference to `vma_anon_name'

ld: mm/mmap.o: in function `can_vma_merge_before':

Quoted 1 line

Quoted 1 line

mmap.c:(.text+0x394): undefined reference to `vma_anon_name'

ld: mm/mmap.o: in function `vma_merge':
mmap.c:(.text+0x1341): undefined reference to `vma_anon_name'
ld: mm/mmap.o: in function `copy_vma':
mmap.c:(.text+0x2911): undefined reference to `vma_anon_name'
ld: mm/mprotect.o: in function `mprotect_fixup':

Quoted 1 line

Quoted 1 line

mprotect.c:(.text+0x4a3): undefined reference to `vma_anon_name'

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

While forking a process with high number (64K) of named anonymous vmas the overhead caused by strdup() is noticeable. Experiments with ARM64

Quoted 2 lines · Suren Baghdasaryan

While forking a process with high number (64K) of named anonymous vmas the overhead caused by strdup() is noticeable. Experiments with ARM64

Android

I still believe you should simply use numbers and do the numbers->strings mapping in userspace. We should not need to optimize strdups in kernel...

Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 9 lines · Pavel Machek

Quoted 2 lines · Suren Baghdasaryan

While forking a process with high number (64K) of named anonymous vmas the overhead caused by strdup() is noticeable. Experiments with ARM64

Android

I still believe you should simply use numbers and do the numbers->strings mapping in userspace. We should not need to optimize strdups in kernel...

Here are complications with mapping numbers to strings in the userspace:
Approach 1: hardcode number->string in some header file and let all tools use that mapping. The issue is that whenever that mapping changes all the tools that are using it (including 3rd party ones) have to be rebuilt. This is not really maintainable since we don't control 3rd party tools and even for the ones we control, it will be a maintenance issue figuring out which version of the tool used which header file.
Approach 2: have a centralized facility (a process or a DB) maintaining number->string mapping. This would require an additional request to this facility whenever we want to make a number->string conversion. Moreover, when we want to name a VMA, we would have to register a new VMA name in that facility or check that one already exists and get its ID. So each prctl() call to name a VMA will be preceded by such a request (IPC call), maybe with some optimizations to cache already known number->string pairs. This would be quite expensive performance-wise. Additional issue with this approach is that this mapping will have to be persistent to handle a case when the facility crashes and has to be restored.

As I said before, it complicates userspace quite a bit. Is that a good enough reason to store the names in the kernel and pay a little more memory for that? IMHO yes, but I might be wrong.

Quoted 8 lines · Suren Baghdasaryan

Quoted 2 lines

While forking a process with high number (64K) of named anonymous vmas the overhead caused by strdup() is noticeable. Experiments with ARM64

Android

I still believe you should simply use numbers and do the numbers->strings mapping in userspace. We should not need to optimize strdups in kernel...

Here are complications with mapping numbers to strings in the userspace:
Approach 1: hardcode number->string in some header file and let all tools use that mapping. The issue is that whenever that mapping changes all the tools that are using it (including 3rd party ones) have to be rebuilt. This is not really maintainable since we don't control 3rd party tools and even for the ones we control, it will be a maintenance issue figuring out which version of the tool used which header file.

Approach 2: have a centralized facility (a process or a DB) maintaining number->string mapping. This would require an additional request to this facility whenever we want to make a number->string conversion. Moreover, when we want to name a VMA, we would have to

Hi!

Quoted 9 lines

Quoted 8 lines · Suren Baghdasaryan

Quoted 2 lines

While forking a process with high number (64K) of named anonymous vmas the overhead caused by strdup() is noticeable. Experiments with ARM64

Android

I still believe you should simply use numbers and do the numbers->strings mapping in userspace. We should not need to optimize strdups in kernel...

Quoted 8 lines

Here are complications with mapping numbers to strings in the userspace:
Approach 1: hardcode number->string in some header file and let all tools use that mapping. The issue is that whenever that mapping changes all the tools that are using it (including 3rd party ones) have to be rebuilt. This is not really maintainable since we don't control 3rd party tools and even for the ones we control, it will be a maintenance issue figuring out which version of the tool used which header file.

1a) Just put it into a file in /etc... Similar to header file but easier...

Quoted 4 lines

Approach 2: have a centralized facility (a process or a DB) maintaining number->string mapping. This would require an additional request to this facility whenever we want to make a number->string conversion. Moreover, when we want to name a VMA, we would have to

I see it complicates userspace. But that's better than complicating kernel, and I don't know what limits on strings you plan, but considering you'll be outputing the strings in /proc... someone is going to get confused with parsing.

Pavel
--
http://www.livejournal.com/~pavelmachek

Quoted 29 lines · Pavel Machek

Hi!

Quoted 9 lines

Quoted 8 lines · Suren Baghdasaryan

Quoted 2 lines

While forking a process with high number (64K) of named anonymous vmas the overhead caused by strdup() is noticeable. Experiments with ARM64

Android

I still believe you should simply use numbers and do the numbers->strings mapping in userspace. We should not need to optimize strdups in kernel...

Quoted 8 lines

Here are complications with mapping numbers to strings in the userspace:
Approach 1: hardcode number->string in some header file and let all tools use that mapping. The issue is that whenever that mapping changes all the tools that are using it (including 3rd party ones) have to be rebuilt. This is not really maintainable since we don't control 3rd party tools and even for the ones we control, it will be a maintenance issue figuring out which version of the tool used which header file.

1a) Just put it into a file in /etc... Similar to header file but easier...

Quoted 4 lines

Approach 2: have a centralized facility (a process or a DB) maintaining number->string mapping. This would require an additional request to this facility whenever we want to make a number->string conversion. Moreover, when we want to name a VMA, we would have to

Quoted 4 lines

I see it complicates userspace. But that's better than complicating kernel, and I don't know what limits on strings you plan, but considering you'll be outputing the strings in /proc... someone is going to get confused with parsing.

I'm not a fan of complicating kernel but the proposed approach seems simple enough to me. Again this is subjective, so I can't really have a good argument here. Maybe, as Andrew suggested, I should keep it under a separate config so that whoever does not care about this feature pays no price for it?

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

The kernel changes at a different rate than distros and user space, and keeping number->string mappings updated and correct is just basically hopeless.

And you've beaten down the perf problems with kref, so it's fine.

2) At the same time, this feature is Just Not Needed! ...usually. So the config option seems absolutely appropriate.

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

--
Thanks,

David / dhildenb

Quoted 30 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

Quoted 1 line

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

Quoted 6 lines

Quoted 1 line

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Valuable information

Quoted 4 lines

Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

So, you'd actually want to generate/reserve an id for a given string at runtime, assign that id to the VMA, and have a way to match id <-> string somehow?

That reservation service could be inside the kernel or even (better?) in user space. The service could for example de-duplicates strings.

My question would be, if we really have to expose these strings to the kernel, or if an id is sufficient. Sure, it would move complexity to user space, but keeping complexity out of the kernel is usually a good idea.

--
Thanks,

David / dhildenb

Quoted 35 lines · David Hildenbrand

Quoted 33 lines · Suren Baghdasaryan

Quoted 31 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

Quoted 6 lines

Quoted 4 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Valuable information

Yeah, I should have described it clearer the first time around.

Quoted 3 lines

My question would be, if we really have to expose these strings to the kernel, or if an id is sufficient. Sure, it would move complexity to user space, but keeping complexity out of the kernel is usually a good idea.

My worry here is not the additional complexity on the userspace side but the performance hit we would have to endure due to these conversions.

Quoted 41 lines · On Wed, 6 Oct 2021 08:20:20 -0700 Suren Baghdasaryan <[email protected]> wrote:

Quoted 40 lines · David Hildenbrand

Quoted 37 lines · Suren Baghdasaryan

Quoted 26 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

Quoted 5 lines

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Quoted 3 lines

Quoted 1 line

Valuable information

Yeah, I should have described it clearer the first time around.

If it gets this fancy then the 80 char limit is likely to become a significant limitation and the choice should be explained & justified.

Why not 97? 1034? Why not just strndup_user() and be done with it?

Quoted 8 lines

Quoted 4 lines

My question would be, if we really have to expose these strings to the kernel, or if an id is sufficient. Sure, it would move complexity to user space, but keeping complexity out of the kernel is usually a good idea.

My worry here is not the additional complexity on the userspace side but the performance hit we would have to endure due to these conversions.

Has the performance hit been quantified?

I've seen this many times down the ages. Something which *could* be done in userspace is instead done in the kernel because coordinating userspace is Just So Damn Hard. I guess the central problem is that userspace isn't centrally coordinated. I wish we were better at this.

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 12 lines

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Valuable information

Yeah, I should have described it clearer the first time around.

If it gets this fancy then the 80 char limit is likely to become a significant limitation and the choice should be explained & justified.

Quoted 1 line

Why not 97? 1034? Why not just strndup_user() and be done with it?

The original patch from 8 years ago used 256 as the limit but Rasmus argued that the string content should be human-readable, so 80 chars seems to be a reasonable limit (see:
https://lore.kernel.org/all/[email protected]),
which makes sense to me. We should be able to handle the 80 char limit by trimming it before calling prctl().

Quoted 14 lines · On Wed, 6 Oct 2021 19:46:57 -0700 Suren Baghdasaryan <[email protected]> wrote:

Quoted 13 lines

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Valuable information

Yeah, I should have described it clearer the first time around.

If it gets this fancy then the 80 char limit is likely to become a significant limitation and the choice should be explained & justified.

Why not 97? 1034? Why not just strndup_user() and be done with it?

Quoted 6 lines

The original patch from 8 years ago used 256 as the limit but Rasmus argued that the string content should be human-readable, so 80 chars seems to be a reasonable limit (see:
https://lore.kernel.org/all/[email protected]),
which makes sense to me. We should be able to handle the 80 char limit by trimming it before calling prctl().

What's the downside to making it unlimited?

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 24 lines · Andrew Morton

Quoted 20 lines · On Wed, 6 Oct 2021 19:46:57 -0700 Suren Baghdasaryan <[email protected]> wrote:

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Valuable information

Yeah, I should have described it clearer the first time around.

If it gets this fancy then the 80 char limit is likely to become a significant limitation and the choice should be explained & justified.

Why not 97? 1034? Why not just strndup_user() and be done with it?

The original patch from 8 years ago used 256 as the limit but Rasmus argued that the string content should be human-readable, so 80 chars seems to be a reasonable limit (see:
https://lore.kernel.org/all/[email protected]),
which makes sense to me. We should be able to handle the 80 char limit by trimming it before calling prctl().

Quoted 1 line

What's the downside to making it unlimited?

If we ignore the human-readability argument, I guess the possibility of abuse and increased memory consumption? I'm guessing parsing such a string is also easier if there is a known limit?

Quoted 33 lines · David Hildenbrand

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Valuable information

Yeah, I should have described it clearer the first time around.

Quoted 8 lines

Quoted 4 lines

Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

So, you'd actually want to generate/reserve an id for a given string at runtime, assign that id to the VMA, and have a way to match id <-> string somehow?

If we go with ids then yes, that is what we would have to do.

Quoted 2 lines

That reservation service could be inside the kernel or even (better?) in user space. The service could for example de-duplicates strings.

Yes but it would require an IPC call to that service potentially on every mmap() when we want to name a mapped vma. This would be prohibitive. Even on consumption side, instead of just dumping /proc/$pid/maps we would have to parse the file and convert all [anon:id] into [anon:name] with each conversion requiring an IPC call (assuming no id->name pair caching on the client side).

Quoted 59 lines · Suren Baghdasaryan

Quoted 33 lines · David Hildenbrand

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Valuable information

Yeah, I should have described it clearer the first time around.

Quoted 8 lines

Quoted 4 lines

Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

So, you'd actually want to generate/reserve an id for a given string at runtime, assign that id to the VMA, and have a way to match id <-> string somehow?

If we go with ids then yes, that is what we would have to do.

Quoted 2 lines

That reservation service could be inside the kernel or even (better?) in user space. The service could for example de-duplicates strings.

Quoted 6 lines

Yes but it would require an IPC call to that service potentially on every mmap() when we want to name a mapped vma. This would be prohibitive. Even on consumption side, instead of just dumping /proc/$pid/maps we would have to parse the file and convert all [anon:id] into [anon:name] with each conversion requiring an IPC call (assuming no id->name pair caching on the client side).

mmap() and prctl() already do take the mmap sem in write, so they are not the "most lightweight" operations so to say.

We already to have two separate operations, first the mmap(), then the prctl(). IMHO you could defer the "naming" part to a later point in time, without creating too many issues, moving it out of the "hot/performance critical phase"

Reading https://lwn.net/Articles/867818/, to me it feels like the use case could live with a little larger delay between the mmap popping up and a name getting assigned.

--
Thanks,

David / dhildenb

Quoted 8 lines

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

Quoted 1 line

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.
That leaves us with the central facility option, which as I described in my previous email would be prohibitive from performance POV (IPC every time we have a new name or want to convert id to name).

Hi!

Quoted 10 lines

Quoted 9 lines

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

Quoted 4 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name.

I'd be careful; if you allow special characters like that, you will confuse some kind of parser.

Quoted 7 lines

Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.
That leaves us with the central facility option, which as I described in my previous email would be prohibitive from performance POV (IPC every time we have a new name or want to convert id to name).

That "central facility" option can be as simple as "mkdir /somewhere/sanitized_id", using inode numbers for example. You don't really need IPC.

Plus, I don't really believe the IPC cost would be prohibitive.

Or you could simply hash the string and use the hash as id...

Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek

On Wed, Oct 6, 2021 at 10:58 AM Pavel Machek <[email protected]> wrote:

Quoted 3 lines

That "central facility" option can be as simple as "mkdir /somewhere/sanitized_id", using inode numbers for example. You don't really need IPC.

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

[...]

Quoted 9 lines

Quoted 3 lines

That "central facility" option can be as simple as "mkdir /somewhere/sanitized_id", using inode numbers for example. You don't really need IPC.

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

YOUR_IDS_DIR can live on a tmpfs and you can even implement a policy on top of that (who can generate new ids, gurantee uniqness etc...).

The above is certainly not for free of course but if you really need a system wide consistency when using names then you need some sort of central authority. How you implement that is not all that important but I do not think we want to handle that in the kernel.

Quoted 1 line · Suren Baghdasaryan

On Wed, Oct 6, 2021 at 10:58 AM Pavel Machek <[email protected]> wrote:

[...]

Quoted 9 lines

Quoted 3 lines

That "central facility" option can be as simple as "mkdir /somewhere/sanitized_id", using inode numbers for example. You don't really need IPC.

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

YOUR_IDS_DIR can live on a tmpfs and you can even implement a policy on top of that (who can generate new ids, gurantee uniqness etc...).

The above is certainly not for free of course but if you really need a system wide consistency when using names then you need some sort of central authority. How you implement that is not all that important but I do not think we want to handle that in the kernel.

Quoted 6 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

Quoted 4 lines

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

So in terms of syscall overhead, that would be open(..., O_CREAT | O_CLOEXEC), fstat(), close() - or one could optimistically start by

Quoted 7 lines

YOUR_IDS_DIR can live on a tmpfs and you can even implement a policy on top of that (who can generate new ids, gurantee uniqness etc...).

The above is certainly not for free of course but if you really need a system wide consistency when using names then you need some sort of central authority. How you implement that is not all that important but I do not think we want to handle that in the kernel.

IDK. If the whole thing could be put behind a CONFIG_ knob, with _zero_ overhead when not enabled (and I'm a bit worried about all the functions that grow an extra argument that gets passed around), I don't mind the string interface. But I don't really have a say either way.

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 3 lines

So in terms of syscall overhead, that would be open(..., O_CREAT | O_CLOEXEC), fstat(), close() - or one could optimistically start by

You could get to two if you used mkdir instead of open.

Quoted 7 lines

Quoted 7 lines

YOUR_IDS_DIR can live on a tmpfs and you can even implement a policy on top of that (who can generate new ids, gurantee uniqness etc...).

The above is certainly not for free of course but if you really need a system wide consistency when using names then you need some sort of central authority. How you implement that is not all that important but I do not think we want to handle that in the kernel.

Quoted 5 lines

IDK. If the whole thing could be put behind a CONFIG_ knob, with _zero_ overhead when not enabled (and I'm a bit worried about all the functions that grow an extra argument that gets passed around), I don't mind the string interface. But I don't really have a say either way.

If this is ever useful outside of Android, eventually distros will have it enabled.

Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

Quoted 15 lines · Suren Baghdasaryan

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

In a way this would be pushing policy into the kernel.

--
Michal Hocko
SUSE Labs

Quoted 34 lines · Michal Hocko

Quoted 15 lines · Suren Baghdasaryan

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 35 lines · Suren Baghdasaryan

Quoted 34 lines · Michal Hocko

Quoted 15 lines · Suren Baghdasaryan

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

--
Michal Hocko
SUSE Labs

Quoted 42 lines · Michal Hocko

Quoted 35 lines · Suren Baghdasaryan

Quoted 34 lines · Michal Hocko

Quoted 15 lines · Suren Baghdasaryan

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 7 lines

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Quoted 51 lines · Suren Baghdasaryan

Quoted 42 lines · Michal Hocko

Quoted 35 lines · Suren Baghdasaryan

Quoted 34 lines · Michal Hocko

Quoted 15 lines · Suren Baghdasaryan

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 7 lines

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

Quoted 6 lines

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.) For those not interested in the feature, it sounds like a CONFIG to keep it away would be sufficient. Can we just move forward with that?

--
Kees Cook

Quoted 42 lines

Quoted 41 lines

Quoted 27 lines

Quoted 22 lines

Quoted 22 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 7 lines

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Quoted 4 lines

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.) For those not interested in the feature, it sounds like a CONFIG to keep it away would be sufficient. Can we just move forward with that?

Would love to if others are ok with this.

On 10/7/21 11:50, Suren Baghdasaryan wrote:
...

Quoted 47 lines

Quoted 46 lines

Quoted 35 lines

Quoted 27 lines

Quoted 22 lines

Quoted 22 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 7 lines

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

Quoted 6 lines

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.) For those not interested in the feature, it sounds like a CONFIG to keep it away would be sufficient. Can we just move forward with that?

Quoted 2 lines

Would love to if others are ok with this.

If this doesn't get accepted, then another way forward would to continue the ideas above to their logical conclusion, and create a new file system:
vma-fs. Like debug-fs and other special file systems, similar policy and motivation. Also protected by a CONFIG option.

Actually this seems at least as natural as the procfs approach, especially given the nature of these strings, which feel more like dir+file names, than simple strings.

thanks,
--
John Hubbard
NVIDIA

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 53 lines · John Hubbard' via kernel-team

On 10/7/21 11:50, Suren Baghdasaryan wrote:
...

Quoted 49 lines

Quoted 42 lines

Quoted 35 lines

Quoted 27 lines

Quoted 22 lines

Quoted 22 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 7 lines

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

Quoted 6 lines

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Quoted 4 lines

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.) For those not interested in the feature, it sounds like a CONFIG to keep it away would be sufficient. Can we just move forward with that?

Would love to if others are ok with this.

Quoted 4 lines

If this doesn't get accepted, then another way forward would to continue the ideas above to their logical conclusion, and create a new file system:
vma-fs. Like debug-fs and other special file systems, similar policy and motivation. Also protected by a CONFIG option.

TBH, I would prefer to have the current simple solution protected with a CONFIG option.

Quoted 4 lines

Actually this seems at least as natural as the procfs approach, especially given the nature of these strings, which feel more like dir+file names, than simple strings.

On 10/7/21 11:50, Suren Baghdasaryan wrote:
...

Quoted 12 lines

Quoted 11 lines

Quoted 6 lines

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.) For those not interested in the feature, it sounds like a CONFIG to keep it away would be sufficient. Can we just move forward with that?

Quoted 2 lines

Would love to if others are ok with this.

If this doesn't get accepted, then another way forward would to continue the ideas above to their logical conclusion, and create a new file system:
vma-fs.

Quoted 15 lines · John Hubbard

On 10/7/21 11:50, Suren Baghdasaryan wrote:
...

Quoted 12 lines

Quoted 11 lines

Quoted 6 lines

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.) For those not interested in the feature, it sounds like a CONFIG to keep it away would be sufficient. Can we just move forward with that?

Quoted 6 lines

Quoted 2 lines

Would love to if others are ok with this.

If this doesn't get accepted, then another way forward would to continue the ideas above to their logical conclusion, and create a new file system:
vma-fs.

Or: Why can't the library/application that wants a VMA backed by memory to have some associated name not just

   fd = open("/run/named-vmas/foobar#24", O_CLOEXEC|O_RDWR|O_EXCL|O_CREAT);
   unlink("/run/named-vmas/foobar#24");
   ftruncate(fd, ...);
   mmap(fd);

where /run/named-vmas is a tmpfs (probably with some per-user/per-app subdirs). That requires no changes in the kernel at all. Yes, it lacks the automatic cleanup of using real anon mmap in case there's a crash between open and unlink, but in an environment like Android that should be solvable.

I'm going to point out that we already do have names for memfds.

[...] It's also displayed in /proc/self/maps.

Quoted 51 lines · Suren Baghdasaryan

Quoted 42 lines · Michal Hocko

Quoted 35 lines · Suren Baghdasaryan

Quoted 34 lines · Michal Hocko

Quoted 15 lines · Suren Baghdasaryan

Quoted 13 lines · Pavel Machek

Hi!

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Hmm, so the suggestion is to have some directory which contains files representing IDs, each containing the string name of the associated vma? Then let's say we are creating a new VMA and want to name it. We would have to scan that directory, check all files and see if any of them contain the name we want to reuse the same ID.

I believe Pavel meant something as simple as

$ YOUR_FILE=$YOUR_IDS_DIR/my_string_name
$ touch $YOUR_FILE
$ stat -c %i $YOUR_FILE

Quoted 10 lines

Ah, ok, now I understand the proposal. Thanks for the clarification! So, this would use filesystem as a directory for inode->name mappings. One rough edge for me is that the consumer would still need to parse /proc/$pid/maps and convert [anon:inode] into [anon:name] instead of just dumping the content for the user. Would it be acceptable if we require the ID provided by prctl() to always be a valid inode and show_map_vma() would do the inode-to-filename conversion when generating maps/smaps files? I know that inode->dentry is not one-to-one mapping but we can simply output the first dentry name. WDYT?

No. You do not want to dictate any particular way of the mapping. The above is just one way to do that without developing any actual mapping yourself. You just use a filesystem for that. Kernel doesn't and shouldn't understand the meaning of those numbers. It has no business in that.

Quoted 4 lines

Quoted 1 line

In a way this would be pushing policy into the kernel.

I can see your point. Any other ideas on how to prevent tools from doing this id-to-name conversion themselves?

Quoted 7 lines

I really fail to understand why you really want to prevent them from that. Really, the whole thing is just a cookie that kernel maintains for memory mappings so that two parties can understand what the meaning of that mapping is from a higher level. They both have to agree on the naming but the kernel shouldn't dictate any specific convention because the kernel _doesn't_ _care_. These things are not really anything actionable for the kernel. It is just a metadata.

Quoted 6 lines

The desire is for one of these two parties to be a human who can get the data and use it as is without additional conversions. /proc/$pid/maps could report FD numbers instead of pathnames, which could be converted to pathnames in userspace. However we do not do that because pathnames are more convenient for humans to identify a specific resource. Same logic applies here IMHO.

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.)

For those not interested in the
feature, it sounds like a CONFIG to keep it away would be sufficient.

Quoted 2 lines

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.)

I am sorry but there were no strong arguments mentioned for strings so far.

Quoted 4 lines · Michal Hocko

Quoted 2 lines

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.)

I am sorry but there were no strong arguments mentioned for strings so far.

The folks who want this have maintained an out-of-tree patch using strings. They've maintained it for the better part of a decade. I don't know how widely this shipped in the Android ecosystem, but I suspect we're talking about billions of devices. Right?

This is a feature that, if accepted into mainline, will get enabled and used on billions of devices. If we dumb this down to integers, it's not 100% clear that it _will_ get used.

That's a pretty strong argument in my book, even if the contributors have difficulty articulating exactly why they want strings.

Quoted 3 lines · Dave Hansen

Quoted 2 lines · Michal Hocko

Quoted 2 lines

Yes, please. It really seems like the folks that are interested in this feature want strings. (I certainly do.)

Quoted 7 lines

Quoted 2 lines

I am sorry but there were no strong arguments mentioned for strings so far.

The folks who want this have maintained an out-of-tree patch using strings. They've maintained it for the better part of a decade. I don't know how widely this shipped in the Android ecosystem, but I suspect we're talking about billions of devices. Right?

Quoted 4 lines

This is a feature that, if accepted into mainline, will get enabled and used on billions of devices. If we dumb this down to integers, it's not 100% clear that it _will_ get used.

Quoted 3 lines

That's a pretty strong argument in my book, even if the contributors have difficulty articulating exactly why they want strings.

I would agree that if integers would make this unusable then this would be a strong argument. But I haven't really heard any arguments like that so far. I have heard about IPC overhead and other speculations that do not seem really convincing. We shouldn't hand wave concerns regarding the implementation complexity and resource handling just by "somebody has been using this for decates", right?

Do not get me wrong. This is going to become a user interface and we will have to maintain it for ever. As such an extra scrutiny has to be applied.

--
Michal Hocko
SUSE Labs

I am sorry but there were no strong arguments mentioned for strings so far. Effectively string require a more complex and more resource hungry solution. The only advantage is that strings are nicer to read for humans.

There hasn't been any plan presented for actual naming convention or how those names would be used in practice. Except for a more advanced resource management and that sounds like something that can work with ids just fine.

Initially I was not really feeling strongly one way or other but more we are discussing the topic the more I see that strings have a very weak justification behind.

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

--
Michal Hocko
SUSE Labs

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

I went ahead and implemented the proposed userspace solution involving tmpfs as a repository for name->id mapping (more precisely filename->inode mapping). Profiling shows that open()+fstat()+close() takes:
- roughly 15 times longer than mmap() with 1000 unique names each being reused 50 times.
- roughly 3 times longer than mmap() with 100 unique names each being reused 500 times. This is due to lstat() optimization suggested by Rasmus which avoids open() and close().
For comparison, proposed prctl() takes roughly the same amount of time as mmap() and does not depend on the number of unique names.

I'm still evaluating the proposal to use memfds but I'm not sure if the issue that David Hildenbrand mentioned about additional memory consumed in pagecache (which has to be addressed) is the only one we will encounter with this approach. If anyone knows of any potential issues with using memfds as named anonymous memory, I would really appreciate your feedback before I go too far in that direction. Thanks,
Suren.

Quoted 4 lines

--
Michal Hocko
SUSE Labs

Quoted 30 lines · Suren Baghdasaryan

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · Suren Baghdasaryan

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Is that something you need to do client-side? Or could the bug tool upload the userspace-maintained name:ids database alongside the /proc/pid/maps dump for external processing?

Quoted 44 lines · Johannes Weiner

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · Suren Baghdasaryan

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Quoted 3 lines

Is that something you need to do client-side? Or could the bug tool upload the userspace-maintained name:ids database alongside the /proc/pid/maps dump for external processing?

You can generate a bugreport and analyze it locally or submit it as an attachment to a bug for further analyzes.
Sure, we can attach the id->name conversion table to the bugreport but either way, some tool would have to post-process it to resolve the ids. If we are not analyzing the results immediately then that step can be postponed and I think that's what you mean? If so, then yes, that is correct.

Quoted 49 lines · Suren Baghdasaryan

Quoted 44 lines · Johannes Weiner

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · Suren Baghdasaryan

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Quoted 3 lines

Is that something you need to do client-side? Or could the bug tool upload the userspace-maintained name:ids database alongside the /proc/pid/maps dump for external processing?

Quoted 7 lines

You can generate a bugreport and analyze it locally or submit it as an attachment to a bug for further analyzes.
Sure, we can attach the id->name conversion table to the bugreport but either way, some tool would have to post-process it to resolve the ids. If we are not analyzing the results immediately then that step can be postponed and I think that's what you mean? If so, then yes, that is correct.

Right, somebody needs to do it at some point, but I suppose it's less of a problem if a developer machine does it than a mobile device.

One advantage of an ID over a string - besides not having to maintain a deduplicating arbitrary string storage in the kernel - is that we may be able to auto-assign unique IDs to VMAs in the kernel, in a way that we could not with strings. You'd still have to do IPC calls to write new name mappings into your db, but you wouldn't have to do the prctl() to assign stuff in the kernel at all.

(We'd have to think of a solution of how IDs work with vma merging and splitting, but I think to a certain degree that's policy and we should be able to find something workable - a MAP_ID flag, using anon_vma as identity, assigning IDs at mmap time and do merges only for protection changes etc. etc.)

Quoted 59 lines · Johannes Weiner

Quoted 49 lines · Suren Baghdasaryan

Quoted 44 lines · Johannes Weiner

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · Suren Baghdasaryan

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Quoted 3 lines

Is that something you need to do client-side? Or could the bug tool upload the userspace-maintained name:ids database alongside the /proc/pid/maps dump for external processing?

Quoted 7 lines

You can generate a bugreport and analyze it locally or submit it as an attachment to a bug for further analyzes.
Sure, we can attach the id->name conversion table to the bugreport but either way, some tool would have to post-process it to resolve the ids. If we are not analyzing the results immediately then that step can be postponed and I think that's what you mean? If so, then yes, that is correct.

Quoted 2 lines

Right, somebody needs to do it at some point, but I suppose it's less of a problem if a developer machine does it than a mobile device.

True, and that's why I mentioned that it's not as critical as the efficiency at mmap() time. In any case, if we could avoid translations at all that would be ideal.

Quoted 7 lines

One advantage of an ID over a string - besides not having to maintain a deduplicating arbitrary string storage in the kernel - is that we may be able to auto-assign unique IDs to VMAs in the kernel, in a way that we could not with strings. You'd still have to do IPC calls to write new name mappings into your db, but you wouldn't have to do the prctl() to assign stuff in the kernel at all.

You still have to retrieve that tag from the kernel to record it in your db, so this would still require some syscall, no?

Quoted 5 lines

(We'd have to think of a solution of how IDs work with vma merging and splitting, but I think to a certain degree that's policy and we should be able to find something workable - a MAP_ID flag, using anon_vma as identity, assigning IDs at mmap time and do merges only for protection changes etc. etc.)

Overall, I think keeping the kernel out of this and letting it treat this tag as a cookie which only userspace cares about is simpler. Unless you see other uses where kernel's involvement is needed.

Quoted 75 lines · Suren Baghdasaryan

Quoted 59 lines · Johannes Weiner

Quoted 49 lines · Suren Baghdasaryan

Quoted 44 lines · Johannes Weiner

Quoted 31 lines · Suren Baghdasaryan

Quoted 30 lines · Suren Baghdasaryan

Quoted 13 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

Quoted 4 lines

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Quoted 3 lines

Is that something you need to do client-side? Or could the bug tool upload the userspace-maintained name:ids database alongside the /proc/pid/maps dump for external processing?

Quoted 7 lines

You can generate a bugreport and analyze it locally or submit it as an attachment to a bug for further analyzes.
Sure, we can attach the id->name conversion table to the bugreport but either way, some tool would have to post-process it to resolve the ids. If we are not analyzing the results immediately then that step can be postponed and I think that's what you mean? If so, then yes, that is correct.

Quoted 2 lines

Right, somebody needs to do it at some point, but I suppose it's less of a problem if a developer machine does it than a mobile device.

True, and that's why I mentioned that it's not as critical as the efficiency at mmap() time. In any case, if we could avoid translations at all that would be ideal.

Quoted 7 lines

One advantage of an ID over a string - besides not having to maintain a deduplicating arbitrary string storage in the kernel - is that we may be able to auto-assign unique IDs to VMAs in the kernel, in a way that we could not with strings. You'd still have to do IPC calls to write new name mappings into your db, but you wouldn't have to do the prctl() to assign stuff in the kernel at all.

Quoted 2 lines

You still have to retrieve that tag from the kernel to record it in your db, so this would still require some syscall, no?

Don't you have to do this with the string setting interface as well? How do you know the vma address to pass into the prctl()? Is this somehow coordinated with the mmap()?

Quoted 7 lines

Quoted 6 lines

(We'd have to think of a solution of how IDs work with vma merging and splitting, but I think to a certain degree that's policy and we should be able to find something workable - a MAP_ID flag, using anon_vma as identity, assigning IDs at mmap time and do merges only for protection changes etc. etc.)

Quoted 3 lines

Overall, I think keeping the kernel out of this and letting it treat this tag as a cookie which only userspace cares about is simpler. Unless you see other uses where kernel's involvement is needed.

It depends on what you consider keeping the kernel out of it. A small extension to assign unique IDs to mappings automatically in an intuitive way (with a compat option to disable) is a much smaller ABI commitment than a prctl()-controlled string storage.

When I say policy on how to assign the ID, I didn't mean that it should be a free for all. Rather that we should pick one reasonable way to do it, comparable to picking the parameters for how long the stored strings could be, which characters to allow etc.

Quoted 20 lines · Suren Baghdasaryan

Quoted 17 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

Quoted 9 lines

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

Quoted 10 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Quoted 21 lines · Suren Baghdasaryan

Quoted 20 lines · Suren Baghdasaryan

Quoted 17 lines · Michal Hocko

Quoted 5 lines · Kees Cook

- Strings for "anon" specifically have no required format (this is good) it's informational like the task_struct::comm and can (roughly) anything. There's no naming convention for memfds, AF_UNIX, etc. Why is one needed here? That seems like a completely unreasonable requirement.

I might be misreading the justification for the feature. Patch 2 is talking about tools that need to understand memeory usage to make further actions. Also Suren was suggesting "numbering convetion" as an argument against.

So can we get a clear example how is this being used actually? If this is just to be used to debug by humans than I can see an argument for human readable form. If this is, however, meant to be used by tools to make some actions then the argument for strings is much weaker.

Quoted 9 lines

Quoted 9 lines

The simplest usecase is when we notice that a process consumes more memory than usual and we do "cat /proc/$(pidof my_process)/maps" to check which area is contributing to this growth. The names we assign to anonymous areas are descriptive enough for a developer to get an idea where the increased consumption is coming from and how to proceed with their investigation.
There are of course cases when tools are involved, but the end-user is always a human and the final report should contain easily understandable data.

OK, it would have been much more preferable to be explicit about this main use case from the very beginning. Just to make sure we are at the same page. Is the primary usecase usage and bug reporting?

My initial understanding was that at userspace managed memory management could make an educated guess about targeted reclaim (e.g. MADV_{FREE,COLD,PAGEOUT} for cached data in memory like uncompressed images/data). Such a usecase would clearly require a standardized id/naming convention to be application neutral.

Quoted 11 lines

Quoted 11 lines

IIUC, the main argument here is whether the userspace can provide tools to perform the translations between ids and names, with the kernel accepting and reporting ids instead of strings. Technically it's possible, but to be practical that conversion should be fast because we will need to make name->id conversion potentially for each mmap. On the consumer side the performance is not as critical, but the fact that instead of dumping /proc/$pid/maps we will have to parse the file, do id->name conversion and replace all [anon:id] with [anon:name] would be an issue when we do that in bulk, for example when collecting system-wide data for a bugreport.

Whether you use ids or human readable strings you still have to understand the underlying meaning to make any educated guess. Let me give you an example. Say I have an application with a memory leak. Right now I can only tell that it is anonymous memory growing but it is not clear who uses that anonymous. You are adding a means to tell different users appart. That is really helpful. Now I know this is an anon user 1234 or MySuperAnonMemory. Neither of the will not tell me more without a id/naming convention or reading the code. A convention can be useful for the most common users (e.g. a specific allocator) but I am rather dubious there are many more that would be _generally_ recognized without some understanding of the said application.

Maybe the situation in Android is different because the runtime is more coupled but is it reasonable to expect any common naming conventions for general Linux platforms?

I am slightly worried that we have spent way too much time talking specifics about id->name translation rather than the actual usability of the token.

--
Michal Hocko
SUSE Labs

I'm still evaluating the proposal to use memfds but I'm not sure if the issue that David Hildenbrand mentioned about additional memory consumed in pagecache (which has to be addressed) is the only one we will encounter with this approach. If anyone knows of any potential issues with using memfds as named anonymous memory, I would really appreciate your feedback before I go too far in that direction.

Quoted 7 lines

I'm still evaluating the proposal to use memfds but I'm not sure if the issue that David Hildenbrand mentioned about additional memory consumed in pagecache (which has to be addressed) is the only one we will encounter with this approach. If anyone knows of any potential issues with using memfds as named anonymous memory, I would really appreciate your feedback before I go too far in that direction.

[MAP_PRIVATE memfd only behave that way with 4k, not with huge pages, so I think it just has to be fixed. It doesn't make any sense to allocate a page for the pagecache ("populate the file") when accessing via a private mapping that's supposed to leave the file untouched]

My gut feeling is if you really need a string as identifier, then try going with memfds. Yes, we might hit some road blocks to be sorted out, but it just logically makes sense to me: Files have names. These names exist before mapping and after mapping. They "name" the content.

Maybe it's just me, but the whole interface, setting the name via a prctl after the mapping was already instantiated doesn't really spark joy at my end. That's not a strong pushback, but if we can avoid it using something that's already there, that would be very much preferred.

--
Thanks,

David / dhildenb

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 13 lines · David Hildenbrand

Quoted 7 lines

I'm still evaluating the proposal to use memfds but I'm not sure if the issue that David Hildenbrand mentioned about additional memory consumed in pagecache (which has to be addressed) is the only one we will encounter with this approach. If anyone knows of any potential issues with using memfds as named anonymous memory, I would really appreciate your feedback before I go too far in that direction.

[MAP_PRIVATE memfd only behave that way with 4k, not with huge pages, so I think it just has to be fixed. It doesn't make any sense to allocate a page for the pagecache ("populate the file") when accessing via a private mapping that's supposed to leave the file untouched]

Quoted 4 lines

My gut feeling is if you really need a string as identifier, then try going with memfds. Yes, we might hit some road blocks to be sorted out, but it just logically makes sense to me: Files have names. These names exist before mapping and after mapping. They "name" the content.

I'm investigating this direction. I don't have much background with memfds, so I'll need to digest the code first.

Quoted 6 lines

Maybe it's just me, but the whole interface, setting the name via a prctl after the mapping was already instantiated doesn't really spark joy at my end. That's not a strong pushback, but if we can avoid it using something that's already there, that would be very much preferred.

Actually that's one of my worries about using memfds. There might be cases when we need to name a vma after it was mapped. memfd_create() would not allow us to do that AFAIKT. But I need to check all usages to say if that's really an issue.
Thanks!

Quoted 9 lines

--
Thanks,

David / dhildenb

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 20 lines · Suren Baghdasaryan

Quoted 17 lines · David Hildenbrand

Quoted 7 lines

I'm still evaluating the proposal to use memfds but I'm not sure if the issue that David Hildenbrand mentioned about additional memory consumed in pagecache (which has to be addressed) is the only one we will encounter with this approach. If anyone knows of any potential issues with using memfds as named anonymous memory, I would really appreciate your feedback before I go too far in that direction.

[MAP_PRIVATE memfd only behave that way with 4k, not with huge pages, so I think it just has to be fixed. It doesn't make any sense to allocate a page for the pagecache ("populate the file") when accessing via a private mapping that's supposed to leave the file untouched]

My gut feeling is if you really need a string as identifier, then try going with memfds. Yes, we might hit some road blocks to be sorted out, but it just logically makes sense to me: Files have names. These names exist before mapping and after mapping. They "name" the content.

Quoted 2 lines

I'm investigating this direction. I don't have much background with memfds, so I'll need to digest the code first.

I've done some investigation into the possibility of using memfds to name anonymous VMAs. Here are my findings:

1. Forking a process with anonymous vmas named using memfd is 5-15% slower than with prctl (depends on the number of VMAs in the process being forked). Profiling shows that i_mmap_lock_write() dominates dup_mmap(). Exit path is also slower by roughly 9% with free_pgtables() and fput() dominating exit_mmap(). Fork performance is important for Android because almost all processes are forked from zygote, therefore this limitation already makes this approach prohibitive.

2. mremap() usage to grow the mapping has an issue when used with memfds:

fd = memfd_create(name, MFD_ALLOW_SEALING); ftruncate(fd, size_bytes);
ptr = mmap(NULL, size_bytes, prot, MAP_PRIVATE, fd, 0); close(fd);
ptr = mremap(ptr, size_bytes, size_bytes * 2, MREMAP_MAYMOVE); touch_mem(ptr, size_bytes * 2);

This would generate a SIGBUS in touch_mem(). I believe it's because ftruncate() specified the size to be size_bytes and we are accessing more than that after remapping. prctl() does not have this limitation and we do have a usecase for growing a named VMA.

3. Leaves an fd exposed, even briefly, which may lead to unexpected flaws (e.g. anything using mmap MAP_SHARED could allow exposures or overwrites). Even MAP_PRIVATE, if an attacker writes into the file after ftruncate() and before mmap(), can cause private memory to be initialized with unexpected data.

4. There is a usecase in the Android userspace where vma naming happens after memory was allocated. Bionic linker does in-memory relocations and then names some relocated sections.

In the light of these findings, could the current patchset be reconsidered? Thanks,
Suren.

Quoted 21 lines · Suren Baghdasaryan

Quoted 20 lines · Suren Baghdasaryan

Quoted 17 lines · David Hildenbrand

Quoted 7 lines

I'm still evaluating the proposal to use memfds but I'm not sure if the issue that David Hildenbrand mentioned about additional memory consumed in pagecache (which has to be addressed) is the only one we will encounter with this approach. If anyone knows of any potential issues with using memfds as named anonymous memory, I would really appreciate your feedback before I go too far in that direction.

[MAP_PRIVATE memfd only behave that way with 4k, not with huge pages, so I think it just has to be fixed. It doesn't make any sense to allocate a page for the pagecache ("populate the file") when accessing via a private mapping that's supposed to leave the file untouched]

My gut feeling is if you really need a string as identifier, then try going with memfds. Yes, we might hit some road blocks to be sorted out, but it just logically makes sense to me: Files have names. These names exist before mapping and after mapping. They "name" the content.

Quoted 5 lines

Quoted 2 lines

I'm investigating this direction. I don't have much background with memfds, so I'll need to digest the code first.

I've done some investigation into the possibility of using memfds to name anonymous VMAs. Here are my findings:

Thanks for exploring the alternatives!

Quoted 10 lines

1. Forking a process with anonymous vmas named using memfd is 5-15% slower than with prctl (depends on the number of VMAs in the process being forked). Profiling shows that i_mmap_lock_write() dominates dup_mmap(). Exit path is also slower by roughly 9% with free_pgtables() and fput() dominating exit_mmap(). Fork performance is important for Android because almost all processes are forked from zygote, therefore this limitation already makes this approach prohibitive.

Interesting, naturally I wonder if that can be optimized.

Quoted 11 lines

2. mremap() usage to grow the mapping has an issue when used with memfds:

fd = memfd_create(name, MFD_ALLOW_SEALING); ftruncate(fd, size_bytes);
ptr = mmap(NULL, size_bytes, prot, MAP_PRIVATE, fd, 0); close(fd);
ptr = mremap(ptr, size_bytes, size_bytes * 2, MREMAP_MAYMOVE); touch_mem(ptr, size_bytes * 2);

Quoted 4 lines

This would generate a SIGBUS in touch_mem(). I believe it's because ftruncate() specified the size to be size_bytes and we are accessing more than that after remapping. prctl() does not have this limitation and we do have a usecase for growing a named VMA.

Can't you simply size the memfd much larger? I mean, it doesn't really cost much, does it?

Quoted 7 lines

3. Leaves an fd exposed, even briefly, which may lead to unexpected flaws (e.g. anything using mmap MAP_SHARED could allow exposures or overwrites). Even MAP_PRIVATE, if an attacker writes into the file after ftruncate() and before mmap(), can cause private memory to be initialized with unexpected data.

I don't quite follow. Can you elaborate what exactly the issue here is? We use a temporary fd, yes, but how is that a problem?

Any attacker can just write any random memory memory in the address space, so I don't see the issue.

Quoted 5 lines

4. There is a usecase in the Android userspace where vma naming happens after memory was allocated. Bionic linker does in-memory relocations and then names some relocated sections.

Would renaming a memfd be an option or is that "too late" ?

Quoted 6 lines

In the light of these findings, could the current patchset be reconsidered? Thanks,
Suren.

--
Thanks,

David / dhildenb

Quoted 11 lines

Quoted 10 lines

1. Forking a process with anonymous vmas named using memfd is 5-15% slower than with prctl (depends on the number of VMAs in the process being forked). Profiling shows that i_mmap_lock_write() dominates dup_mmap(). Exit path is also slower by roughly 9% with free_pgtables() and fput() dominating exit_mmap(). Fork performance is important for Android because almost all processes are forked from zygote, therefore this limitation already makes this approach prohibitive.

Quoted 1 line

Interesting, naturally I wonder if that can be optimized.

Maybe but it looks like we simply do additional things for file-backed memory, which seems natural. The call to i_mmap_lock_write() is from
here: https://elixir.bootlin.com/linux/latest/source/kernel/fork.c#L565

Quoted 16 lines

Quoted 15 lines

2. mremap() usage to grow the mapping has an issue when used with memfds:

fd = memfd_create(name, MFD_ALLOW_SEALING); ftruncate(fd, size_bytes);
ptr = mmap(NULL, size_bytes, prot, MAP_PRIVATE, fd, 0); close(fd);
ptr = mremap(ptr, size_bytes, size_bytes * 2, MREMAP_MAYMOVE); touch_mem(ptr, size_bytes * 2);

This would generate a SIGBUS in touch_mem(). I believe it's because ftruncate() specified the size to be size_bytes and we are accessing more than that after remapping. prctl() does not have this limitation and we do have a usecase for growing a named VMA.

Quoted 2 lines

Can't you simply size the memfd much larger? I mean, it doesn't really cost much, does it?

If we know beforehand what the max size it can reach then that would be possible. I would really hate to miscalculate here and cause a simple memory access to generate signals. Tracking such corner cases in the field is not an easy task and I would rather avoid the possibility of it.

Quoted 7 lines

Quoted 5 lines

4. There is a usecase in the Android userspace where vma naming happens after memory was allocated. Bionic linker does in-memory relocations and then names some relocated sections.

Would renaming a memfd be an option or is that "too late" ?

My understanding is that linker allocates space to load and relocate the code, performs the relocations in that space and then names some of the regions after that. Whether it can be redesigned to allocate multiple named regions and perform the relocation between them I did not really try since it would be a project by itself.

TBH, at some point I just look at the amount of required changes (both kernel and userspace) and new limitations that userspace has to adhere to for fitting memfds to my usecase, and I feel that it's just not worth it. In the end we end up using the same refcounted strings with vma->vm_file->f_count as the refcount and name stored in vma->vm_file->f_path->dentry but with more overhead.

Quoted 34 lines

Quoted 12 lines

Quoted 11 lines

1. Forking a process with anonymous vmas named using memfd is 5-15% slower than with prctl (depends on the number of VMAs in the process being forked). Profiling shows that i_mmap_lock_write() dominates dup_mmap(). Exit path is also slower by roughly 9% with free_pgtables() and fput() dominating exit_mmap(). Fork performance is important for Android because almost all processes are forked from zygote, therefore this limitation already makes this approach prohibitive.

Quoted 1 line

Interesting, naturally I wonder if that can be optimized.

Maybe but it looks like we simply do additional things for file-backed memory, which seems natural. The call to i_mmap_lock_write() is from
here: https://elixir.bootlin.com/linux/latest/source/kernel/fork.c#L565

Quoted 16 lines

Quoted 15 lines

2. mremap() usage to grow the mapping has an issue when used with memfds:

fd = memfd_create(name, MFD_ALLOW_SEALING); ftruncate(fd, size_bytes);
ptr = mmap(NULL, size_bytes, prot, MAP_PRIVATE, fd, 0); close(fd);
ptr = mremap(ptr, size_bytes, size_bytes * 2, MREMAP_MAYMOVE); touch_mem(ptr, size_bytes * 2);

This would generate a SIGBUS in touch_mem(). I believe it's because ftruncate() specified the size to be size_bytes and we are accessing more than that after remapping. prctl() does not have this limitation and we do have a usecase for growing a named VMA.

Quoted 8 lines

Quoted 2 lines

Can't you simply size the memfd much larger? I mean, it doesn't really cost much, does it?

If we know beforehand what the max size it can reach then that would be possible. I would really hate to miscalculate here and cause a simple memory access to generate signals. Tracking such corner cases in the field is not an easy task and I would rather avoid the possibility of it.

The question would be if you cannot simply add some extremely large number, because the file size itself doesn't really matter for memfd IIRC.

Having that said, without trying it out, I wouldn't know from the top of my head if memremap would work that way on an already closed fd that ahs a sufficient size :/ If you have the example still somewhere, I would be interested if that would work in general.

[...]

Quoted 14 lines

Quoted 7 lines

Quoted 5 lines

4. There is a usecase in the Android userspace where vma naming happens after memory was allocated. Bionic linker does in-memory relocations and then names some relocated sections.

Would renaming a memfd be an option or is that "too late" ?

My understanding is that linker allocates space to load and relocate the code, performs the relocations in that space and then names some of the regions after that. Whether it can be redesigned to allocate multiple named regions and perform the relocation between them I did not really try since it would be a project by itself.

Quoted 6 lines

TBH, at some point I just look at the amount of required changes (both kernel and userspace) and new limitations that userspace has to adhere to for fitting memfds to my usecase, and I feel that it's just not worth it. In the end we end up using the same refcounted strings with vma->vm_file->f_count as the refcount and name stored in vma->vm_file->f_path->dentry but with more overhead.

Yes, but it's glued to files which naturally have names :)

Again, I appreciate that you looked into alternatives! I can see the late renaming could be the biggest blocker if user space cannot be adjusted easily to be compatible with that using memfds.

--
Thanks,

David / dhildenb

Quoted 12 lines · David Hildenbrand

Quoted 6 lines · Suren Baghdasaryan

[...]
3. Leaves an fd exposed, even briefly, which may lead to unexpected flaws (e.g. anything using mmap MAP_SHARED could allow exposures or overwrites). Even MAP_PRIVATE, if an attacker writes into the file after ftruncate() and before mmap(), can cause private memory to be initialized with unexpected data.

I don't quite follow. Can you elaborate what exactly the issue here is? We use a temporary fd, yes, but how is that a problem?

Quoted 2 lines

Any attacker can just write any random memory memory in the address space, so I don't see the issue.

It feels to me that introducing another handle to the memory region is a potential attack vector but I'm not a security expert. Maybe Kees can assess this better?

Quoted 26 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

Quoted 5 lines

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name. Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

Quoted 33 lines · Suren Baghdasaryan

Quoted 26 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

Quoted 5 lines

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

Quoted 7 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name. Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

I still do not follow. If you need a globaly consistent naming then you need clear rules for that, no matter whether that is number or a file. How do you handle this with strings currently?

--
Michal Hocko
SUSE Labs

Quoted 43 lines · Michal Hocko

Quoted 33 lines · Suren Baghdasaryan

Quoted 26 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

Quoted 5 lines

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

Quoted 7 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name. Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

Quoted 3 lines

I still do not follow. If you need a globaly consistent naming then you need clear rules for that, no matter whether that is number or a file. How do you handle this with strings currently?

Some names represent standard categories, some are unique. A simple tool could calculate and report the total for each name, a more advanced tool might recognize some standard names and process them differently. From kernel's POV, it's just a name used by the userspace to categorize anonymous memory areas.

Quoted 48 lines · Suren Baghdasaryan

Quoted 43 lines · Michal Hocko

Quoted 33 lines · Suren Baghdasaryan

Quoted 26 lines · David Hildenbrand

Quoted 15 lines · Michal Hocko

On Tue 05-10-21 23:57:36, John Hubbard wrote:
[...]

Quoted 3 lines

1) Yes, just leave the strings in the kernel, that's simple and it works, and the alternatives don't really help your case nearly enough.

I do not have a strong opinion. Strings are easier to use but they are more involved and the necessity of kref approach just underlines that. There are going to be new allocations and that always can lead to surprising side effects. These are small (80B at maximum) so the overall footpring shouldn't all that large by default but it can grow quite large with a very high max_map_count. There are workloads which really require the default to be set high (e.g. heavy mremap users). So if anything all those should be __GFP_ACCOUNT and memcg accounted.

Quoted 2 lines

I do agree that numbers are just much more simpler from accounting, performance and implementation POV.

+1

I can understand that having a string can be quite beneficial e.g., when dumping mmaps. If only user space knows the id <-> string mapping, that can be quite tricky.

Quoted 5 lines

However, I also do wonder if there would be a way to standardize/reserve ids, such that a given id always corresponds to a specific user. If we use an uint64_t for an id, there would be plenty room to reserve ids ...

I'd really prefer if we can avoid using strings and instead using ids.

Quoted 7 lines

I wish it was that simple and for some names like [anon:.bss] or [anon:dalvik-zygote space] reserving a unique id would work, however some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] are generated dynamically at runtime and include package name. Packages are constantly evolving, new ones are developed, names can change, etc. So assigning a unique id for these names is not really feasible.

Quoted 3 lines

I still do not follow. If you need a globaly consistent naming then you need clear rules for that, no matter whether that is number or a file. How do you handle this with strings currently?

Quoted 5 lines

Some names represent standard categories, some are unique. A simple tool could calculate and report the total for each name, a more advanced tool might recognize some standard names and process them differently. From kernel's POV, it's just a name used by the userspace to categorize anonymous memory areas.

OK, so there is no real authority or any real naming convention. You just hope that applications will behave so that the consumer of those names can make proper calls. Correct?

In that case the same applies to numbers and I do not see any strong argument for strings other than it is more pleasing to a human eye when reading the file. And that doesn't sound like a strong argument to make the kernel more complicated. Functionally both approaches are equal from a practical POV.

--
Michal Hocko
SUSE Labs

On Thu, Oct 7, 2021 at 9:37 AM Michal Hocko <[email protected]> wrote:

Quoted 9 lines

OK, so there is no real authority or any real naming convention. You just hope that applications will behave so that the consumer of those names can make proper calls. Correct?

In that case the same applies to numbers and I do not see any strong argument for strings other than it is more pleasing to a human eye when reading the file. And that doesn't sound like a strong argument to make the kernel more complicated. Functionally both approaches are equal from a practical POV.

I don't think that's correct. Names like [anon:.bss], [anon:dalvik-zygote space] and [anon:dalvik-/system/framework/boot-core-icu4j.art] provide user with actionable information about the use of that memory or the allocator using it.

Quoted 1 line · Suren Baghdasaryan

On Thu, Oct 7, 2021 at 9:37 AM Michal Hocko <[email protected]> wrote:

[...]

Quoted 10 lines

Quoted 9 lines

OK, so there is no real authority or any real naming convention. You just hope that applications will behave so that the consumer of those names can make proper calls. Correct?

In that case the same applies to numbers and I do not see any strong argument for strings other than it is more pleasing to a human eye when reading the file. And that doesn't sound like a strong argument to make the kernel more complicated. Functionally both approaches are equal from a practical POV.

Quoted 5 lines

I don't think that's correct. Names like [anon:.bss], [anon:dalvik-zygote space] and [anon:dalvik-/system/framework/boot-core-icu4j.art] provide user with actionable information about the use of that memory or the allocator using it.

No, none of the above is really actionable without a common understanding. Both dalvik* are a complete gibberish to me.

--
Michal Hocko
SUSE Labs

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

mm/madvise.c static int madvise_need_mmap_write(int behavior) · line 66

Quoted 10 lines

mm/madvise.c +1 -2

@@ -63,76 +63,20 @@ static int madvise_need_mmap_write(int behavior)
 }
 
 /*
- * We can potentially split a vm area into separate
- * areas, each area with its own behavior.
+ * Update the vm_flags on regiion of a vma, splitting it or merging it as

                                ^^

Eike
--
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160 Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 6 lines · Rolf Eike Beer

Quoted 6 lines

mm/madvise.c +0 -0

@@ -63,76 +63,20 @@ static int madvise_need_mmap_write(int behavior)
 }
 

Quoted 5 lines

mm/madvise.c static int madvise_need_mmap_write(int behavior) · line 66

Quoted 4 lines

mm/madvise.c +1 -2

 /*
- * We can potentially split a vm area into separate
- * areas, each area with its own behavior.
+ * Update the vm_flags on regiion of a vma, splitting it or merging it as

                                ^^

Thanks! Will fix in the next version.

Quoted 14 lines

Eike
--
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160 Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 13 lines · Suren Baghdasaryan

Quoted 11 lines · Rolf Eike Beer

mm/madvise.c static int madvise_need_mmap_write(int behavior) · line 66

Quoted 10 lines

mm/madvise.c +1 -2

@@ -63,76 +63,20 @@ static int madvise_need_mmap_write(int behavior)
 }
 
 /*
- * We can potentially split a vm area into separate
- * areas, each area with its own behavior.
+ * Update the vm_flags on regiion of a vma, splitting it or merging it as

                                ^^

Quoted 1 line

Thanks! Will fix in the next version.

Since you'll be respinning for this comment, can you please point out that the split will keep the VMA as [vma->vm_start, new_end)? That is, __split_vma() is passed 0 for new_below. It might prove useful since the code is being reused.

Thanks,
Liam

Quoted 15 lines

Quoted 14 lines

Eike
--
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160 Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

--
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Quoted 15 lines · Liam Howlett

Quoted 13 lines · Suren Baghdasaryan

Quoted 11 lines · Rolf Eike Beer

mm/madvise.c static int madvise_need_mmap_write(int behavior) · line 66

Quoted 10 lines

mm/madvise.c +1 -2

@@ -63,76 +63,20 @@ static int madvise_need_mmap_write(int behavior)
 }
 
 /*
- * We can potentially split a vm area into separate
- * areas, each area with its own behavior.
+ * Update the vm_flags on regiion of a vma, splitting it or merging it as

                                ^^

Quoted 6 lines

Quoted 1 line

Thanks! Will fix in the next version.

Since you'll be respinning for this comment, can you please point out that the split will keep the VMA as [vma->vm_start, new_end)? That is, __split_vma() is passed 0 for new_below. It might prove useful since the code is being reused.

Hmm. There are two cases here:

        if (start != vma->vm_start) {
                ...
                error = __split_vma(mm, vma, start, 1);
        }

and

        if (end != vma->vm_end) {
                ...
                error = __split_vma(mm, vma, end, 0);
        }

so, I don't think such a comment would be completely correct, no?