| Message ID | cover.1681430907.git.ackerleytng@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Setting memory policy for restrictedmem file | expand |
On Fri 14-04-23 00:11:49, Ackerley Tng wrote: > Hello, > > This patchset builds upon the memfd_restricted() system call that was > discussed in the 'KVM: mm: fd-based approach for supporting KVM' patch > series [1]. > > The tree can be found at: > https://github.com/googleprodkernel/linux-cc/tree/restrictedmem-set-memory-policy > > In this patchset, a new syscall is introduced, which allows userspace > to set the memory policy (e.g. NUMA bindings) for a restrictedmem > file, to the granularity of offsets within the file. > > The offset/length tuple is termed a file_range which is passed to the > kernel via a pointer to get around the limit of 6 arguments for a > syscall. > > The following other approaches were also considered: > > 1. Pre-configuring a mount with a memory policy and providing that > mount to memfd_restricted() as proposed at [2]. > + Pro: It allows choice of a specific backing mount with custom > memory policy configurations > + Con: Will need to create an entire new mount just to set memory > policy for a restrictedmem file; files on the same mount cannot > have different memory policies. Could you expand on this some more please? How many restricted files/mounts do we expect? My understanding was that this would be essentially a backing store for guest memory so it would scale with the number of guests. > 2. Passing memory policy to the memfd_restricted() syscall at creation time. > + Pro: Only need to make a single syscall to create a file with a > given memory policy > + Con: At creation time, the kernel doesn’t know the size of the > restrictedmem file. Given that memory policy is stored in the > inode based on ranges (start, end), it is awkward for the kernel > to store the memory policy and then add hooks to set the memory > policy when allocation is done. > > 3. A more generic fbind(): it seems like this new functionality is > really only needed for restrictedmem files, hence a separate, > specific syscall was proposed to avoid complexities with handling > conflicting policies that may be specified via other syscalls like > mbind() I do not think it is a good idea to make the syscall restrict mem specific. History shows that users are much more creative when it comes to usecases than us. I do understand that the nature of restricted memory is that it is not mapable but memory policies without a mapping are a reasonable concept in genereal. After all this just tells where the memory should be allocated from. Do we need to implement that for any other fs? No, you can safely return EINVAL for anything but memfd_restricted fd for now but you shouldn't limit usecases upfront. > > TODOs How do you query a policy for the specific fd? Are there any plans to add a syscall for that as well but you just wait for the direction for the set method? > + Return -EINVAL if file_range is not within the size of the file and > tests for this > > Dependencies: > > + Chao’s work on UPM [3] > > [1] https://lore.kernel.org/lkml/20221202061347.1070246-1-chao.p.peng@linux.intel.com/T/ > [2] https://lore.kernel.org/lkml/cover.1681176340.git.ackerleytng@google.com/T/ > [3] https://github.com/chao-p/linux/commits/privmem-v11.5 > > --- > > Ackerley Tng (6): > mm: shmem: Refactor out shmem_shared_policy() function > mm: mempolicy: Refactor out mpol_init_from_nodemask > mm: mempolicy: Refactor out __mpol_set_shared_policy() > mm: mempolicy: Add and expose mpol_create > mm: restrictedmem: Add memfd_restricted_bind() syscall > selftests: mm: Add selftest for memfd_restricted_bind() > > arch/x86/entry/syscalls/syscall_32.tbl | 1 + > arch/x86/entry/syscalls/syscall_64.tbl | 1 + > include/linux/mempolicy.h | 4 + > include/linux/shmem_fs.h | 7 + > include/linux/syscalls.h | 5 + > include/uapi/asm-generic/unistd.h | 5 +- > include/uapi/linux/mempolicy.h | 7 +- > kernel/sys_ni.c | 1 + > mm/mempolicy.c | 100 ++++++++++--- > mm/restrictedmem.c | 75 ++++++++++ > mm/shmem.c | 10 +- > scripts/checksyscalls.sh | 1 + > tools/testing/selftests/mm/.gitignore | 1 + > tools/testing/selftests/mm/Makefile | 8 + > .../selftests/mm/memfd_restricted_bind.c | 139 ++++++++++++++++++ > .../mm/restrictedmem_testmod/Makefile | 21 +++ > .../restrictedmem_testmod.c | 89 +++++++++++ > tools/testing/selftests/mm/run_vmtests.sh | 6 + > 18 files changed, 454 insertions(+), 27 deletions(-) > create mode 100644 tools/testing/selftests/mm/memfd_restricted_bind.c > create mode 100644 tools/testing/selftests/mm/restrictedmem_testmod/Makefile > create mode 100644 tools/testing/selftests/mm/restrictedmem_testmod/restrictedmem_testmod.c > > -- > 2.40.0.634.g4ca3ef3211-goog
On Fri, Apr 14, 2023, Michal Hocko wrote: > On Fri 14-04-23 00:11:49, Ackerley Tng wrote: > > 3. A more generic fbind(): it seems like this new functionality is > > really only needed for restrictedmem files, hence a separate, > > specific syscall was proposed to avoid complexities with handling > > conflicting policies that may be specified via other syscalls like > > mbind() > > I do not think it is a good idea to make the syscall restrict mem > specific. +1. IMO, any uAPI that isn't directly related to the fundamental properties of restricted memory, i.e. isn't truly unique to restrictedmem, should be added as generic fd-based uAPI. > History shows that users are much more creative when it comes > to usecases than us. I do understand that the nature of restricted > memory is that it is not mapable but memory policies without a mapping > are a reasonable concept in genereal. After all this just tells where > the memory should be allocated from. Do we need to implement that for > any other fs? No, you can safely return EINVAL for anything but > memfd_restricted fd for now but you shouldn't limit usecases upfront. I would even go a step further and say that we should seriously reconsider the design/implemenation of memfd_restricted() if a generic fbind() needs explicit handling from the restricted memory code. One of the goals with memfd_restricted() is to rely on the underlying backing store to handle all of the "normal" behaviors.
Hello, This patchset builds upon the memfd_restricted() system call that was discussed in the 'KVM: mm: fd-based approach for supporting KVM' patch series [1]. The tree can be found at: https://github.com/googleprodkernel/linux-cc/tree/restrictedmem-set-memory-policy In this patchset, a new syscall is introduced, which allows userspace to set the memory policy (e.g. NUMA bindings) for a restrictedmem file, to the granularity of offsets within the file. The offset/length tuple is termed a file_range which is passed to the kernel via a pointer to get around the limit of 6 arguments for a syscall. The following other approaches were also considered: 1. Pre-configuring a mount with a memory policy and providing that mount to memfd_restricted() as proposed at [2]. + Pro: It allows choice of a specific backing mount with custom memory policy configurations + Con: Will need to create an entire new mount just to set memory policy for a restrictedmem file; files on the same mount cannot have different memory policies. 2. Passing memory policy to the memfd_restricted() syscall at creation time. + Pro: Only need to make a single syscall to create a file with a given memory policy + Con: At creation time, the kernel doesn’t know the size of the restrictedmem file. Given that memory policy is stored in the inode based on ranges (start, end), it is awkward for the kernel to store the memory policy and then add hooks to set the memory policy when allocation is done. 3. A more generic fbind(): it seems like this new functionality is really only needed for restrictedmem files, hence a separate, specific syscall was proposed to avoid complexities with handling conflicting policies that may be specified via other syscalls like mbind() TODOs + Return -EINVAL if file_range is not within the size of the file and tests for this Dependencies: + Chao’s work on UPM [3] [1] https://lore.kernel.org/lkml/20221202061347.1070246-1-chao.p.peng@linux.intel.com/T/ [2] https://lore.kernel.org/lkml/cover.1681176340.git.ackerleytng@google.com/T/ [3] https://github.com/chao-p/linux/commits/privmem-v11.5 --- Ackerley Tng (6): mm: shmem: Refactor out shmem_shared_policy() function mm: mempolicy: Refactor out mpol_init_from_nodemask mm: mempolicy: Refactor out __mpol_set_shared_policy() mm: mempolicy: Add and expose mpol_create mm: restrictedmem: Add memfd_restricted_bind() syscall selftests: mm: Add selftest for memfd_restricted_bind() arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/x86/entry/syscalls/syscall_64.tbl | 1 + include/linux/mempolicy.h | 4 + include/linux/shmem_fs.h | 7 + include/linux/syscalls.h | 5 + include/uapi/asm-generic/unistd.h | 5 +- include/uapi/linux/mempolicy.h | 7 +- kernel/sys_ni.c | 1 + mm/mempolicy.c | 100 ++++++++++--- mm/restrictedmem.c | 75 ++++++++++ mm/shmem.c | 10 +- scripts/checksyscalls.sh | 1 + tools/testing/selftests/mm/.gitignore | 1 + tools/testing/selftests/mm/Makefile | 8 + .../selftests/mm/memfd_restricted_bind.c | 139 ++++++++++++++++++ .../mm/restrictedmem_testmod/Makefile | 21 +++ .../restrictedmem_testmod.c | 89 +++++++++++ tools/testing/selftests/mm/run_vmtests.sh | 6 + 18 files changed, 454 insertions(+), 27 deletions(-) create mode 100644 tools/testing/selftests/mm/memfd_restricted_bind.c create mode 100644 tools/testing/selftests/mm/restrictedmem_testmod/Makefile create mode 100644 tools/testing/selftests/mm/restrictedmem_testmod/restrictedmem_testmod.c -- 2.40.0.634.g4ca3ef3211-goog