* Re: [PATCH v2 1/1] mm/madvise: add MADV_F_COLLAPSE_LIGHT to process_madvise()
From: Lance Yang @ 2024-01-19 1:46 UTC (permalink / raw)
To: Zach O'Keefe
Cc: Michal Hocko, akpm, david, songmuchun, shy828301, peterx,
mknyszek, minchan, linux-mm, linux-kernel, linux-api
In-Reply-To: <CAAa6QmTN2B-JAO=38A09hMtUp=srLiUfs=sDbck7Chkr=W-dCw@mail.gmail.com>
On Thu, Jan 18, 2024 at 10:59 PM Zach O'Keefe <zokeefe@google.com> wrote:
>
> On Thu, Jan 18, 2024 at 5:43 AM Michal Hocko <mhocko@suse.com> wrote:
> >
> > Dang, forgot to cc linux-api...
> >
> > On Thu 18-01-24 14:40:19, Michal Hocko wrote:
> > > On Thu 18-01-24 20:03:46, Lance Yang wrote:
> > > [...]
> > >
> > > before we discuss the semantic, let's focus on the usecase.
> > >
> > > > Use Cases
> > > >
> > > > An immediate user of this new functionality is the Go runtime heap allocator
> > > > that manages memory in hugepage-sized chunks. In the past, whether it was a
> > > > newly allocated chunk through mmap() or a reused chunk released by
> > > > madvise(MADV_DONTNEED), the allocator attempted to eagerly back memory with
> > > > huge pages using madvise(MADV_HUGEPAGE)[2] and madvise(MADV_COLLAPSE)[3]
> > > > respectively. However, both approaches resulted in performance issues; for
> > > > both scenarios, there could be entries into direct reclaim and/or compaction,
> > > > leading to unpredictable stalls[4]. Now, the allocator can confidently use
> > > > process_madvise(MADV_F_COLLAPSE_LIGHT) to attempt the allocation of huge pages.
>
> Aside: The thought was a MADV_F_COLLAPSE_LIGHT _flag_; so it'd be
> process_madvise(..., MADV_COLLAPSE, MADV_F_COLLAPSE_LIGHT)
I apologize for the misunderstanding. I will provide the correct implementation
in version 3.
BR,
Lance
>
> > > IIUC the primary reason is the cost of the huge page allocation which
> > > can be really high if the memory is heavily fragmented and it is called
> > > synchronously from the process directly, correct? Can that be worked
> > > around by process_madvise and performing the operation from a different
> > > context? Are there any other reasons to have a different mode?
> > >
> > > I mean I can think of a more relaxed (opportunistic) MADV_COLLAPSE -
> > > e.g. non blocking one to make sure that the caller doesn't really block
> > > on resource contention (be it locks or memory availability) because that
> > > matches our non-blocking interface in other areas but having a LIGHT
> > > operation sounds really vague and the exact semantic would be
> > > implementation specific and might change over time. Non-blocking has a
> > > clear semantic but it is not really clear whether that is what you
> > > really need/want.
>
> IIUC, usecase from Go is unbounded latency due to sync compaction in a
> context where the latency is unacceptable. Working w/ them to
> understand how things can be improved -- it's possible the changes can
> occur entirely on their side, w/o any additional kernel support.
>
> The non-blocking case awkwardly sits between MADV_COLLAPSE today, and
> khugepaged; esp when common case is that the allocation can probably
> be satisfied in fast path.
>
> The suggestion for something like "LIGHT" was intentionally vague
> because it could allow for other optimizations / changes down the
> line, as you point out. I think that might be a win, vs tying to a
> specific optimization (e.g. like a MADV_F_COLLAPSE_NODEFRAG). But I
> could be alone on that front, given the design of
> /sys/kernel/mm/transparent_hugepage.
>
> But circling back, I agree w/ you that the first order of business is to
> iron out a real usecase. As of right now, it's not clear something
> like this is required or helpful.
>
> Thanks,
> Zach
>
>
>
>
> > > > [1] https://github.com/torvalds/linux/commit/7d8faaf155454f8798ec56404faca29a82689c77
> > > > [2] https://github.com/golang/go/commit/8fa9e3beee8b0e6baa7333740996181268b60a3a
> > > > [3] https://github.com/golang/go/commit/9f9bb26880388c5bead158e9eca3be4b3a9bd2af
> > > > [4] https://github.com/golang/go/issues/63334
> > > >
> > > > [v1] https://lore.kernel.org/lkml/20240117050217.43610-1-ioworker0@gmail.com/
> > > --
> > > Michal Hocko
> > > SUSE Labs
> >
> > --
> > Michal Hocko
> > SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 1/1] mm/madvise: add MADV_F_COLLAPSE_LIGHT to process_madvise()
From: Yang Shi @ 2024-01-18 19:00 UTC (permalink / raw)
To: Zach O'Keefe
Cc: Michal Hocko, Lance Yang, akpm, david, songmuchun, peterx,
mknyszek, minchan, linux-mm, linux-kernel, linux-api
In-Reply-To: <CAAa6QmTN2B-JAO=38A09hMtUp=srLiUfs=sDbck7Chkr=W-dCw@mail.gmail.com>
On Thu, Jan 18, 2024 at 6:59 AM Zach O'Keefe <zokeefe@google.com> wrote:
>
> On Thu, Jan 18, 2024 at 5:43 AM Michal Hocko <mhocko@suse.com> wrote:
> >
> > Dang, forgot to cc linux-api...
> >
> > On Thu 18-01-24 14:40:19, Michal Hocko wrote:
> > > On Thu 18-01-24 20:03:46, Lance Yang wrote:
> > > [...]
> > >
> > > before we discuss the semantic, let's focus on the usecase.
> > >
> > > > Use Cases
> > > >
> > > > An immediate user of this new functionality is the Go runtime heap allocator
> > > > that manages memory in hugepage-sized chunks. In the past, whether it was a
> > > > newly allocated chunk through mmap() or a reused chunk released by
> > > > madvise(MADV_DONTNEED), the allocator attempted to eagerly back memory with
> > > > huge pages using madvise(MADV_HUGEPAGE)[2] and madvise(MADV_COLLAPSE)[3]
> > > > respectively. However, both approaches resulted in performance issues; for
> > > > both scenarios, there could be entries into direct reclaim and/or compaction,
> > > > leading to unpredictable stalls[4]. Now, the allocator can confidently use
> > > > process_madvise(MADV_F_COLLAPSE_LIGHT) to attempt the allocation of huge pages.
>
> Aside: The thought was a MADV_F_COLLAPSE_LIGHT _flag_; so it'd be
> process_madvise(..., MADV_COLLAPSE, MADV_F_COLLAPSE_LIGHT)
>
> > > IIUC the primary reason is the cost of the huge page allocation which
> > > can be really high if the memory is heavily fragmented and it is called
> > > synchronously from the process directly, correct? Can that be worked
> > > around by process_madvise and performing the operation from a different
> > > context? Are there any other reasons to have a different mode?
> > >
> > > I mean I can think of a more relaxed (opportunistic) MADV_COLLAPSE -
> > > e.g. non blocking one to make sure that the caller doesn't really block
> > > on resource contention (be it locks or memory availability) because that
> > > matches our non-blocking interface in other areas but having a LIGHT
> > > operation sounds really vague and the exact semantic would be
> > > implementation specific and might change over time. Non-blocking has a
> > > clear semantic but it is not really clear whether that is what you
> > > really need/want.
>
> IIUC, usecase from Go is unbounded latency due to sync compaction in a
> context where the latency is unacceptable. Working w/ them to
> understand how things can be improved -- it's possible the changes can
> occur entirely on their side, w/o any additional kernel support.
>
> The non-blocking case awkwardly sits between MADV_COLLAPSE today, and
> khugepaged; esp when common case is that the allocation can probably
> be satisfied in fast path.
>
> The suggestion for something like "LIGHT" was intentionally vague
> because it could allow for other optimizations / changes down the
> line, as you point out. I think that might be a win, vs tying to a
> specific optimization (e.g. like a MADV_F_COLLAPSE_NODEFRAG). But I
> could be alone on that front, given the design of
> /sys/kernel/mm/transparent_hugepage.
Per the description Go marks the address spaces with MADV_HUGEPAGE. It
means the application really wants to have huge page back the address
space so kernel will try as hard as possible to get huge page. This is
the default behavior of MADV_HUGEPAGE. If they don't want to enter
direct reclaim, they can configure the defrag mode to "defer", which
means no direct reclaim and wakeup kswapd and kcompactd, and rely on
khugepaged to install huge page later on. But this mode is not
supported by khugepaged defrag, so MADV_COLLAPSE may not support it
(IIRC MADV_COLLAPSE uses khugepaged defrag mode). Or they can just not
call MADV_HUGEPAGE and leave the decision to the users, IIRC Java does
so (specifying a flag to indicate use huge page or not by the users).
>
> But circling back, I agree w/ you that the first order of business is to
> iron out a real usecase. As of right now, it's not clear something
> like this is required or helpful.
>
> Thanks,
> Zach
>
>
>
>
> > > > [1] https://github.com/torvalds/linux/commit/7d8faaf155454f8798ec56404faca29a82689c77
> > > > [2] https://github.com/golang/go/commit/8fa9e3beee8b0e6baa7333740996181268b60a3a
> > > > [3] https://github.com/golang/go/commit/9f9bb26880388c5bead158e9eca3be4b3a9bd2af
> > > > [4] https://github.com/golang/go/issues/63334
> > > >
> > > > [v1] https://lore.kernel.org/lkml/20240117050217.43610-1-ioworker0@gmail.com/
> > > --
> > > Michal Hocko
> > > SUSE Labs
> >
> > --
> > Michal Hocko
> > SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] vfs: add RWF_NOAPPEND flag for pwritev2
From: Jens Axboe @ 2024-01-18 16:02 UTC (permalink / raw)
To: Rich Felker
Cc: Jann Horn, Alexander Viro, linux-fsdevel, kernel list, Linux API,
Pavel Begunkov, Christian Brauner
In-Reply-To: <20240118155735.GS22081@brightrain.aerifal.cx>
On 1/18/24 8:57 AM, Rich Felker wrote:
> On Mon, Aug 31, 2020 at 11:05:34AM -0600, Jens Axboe wrote:
>> On 8/31/20 9:46 AM, Jann Horn wrote:
>>> On Mon, Aug 31, 2020 at 5:32 PM Rich Felker <dalias@libc.org> wrote:
>>>> The pwrite function, originally defined by POSIX (thus the "p"), is
>>>> defined to ignore O_APPEND and write at the offset passed as its
>>>> argument. However, historically Linux honored O_APPEND if set and
>>>> ignored the offset. This cannot be changed due to stability policy,
>>>> but is documented in the man page as a bug.
>>>>
>>>> Now that there's a pwritev2 syscall providing a superset of the pwrite
>>>> functionality that has a flags argument, the conforming behavior can
>>>> be offered to userspace via a new flag. Since pwritev2 checks flag
>>>> validity (in kiocb_set_rw_flags) and reports unknown ones with
>>>> EOPNOTSUPP, callers will not get wrong behavior on old kernels that
>>>> don't support the new flag; the error is reported and the caller can
>>>> decide how to handle it.
>>>>
>>>> Signed-off-by: Rich Felker <dalias@libc.org>
>>>
>>> Reviewed-by: Jann Horn <jannh@google.com>
>>>
>>> Note that if this lands, Michael Kerrisk will probably be happy if you
>>> send a corresponding patch for the manpage man2/readv.2.
>>>
>>> Btw, I'm not really sure whose tree this should go through - VFS is
>>> normally Al Viro's turf, but it looks like the most recent
>>> modifications to this function have gone through Jens Axboe's tree?
>>
>> Should probably go through Al's tree, I've only carried them when
>> they've been associated with io_uring in some shape or form.
>
> This appears to have slipped through the cracks. Do I need to send an
> updated rebase of it? Were there any objections to it I missed?
Let's add Christian.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v2] vfs: add RWF_NOAPPEND flag for pwritev2
From: Rich Felker @ 2024-01-18 15:57 UTC (permalink / raw)
To: Jens Axboe
Cc: Jann Horn, Alexander Viro, linux-fsdevel, kernel list, Linux API,
Pavel Begunkov
In-Reply-To: <a9d26744-ba7a-2223-7284-c0d1a5ddab8a@kernel.dk>
On Mon, Aug 31, 2020 at 11:05:34AM -0600, Jens Axboe wrote:
> On 8/31/20 9:46 AM, Jann Horn wrote:
> > On Mon, Aug 31, 2020 at 5:32 PM Rich Felker <dalias@libc.org> wrote:
> >> The pwrite function, originally defined by POSIX (thus the "p"), is
> >> defined to ignore O_APPEND and write at the offset passed as its
> >> argument. However, historically Linux honored O_APPEND if set and
> >> ignored the offset. This cannot be changed due to stability policy,
> >> but is documented in the man page as a bug.
> >>
> >> Now that there's a pwritev2 syscall providing a superset of the pwrite
> >> functionality that has a flags argument, the conforming behavior can
> >> be offered to userspace via a new flag. Since pwritev2 checks flag
> >> validity (in kiocb_set_rw_flags) and reports unknown ones with
> >> EOPNOTSUPP, callers will not get wrong behavior on old kernels that
> >> don't support the new flag; the error is reported and the caller can
> >> decide how to handle it.
> >>
> >> Signed-off-by: Rich Felker <dalias@libc.org>
> >
> > Reviewed-by: Jann Horn <jannh@google.com>
> >
> > Note that if this lands, Michael Kerrisk will probably be happy if you
> > send a corresponding patch for the manpage man2/readv.2.
> >
> > Btw, I'm not really sure whose tree this should go through - VFS is
> > normally Al Viro's turf, but it looks like the most recent
> > modifications to this function have gone through Jens Axboe's tree?
>
> Should probably go through Al's tree, I've only carried them when
> they've been associated with io_uring in some shape or form.
This appears to have slipped through the cracks. Do I need to send an
updated rebase of it? Were there any objections to it I missed?
Rich
^ permalink raw reply
* Re: [PATCH v2 1/1] mm/madvise: add MADV_F_COLLAPSE_LIGHT to process_madvise()
From: Zach O'Keefe @ 2024-01-18 14:58 UTC (permalink / raw)
To: Michal Hocko
Cc: Lance Yang, akpm, david, songmuchun, shy828301, peterx, mknyszek,
minchan, linux-mm, linux-kernel, linux-api
In-Reply-To: <Zakq-54DFdPu0c2U@tiehlicka>
On Thu, Jan 18, 2024 at 5:43 AM Michal Hocko <mhocko@suse.com> wrote:
>
> Dang, forgot to cc linux-api...
>
> On Thu 18-01-24 14:40:19, Michal Hocko wrote:
> > On Thu 18-01-24 20:03:46, Lance Yang wrote:
> > [...]
> >
> > before we discuss the semantic, let's focus on the usecase.
> >
> > > Use Cases
> > >
> > > An immediate user of this new functionality is the Go runtime heap allocator
> > > that manages memory in hugepage-sized chunks. In the past, whether it was a
> > > newly allocated chunk through mmap() or a reused chunk released by
> > > madvise(MADV_DONTNEED), the allocator attempted to eagerly back memory with
> > > huge pages using madvise(MADV_HUGEPAGE)[2] and madvise(MADV_COLLAPSE)[3]
> > > respectively. However, both approaches resulted in performance issues; for
> > > both scenarios, there could be entries into direct reclaim and/or compaction,
> > > leading to unpredictable stalls[4]. Now, the allocator can confidently use
> > > process_madvise(MADV_F_COLLAPSE_LIGHT) to attempt the allocation of huge pages.
Aside: The thought was a MADV_F_COLLAPSE_LIGHT _flag_; so it'd be
process_madvise(..., MADV_COLLAPSE, MADV_F_COLLAPSE_LIGHT)
> > IIUC the primary reason is the cost of the huge page allocation which
> > can be really high if the memory is heavily fragmented and it is called
> > synchronously from the process directly, correct? Can that be worked
> > around by process_madvise and performing the operation from a different
> > context? Are there any other reasons to have a different mode?
> >
> > I mean I can think of a more relaxed (opportunistic) MADV_COLLAPSE -
> > e.g. non blocking one to make sure that the caller doesn't really block
> > on resource contention (be it locks or memory availability) because that
> > matches our non-blocking interface in other areas but having a LIGHT
> > operation sounds really vague and the exact semantic would be
> > implementation specific and might change over time. Non-blocking has a
> > clear semantic but it is not really clear whether that is what you
> > really need/want.
IIUC, usecase from Go is unbounded latency due to sync compaction in a
context where the latency is unacceptable. Working w/ them to
understand how things can be improved -- it's possible the changes can
occur entirely on their side, w/o any additional kernel support.
The non-blocking case awkwardly sits between MADV_COLLAPSE today, and
khugepaged; esp when common case is that the allocation can probably
be satisfied in fast path.
The suggestion for something like "LIGHT" was intentionally vague
because it could allow for other optimizations / changes down the
line, as you point out. I think that might be a win, vs tying to a
specific optimization (e.g. like a MADV_F_COLLAPSE_NODEFRAG). But I
could be alone on that front, given the design of
/sys/kernel/mm/transparent_hugepage.
But circling back, I agree w/ you that the first order of business is to
iron out a real usecase. As of right now, it's not clear something
like this is required or helpful.
Thanks,
Zach
> > > [1] https://github.com/torvalds/linux/commit/7d8faaf155454f8798ec56404faca29a82689c77
> > > [2] https://github.com/golang/go/commit/8fa9e3beee8b0e6baa7333740996181268b60a3a
> > > [3] https://github.com/golang/go/commit/9f9bb26880388c5bead158e9eca3be4b3a9bd2af
> > > [4] https://github.com/golang/go/issues/63334
> > >
> > > [v1] https://lore.kernel.org/lkml/20240117050217.43610-1-ioworker0@gmail.com/
> > --
> > Michal Hocko
> > SUSE Labs
>
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply
* Re: [PATCH v4] selftests/move_mount_set_group:Make tests build with old libc
From: Christian Brauner @ 2024-01-18 14:37 UTC (permalink / raw)
To: Hu Yadi
Cc: Christian Brauner, linux-api, linux-kernel, linux-security-module,
linux-kselftest, 514118380, jmorris, serge, shuah,
mathieu.desnoyers, mic
In-Reply-To: <20240111113229.10820-1-hu.yadi@h3c.com>
On Thu, 11 Jan 2024 19:32:29 +0800, Hu Yadi wrote:
> Replace SYS_<syscall> with __NR_<syscall>. Using the __NR_<syscall>
> notation, provided by UAPI, is useful to build tests on systems without
> the SYS_<syscall> definitions.
>
> Replace SYS_move_mount with __NR_move_mount
>
> Similar changes: commit 87129ef13603 ("selftests/landlock: Make tests build with old libc")
>
> [...]
Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc
[1/1] selftests/move_mount_set_group:Make tests build with old libc
https://git.kernel.org/vfs/vfs/c/0778b0a1a8d2
^ permalink raw reply
* Re: [PATCH v2 1/1] mm/madvise: add MADV_F_COLLAPSE_LIGHT to process_madvise()
From: Michal Hocko @ 2024-01-18 13:43 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, zokeefe, david, songmuchun, shy828301, peterx, mknyszek,
minchan, linux-mm, linux-kernel, linux-api
In-Reply-To: <ZakqQyL9t2ffNUIf@tiehlicka>
Dang, forgot to cc linux-api...
On Thu 18-01-24 14:40:19, Michal Hocko wrote:
> On Thu 18-01-24 20:03:46, Lance Yang wrote:
> [...]
>
> before we discuss the semantic, let's focus on the usecase.
>
> > Use Cases
> >
> > An immediate user of this new functionality is the Go runtime heap allocator
> > that manages memory in hugepage-sized chunks. In the past, whether it was a
> > newly allocated chunk through mmap() or a reused chunk released by
> > madvise(MADV_DONTNEED), the allocator attempted to eagerly back memory with
> > huge pages using madvise(MADV_HUGEPAGE)[2] and madvise(MADV_COLLAPSE)[3]
> > respectively. However, both approaches resulted in performance issues; for
> > both scenarios, there could be entries into direct reclaim and/or compaction,
> > leading to unpredictable stalls[4]. Now, the allocator can confidently use
> > process_madvise(MADV_F_COLLAPSE_LIGHT) to attempt the allocation of huge pages.
>
> IIUC the primary reason is the cost of the huge page allocation which
> can be really high if the memory is heavily fragmented and it is called
> synchronously from the process directly, correct? Can that be worked
> around by process_madvise and performing the operation from a different
> context? Are there any other reasons to have a different mode?
>
> I mean I can think of a more relaxed (opportunistic) MADV_COLLAPSE -
> e.g. non blocking one to make sure that the caller doesn't really block
> on resource contention (be it locks or memory availability) because that
> matches our non-blocking interface in other areas but having a LIGHT
> operation sounds really vague and the exact semantic would be
> implementation specific and might change over time. Non-blocking has a
> clear semantic but it is not really clear whether that is what you
> really need/want.
>
> > [1] https://github.com/torvalds/linux/commit/7d8faaf155454f8798ec56404faca29a82689c77
> > [2] https://github.com/golang/go/commit/8fa9e3beee8b0e6baa7333740996181268b60a3a
> > [3] https://github.com/golang/go/commit/9f9bb26880388c5bead158e9eca3be4b3a9bd2af
> > [4] https://github.com/golang/go/issues/63334
> >
> > [v1] https://lore.kernel.org/lkml/20240117050217.43610-1-ioworker0@gmail.com/
> --
> Michal Hocko
> SUSE Labs
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 1/1] mm/madvise: add MADV_F_COLLAPSE_LIGHT to process_madvise()
From: Michal Hocko @ 2024-01-18 13:28 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, zokeefe, david, songmuchun, shy828301, peterx, mknyszek,
minchan, linux-mm, linux-kernel, linux-api
In-Reply-To: <20240118120347.61817-1-ioworker0@gmail.com>
[CC linux-api]
On Thu 18-01-24 20:03:46, Lance Yang wrote:
> This idea was inspired by MADV_COLLAPSE introduced by Zach O'Keefe[1].
>
> Allow MADV_F_COLLAPSE_LIGHT behavior for process_madvise(2) if the caller
> has CAP_SYS_ADMIN or is requesting the collapse of its own memory.
>
> The semantics of MADV_F_COLLAPSE_LIGHT are similar to MADV_COLLAPSE, but
> it avoids direct reclaim and/or compaction, quickly failing on allocation
> errors.
>
> This change enables a more flexible and efficient usage of memory collapse
> operations, providing additional control to userspace applications for
> system-wide THP optimization.
>
> Semantics
>
> This call is independent of the system-wide THP sysfs settings, but will
> fail for memory marked VM_NOHUGEPAGE. If the ranges provided span
> multiple VMAs, the semantics of the collapse over each VMA is independent
> from the others. This implies a hugepage cannot cross a VMA boundary. If
> collapse of a given hugepage-aligned/sized region fails, the operation may
> continue to attempt collapsing the remainder of memory specified.
>
> The memory ranges provided must be page-aligned, but are not required to
> be hugepage-aligned. If the memory ranges are not hugepage-aligned, the
> start/end of the range will be clamped to the first/last hugepage-aligned
> address covered by said range. The memory ranges must span at least one
> hugepage-sized region.
>
> All non-resident pages covered by the range will first be
> swapped/faulted-in, before being internally copied onto a freshly
> allocated hugepage. Unmapped pages will have their data directly
> initialized to 0 in the new hugepage. However, for every eligible
> hugepage aligned/sized region to-be collapsed, at least one page must
> currently be backed by memory (a PMD covering the address range must
> already exist).
>
> Allocation for the new hugepage will not enter direct reclaim and/or
> compaction, quickly failing if allocation fails. When the system has
> multiple NUMA nodes, the hugepage will be allocated from the node providing
> the most native pages. This operation operates on the current state of the
> specified process and makes no persistent changes or guarantees on how pages
> will be mapped, constructed, or faulted in the future.
>
> Use Cases
>
> An immediate user of this new functionality is the Go runtime heap allocator
> that manages memory in hugepage-sized chunks. In the past, whether it was a
> newly allocated chunk through mmap() or a reused chunk released by
> madvise(MADV_DONTNEED), the allocator attempted to eagerly back memory with
> huge pages using madvise(MADV_HUGEPAGE)[2] and madvise(MADV_COLLAPSE)[3]
> respectively. However, both approaches resulted in performance issues; for
> both scenarios, there could be entries into direct reclaim and/or compaction,
> leading to unpredictable stalls[4]. Now, the allocator can confidently use
> process_madvise(MADV_F_COLLAPSE_LIGHT) to attempt the allocation of huge pages.
>
> [1] https://github.com/torvalds/linux/commit/7d8faaf155454f8798ec56404faca29a82689c77
> [2] https://github.com/golang/go/commit/8fa9e3beee8b0e6baa7333740996181268b60a3a
> [3] https://github.com/golang/go/commit/9f9bb26880388c5bead158e9eca3be4b3a9bd2af
> [4] https://github.com/golang/go/issues/63334
>
> [v1] https://lore.kernel.org/lkml/20240117050217.43610-1-ioworker0@gmail.com/
>
> Signed-off-by: Lance Yang <ioworker0@gmail.com>
> Suggested-by: Zach O'Keefe <zokeefe@google.com>
> Suggested-by: David Hildenbrand <david@redhat.com>
> ---
> V1 -> V2: Treat process_madvise(MADV_F_COLLAPSE_LIGHT) as the lighter-weight alternative
> to madvise(MADV_COLLAPSE)
>
> arch/alpha/include/uapi/asm/mman.h | 1 +
> arch/mips/include/uapi/asm/mman.h | 1 +
> arch/parisc/include/uapi/asm/mman.h | 1 +
> arch/xtensa/include/uapi/asm/mman.h | 1 +
> include/linux/huge_mm.h | 5 +--
> include/uapi/asm-generic/mman-common.h | 1 +
> mm/khugepaged.c | 15 ++++++--
> mm/madvise.c | 36 +++++++++++++++++---
> tools/include/uapi/asm-generic/mman-common.h | 1 +
> 9 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
> index 763929e814e9..22f23ca04f1a 100644
> --- a/arch/alpha/include/uapi/asm/mman.h
> +++ b/arch/alpha/include/uapi/asm/mman.h
> @@ -77,6 +77,7 @@
> #define MADV_DONTNEED_LOCKED 24 /* like DONTNEED, but drop locked pages too */
>
> #define MADV_COLLAPSE 25 /* Synchronous hugepage collapse */
> +#define MADV_F_COLLAPSE_LIGHT 26 /* Similar to COLLAPSE, but avoids direct reclaim and/or compaction */
>
> /* compatibility flags */
> #define MAP_FILE 0
> diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
> index c6e1fc77c996..acec0b643e9c 100644
> --- a/arch/mips/include/uapi/asm/mman.h
> +++ b/arch/mips/include/uapi/asm/mman.h
> @@ -104,6 +104,7 @@
> #define MADV_DONTNEED_LOCKED 24 /* like DONTNEED, but drop locked pages too */
>
> #define MADV_COLLAPSE 25 /* Synchronous hugepage collapse */
> +#define MADV_F_COLLAPSE_LIGHT 26 /* Similar to COLLAPSE, but avoids direct reclaim and/or compaction */
>
> /* compatibility flags */
> #define MAP_FILE 0
> diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
> index 68c44f99bc93..812029c98cd7 100644
> --- a/arch/parisc/include/uapi/asm/mman.h
> +++ b/arch/parisc/include/uapi/asm/mman.h
> @@ -71,6 +71,7 @@
> #define MADV_DONTNEED_LOCKED 24 /* like DONTNEED, but drop locked pages too */
>
> #define MADV_COLLAPSE 25 /* Synchronous hugepage collapse */
> +#define MADV_F_COLLAPSE_LIGHT 26 /* Similar to COLLAPSE, but avoids direct reclaim and/or compaction */
>
> #define MADV_HWPOISON 100 /* poison a page for testing */
> #define MADV_SOFT_OFFLINE 101 /* soft offline page for testing */
> diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
> index 1ff0c858544f..52ef463dd5b6 100644
> --- a/arch/xtensa/include/uapi/asm/mman.h
> +++ b/arch/xtensa/include/uapi/asm/mman.h
> @@ -112,6 +112,7 @@
> #define MADV_DONTNEED_LOCKED 24 /* like DONTNEED, but drop locked pages too */
>
> #define MADV_COLLAPSE 25 /* Synchronous hugepage collapse */
> +#define MADV_F_COLLAPSE_LIGHT 26 /* Similar to COLLAPSE, but avoids direct reclaim and/or compaction */
>
> /* compatibility flags */
> #define MAP_FILE 0
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 5adb86af35fc..075fdb5d481a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -303,7 +303,7 @@ int hugepage_madvise(struct vm_area_struct *vma, unsigned long *vm_flags,
> int advice);
> int madvise_collapse(struct vm_area_struct *vma,
> struct vm_area_struct **prev,
> - unsigned long start, unsigned long end);
> + unsigned long start, unsigned long end, int behavior);
> void vma_adjust_trans_huge(struct vm_area_struct *vma, unsigned long start,
> unsigned long end, long adjust_next);
> spinlock_t *__pmd_trans_huge_lock(pmd_t *pmd, struct vm_area_struct *vma);
> @@ -450,7 +450,8 @@ static inline int hugepage_madvise(struct vm_area_struct *vma,
>
> static inline int madvise_collapse(struct vm_area_struct *vma,
> struct vm_area_struct **prev,
> - unsigned long start, unsigned long end)
> + unsigned long start, unsigned long end,
> + int behavior)
> {
> return -EINVAL;
> }
> diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
> index 6ce1f1ceb432..92c67bc755da 100644
> --- a/include/uapi/asm-generic/mman-common.h
> +++ b/include/uapi/asm-generic/mman-common.h
> @@ -78,6 +78,7 @@
> #define MADV_DONTNEED_LOCKED 24 /* like DONTNEED, but drop locked pages too */
>
> #define MADV_COLLAPSE 25 /* Synchronous hugepage collapse */
> +#define MADV_F_COLLAPSE_LIGHT 26 /* Similar to COLLAPSE, but avoids direct reclaim and/or compaction */
>
> /* compatibility flags */
> #define MAP_FILE 0
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 2b219acb528e..2840051c0ae2 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -97,6 +97,8 @@ static struct kmem_cache *mm_slot_cache __ro_after_init;
> struct collapse_control {
> bool is_khugepaged;
>
> + int behavior;
> +
> /* Num pages scanned per node */
> u32 node_load[MAX_NUMNODES];
>
> @@ -1058,10 +1060,16 @@ static int __collapse_huge_page_swapin(struct mm_struct *mm,
> static int alloc_charge_hpage(struct page **hpage, struct mm_struct *mm,
> struct collapse_control *cc)
> {
> - gfp_t gfp = (cc->is_khugepaged ? alloc_hugepage_khugepaged_gfpmask() :
> - GFP_TRANSHUGE);
> int node = hpage_collapse_find_target_node(cc);
> struct folio *folio;
> + gfp_t gfp;
> +
> + if (cc->is_khugepaged)
> + gfp = alloc_hugepage_khugepaged_gfpmask();
> + else
> + gfp = (cc->behavior == MADV_F_COLLAPSE_LIGHT ?
> + GFP_TRANSHUGE_LIGHT :
> + GFP_TRANSHUGE);
>
> if (!hpage_collapse_alloc_folio(&folio, gfp, node, &cc->alloc_nmask)) {
> *hpage = NULL;
> @@ -2697,7 +2705,7 @@ static int madvise_collapse_errno(enum scan_result r)
> }
>
> int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev,
> - unsigned long start, unsigned long end)
> + unsigned long start, unsigned long end, int behavior)
> {
> struct collapse_control *cc;
> struct mm_struct *mm = vma->vm_mm;
> @@ -2718,6 +2726,7 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev,
> if (!cc)
> return -ENOMEM;
> cc->is_khugepaged = false;
> + cc->behavior = behavior;
>
> mmgrab(mm);
> lru_add_drain_all();
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 912155a94ed5..9c40226505aa 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -60,6 +60,7 @@ static int madvise_need_mmap_write(int behavior)
> case MADV_POPULATE_READ:
> case MADV_POPULATE_WRITE:
> case MADV_COLLAPSE:
> + case MADV_F_COLLAPSE_LIGHT:
> return 0;
> default:
> /* be safe, default to 1. list exceptions explicitly */
> @@ -1082,8 +1083,9 @@ static int madvise_vma_behavior(struct vm_area_struct *vma,
> if (error)
> goto out;
> break;
> + case MADV_F_COLLAPSE_LIGHT:
> case MADV_COLLAPSE:
> - return madvise_collapse(vma, prev, start, end);
> + return madvise_collapse(vma, prev, start, end, behavior);
> }
>
> anon_name = anon_vma_name(vma);
> @@ -1178,6 +1180,7 @@ madvise_behavior_valid(int behavior)
> case MADV_HUGEPAGE:
> case MADV_NOHUGEPAGE:
> case MADV_COLLAPSE:
> + case MADV_F_COLLAPSE_LIGHT:
> #endif
> case MADV_DONTDUMP:
> case MADV_DODUMP:
> @@ -1194,6 +1197,17 @@ madvise_behavior_valid(int behavior)
> }
> }
>
> +
> +static bool process_madvise_behavior_only(int behavior)
> +{
> + switch (behavior) {
> + case MADV_F_COLLAPSE_LIGHT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static bool process_madvise_behavior_valid(int behavior)
> {
> switch (behavior) {
> @@ -1201,6 +1215,7 @@ static bool process_madvise_behavior_valid(int behavior)
> case MADV_PAGEOUT:
> case MADV_WILLNEED:
> case MADV_COLLAPSE:
> + case MADV_F_COLLAPSE_LIGHT:
> return true;
> default:
> return false;
> @@ -1368,6 +1383,8 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start,
> * transparent huge pages so the existing pages will not be
> * coalesced into THP and new pages will not be allocated as THP.
> * MADV_COLLAPSE - synchronously coalesce pages into new THP.
> + * MADV_F_COLLAPSE_LIGHT - only for process_madvise, avoids direct reclaim and/or
> + * compaction.
> * MADV_DONTDUMP - the application wants to prevent pages in the given range
> * from being included in its core dump.
> * MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
> @@ -1394,7 +1411,8 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start,
> * -EBADF - map exists, but area maps something that isn't a file.
> * -EAGAIN - a kernel resource was temporarily unavailable.
> */
> -int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int behavior)
> +int _do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in,
> + int behavior, bool is_process_madvise)
> {
> unsigned long end;
> int error;
> @@ -1405,6 +1423,9 @@ int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int beh
> if (!madvise_behavior_valid(behavior))
> return -EINVAL;
>
> + if (!is_process_madvise && process_madvise_behavior_only(behavior))
> + return -EINVAL;
> +
> if (!PAGE_ALIGNED(start))
> return -EINVAL;
> len = PAGE_ALIGN(len_in);
> @@ -1448,9 +1469,14 @@ int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int beh
> return error;
> }
>
> +int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int behavior)
> +{
> + return _do_madvise(mm, start, len_in, behavior, false);
> +}
> +
> SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
> {
> - return do_madvise(current->mm, start, len_in, behavior);
> + return _do_madvise(current->mm, start, len_in, behavior, false);
> }
>
> SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
> @@ -1504,8 +1530,8 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
> total_len = iov_iter_count(&iter);
>
> while (iov_iter_count(&iter)) {
> - ret = do_madvise(mm, (unsigned long)iter_iov_addr(&iter),
> - iter_iov_len(&iter), behavior);
> + ret = _do_madvise(mm, (unsigned long)iter_iov_addr(&iter),
> + iter_iov_len(&iter), behavior, true);
> if (ret < 0)
> break;
> iov_iter_advance(&iter, iter_iov_len(&iter));
> diff --git a/tools/include/uapi/asm-generic/mman-common.h b/tools/include/uapi/asm-generic/mman-common.h
> index 6ce1f1ceb432..92c67bc755da 100644
> --- a/tools/include/uapi/asm-generic/mman-common.h
> +++ b/tools/include/uapi/asm-generic/mman-common.h
> @@ -78,6 +78,7 @@
> #define MADV_DONTNEED_LOCKED 24 /* like DONTNEED, but drop locked pages too */
>
> #define MADV_COLLAPSE 25 /* Synchronous hugepage collapse */
> +#define MADV_F_COLLAPSE_LIGHT 26 /* Similar to COLLAPSE, but avoids direct reclaim and/or compaction */
>
> /* compatibility flags */
> #define MAP_FILE 0
> --
> 2.33.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/3] mm/mempolicy: implement the sysfs-based weighted_interleave interface
From: Huang, Ying @ 2024-01-18 4:37 UTC (permalink / raw)
To: Gregory Price
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <ZagSW5TXzZeKErlW@memverge.com>
Gregory Price <gregory.price@memverge.com> writes:
> On Wed, Jan 17, 2024 at 02:58:08PM +0800, Huang, Ying wrote:
>> Gregory Price <gregory.price@memverge.com> writes:
>>
>> > We haven't had the discussion on how/when this should happen yet,
>> > though, and there's some research to be done. (i.e. when should DRAM
>> > weights be set? should the entire table be reweighted on hotplug? etc)
>>
>> Before that, I'm OK to remove default_iw_table and use hard coded "1" as
>> default weight for now.
>>
>
> Can't quite do that. default_iw_table is a static structure because we
> need a reliable default structure not subject to module initialization
> failure. Otherwise we can end up in a situation where iw_table is NULL
> during some allocation path if the sysfs structure fails to setup fully.
As the first simplest implementation, we can avoid default_iw_table[].
Becuse it's constant.
> There's no good reason to fail allocations just because sysfs failed to
> initialization for some reason. I'll leave default_iw_table with a size
> of MAX_NUMNODES for now (nr_node_ids is set up at runtime per your
> reference to `setup_nr_node_ids` below, so we can't use it for this).
We allocate memory during module initialization all over the places in
kernel. I don't think it will cause any issue in practice. Just some
additional checking for "default_iw_table == NULL".
And, we cannot make it just static, because we need to use RCU to keep
it consistent. Otherwise, it may be changed during reading.
>> >
>> >> u8 __rcu *iw_table;
>> >>
>> >> Then, we only need to allocate nr_node_ids elements now.
>> >>
>> >
>> > We need nr_possible_nodes to handle hotplug correctly.
>>
>> nr_node_ids >= num_possible_nodes(). It's larger than any possible node
>> ID.
>>
>
> nr_node_ids gets setup at runtime, while the default_iw_table needs
> to be a static structure (see above). I can make default_iw_table
> MAX_NUMNODES and subsequent allocations of iw_table be nr_node_ids,
> but that makes iw_table a different size at any given time.
>
> This *will* break if "true hotplug" ever shows up and possible_nodes !=
> MAX_NUMNODES. But I can write it up if it's a sticking point for you.
I don't think it is an issue for "true hotplug". Because we can set
nr_node_ids = MAX_NUMNODES even if there is something called "true
hotplug".
> Ultimately we're squabbling over, at most, about ~3kb of memory, just
> keep that in mind. (I guess if you spawn 3000 threads and each tries a
> concurrent write to sysfs/node1, you'd eat 3MB view briefly, but that
> is a truly degenerate case and I can think of more denegerate things).
Not just for memory wastage, it's about proper API too.
>>
>> When "true node hotplug" becomes reality, we can make nr_node_ids ==
>> MAX_NUMNODES. So, it's safe to use it. Please take a look at
>> setup_nr_node_ids().
>>
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-01-18 4:06 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <87fryvz6gf.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Thu, Jan 18, 2024 at 11:05:52AM +0800, Huang, Ying wrote:
> Gregory Price <gourry.memverge@gmail.com> writes:
> > +static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> > + struct mempolicy *pol, unsigned long nr_pages,
> > + struct page **page_array)
> > +{
> > + struct task_struct *me = current;
> > + unsigned long total_allocated = 0;
> > + unsigned long nr_allocated;
> > + unsigned long rounds;
> > + unsigned long node_pages, delta;
> > + u8 weight;
> > + struct iw_table __rcu *table;
> > + u8 *weights;
> > + unsigned int weight_total = 0;
> > + unsigned long rem_pages = nr_pages;
> > + nodemask_t nodes;
> > + int nnodes, node, weight_nodes;
> > + int prev_node = NUMA_NO_NODE;
> > + int i;
> > +
> > + nnodes = read_once_policy_nodemask(pol, &nodes);
> > + if (!nnodes)
> > + return 0;
> > +
> > + /* Continue allocating from most recent node and adjust the nr_pages */
> > + if (pol->wil.cur_weight) {
> > + node = next_node_in(me->il_prev, nodes);
> > + node_pages = pol->wil.cur_weight;
> > + if (node_pages > rem_pages)
> > + node_pages = rem_pages;
> > + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> > + NULL, page_array);
> > + page_array += nr_allocated;
> > + total_allocated += nr_allocated;
> > + /* if that's all the pages, no need to interleave */
> > + if (rem_pages <= pol->wil.cur_weight) {
> > + pol->wil.cur_weight -= rem_pages;
> > + return total_allocated;
> > + }
> > + /* Otherwise we adjust nr_pages down, and continue from there */
> > + rem_pages -= pol->wil.cur_weight;
> > + pol->wil.cur_weight = 0;
> > + prev_node = node;
> > + }
> > +
> > + /* fetch the weights for this operation and calculate total weight */
> > + weights = kmalloc(nnodes, GFP_KERNEL);
> > + if (!weights)
> > + return total_allocated;
> > +
> > + rcu_read_lock();
> > + table = rcu_dereference(iw_table);
> > + weight_nodes = 0;
> > + for_each_node_mask(node, nodes) {
> > + weights[weight_nodes++] = table->weights[node];
> > + weight_total += table->weights[node];
> > + }
> > + rcu_read_unlock();
> > +
> > + if (!weight_total) {
> > + kfree(weights);
> > + return total_allocated;
> > + }
> > +
> > + /* Now we can continue allocating as if from 0 instead of an offset */
> > + rounds = rem_pages / weight_total;
> > + delta = rem_pages % weight_total;
> > + for (i = 0; i < nnodes; i++) {
> > + node = next_node_in(prev_node, nodes);
> > + weight = weights[i];
> > + node_pages = weight * rounds;
> > + if (delta) {
> > + if (delta > weight) {
> > + node_pages += weight;
> > + delta -= weight;
> > + } else {
> > + node_pages += delta;
> > + delta = 0;
> > + }
> > + }
> > + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> > + NULL, page_array);
> > + page_array += nr_allocated;
> > + total_allocated += nr_allocated;
> > + if (total_allocated == nr_pages)
> > + break;
> > + prev_node = node;
> > + }
> > +
> > + /*
> > + * Finally, we need to update me->il_prev and pol->wil.cur_weight
> > + * if there were overflow pages, but not equivalent to the node
> > + * weight, set the cur_weight to node_weight - delta and the
> > + * me->il_prev to the previous node. Otherwise if it was perfect
> > + * we can simply set il_prev to node and cur_weight to 0
> > + */
> > + if (node_pages) {
> > + me->il_prev = prev_node;
> > + node_pages %= weight;
> > + pol->wil.cur_weight = weight - node_pages;
> > + } else {
> > + me->il_prev = node;
> > + pol->wil.cur_weight = 0;
> > + }
>
>
> It appears that we should set me->il_prev and pol->wil.cur_weight when
> delta becomes 0? That is, following allocation should start from there?
>
So the observation is that when delta reaches 0, we know what the prior
node should be. The only corner case being that delta is 0 when we
enter the loop (in which case current prev_node is the correct
prev_node).
Eyeballing it, this seems correct, but I'll do some additional
validation tomorrow. That should clean up the last block a bit.
Thanks!
~Gregory
^ permalink raw reply
* Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Huang, Ying @ 2024-01-18 3:05 UTC (permalink / raw)
To: Gregory Price
Cc: linux-mm, linux-kernel, linux-doc, linux-fsdevel, linux-api,
corbet, akpm, gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <20240112210834.8035-4-gregory.price@memverge.com>
Gregory Price <gourry.memverge@gmail.com> writes:
> When a system has multiple NUMA nodes and it becomes bandwidth hungry,
> using the current MPOL_INTERLEAVE could be an wise option.
>
> However, if those NUMA nodes consist of different types of memory such
> as socket-attached DRAM and CXL/PCIe attached DRAM, the round-robin
> based interleave policy does not optimally distribute data to make use
> of their different bandwidth characteristics.
>
> Instead, interleave is more effective when the allocation policy follows
> each NUMA nodes' bandwidth weight rather than a simple 1:1 distribution.
>
> This patch introduces a new memory policy, MPOL_WEIGHTED_INTERLEAVE,
> enabling weighted interleave between NUMA nodes. Weighted interleave
> allows for proportional distribution of memory across multiple numa
> nodes, preferably apportioned to match the bandwidth of each node.
>
> For example, if a system has 1 CPU node (0), and 2 memory nodes (0,1),
> with bandwidth of (100GB/s, 50GB/s) respectively, the appropriate
> weight distribution is (2:1).
>
> Weights for each node can be assigned via the new sysfs extension:
> /sys/kernel/mm/mempolicy/weighted_interleave/
>
> In addition, the `default_iw_table` is created, which will be extended
> in the future to allow defaults to be registered by drivers. For now,
> the default value of all nodes will be `1`, which matches the behavior
> of standard 1:1 round-robin interleave.
>
> The policy allocates a number of pages equal to the set weights. For
> example, if the weights are (2,1), then 2 pages will be allocated on
> node0 for every 1 page allocated on node1.
>
> The new flag MPOL_WEIGHTED_INTERLEAVE can be used in set_mempolicy(2)
> and mbind(2).
>
> There are 3 integration points:
>
> weighted_interleave_nodes:
> Counts the number of allocations as they occur, and applies the
> weight for the current node. When the weight reaches 0, switch
> to the next node.
>
> weighted_interleave_nid:
> Gets the total weight of the nodemask as well as each individual
> node weight, then calculates the node based on the given index.
>
> bulk_array_weighted_interleave:
> Gets the total weight of the nodemask as well as each individual
> node weight, then calculates the number of "interleave rounds" as
> well as any delta ("partial round"). Calculates the number of
> pages for each node and allocates them.
>
> If a node was scheduled for interleave via interleave_nodes, the
> current weight (pol->cur_weight) will be allocated first, before
> the remaining bulk calculation is done.
>
> One piece of complexity is the interaction between a recent refactor
> which split the logic to acquire the "ilx" (interleave index) of an
> allocation and the actually application of the interleave. The
> calculation of the `interleave index` is done by `get_vma_policy()`,
> while the actual selection of the node will be later appliex by the
> relevant weighted_interleave function.
>
> Suggested-by: Hasan Al Maruf <Hasan.Maruf@amd.com>
> Signed-off-by: Gregory Price <gregory.price@memverge.com>
> Co-developed-by: Rakie Kim <rakie.kim@sk.com>
> Signed-off-by: Rakie Kim <rakie.kim@sk.com>
> Co-developed-by: Honggyu Kim <honggyu.kim@sk.com>
> Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
> Co-developed-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
> Signed-off-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
> Co-developed-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> Co-developed-by: Ravi Jonnalagadda <ravis.opensrc@micron.com>
> Signed-off-by: Ravi Jonnalagadda <ravis.opensrc@micron.com>
> ---
> .../admin-guide/mm/numa_memory_policy.rst | 9 +
> include/linux/mempolicy.h | 5 +
> include/uapi/linux/mempolicy.h | 1 +
> mm/mempolicy.c | 214 +++++++++++++++++-
> 4 files changed, 226 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/mm/numa_memory_policy.rst b/Documentation/admin-guide/mm/numa_memory_policy.rst
> index eca38fa81e0f..a70f20ce1ffb 100644
> --- a/Documentation/admin-guide/mm/numa_memory_policy.rst
> +++ b/Documentation/admin-guide/mm/numa_memory_policy.rst
> @@ -250,6 +250,15 @@ MPOL_PREFERRED_MANY
> can fall back to all existing numa nodes. This is effectively
> MPOL_PREFERRED allowed for a mask rather than a single node.
>
> +MPOL_WEIGHTED_INTERLEAVE
> + This mode operates the same as MPOL_INTERLEAVE, except that
> + interleaving behavior is executed based on weights set in
> + /sys/kernel/mm/mempolicy/weighted_interleave/
> +
> + Weighted interleave allocates pages on nodes according to a
> + weight. For example if nodes [0,1] are weighted [5,2], 5 pages
> + will be allocated on node0 for every 2 pages allocated on node1.
> +
> NUMA memory policy supports the following optional mode flags:
>
> MPOL_F_STATIC_NODES
> diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
> index 931b118336f4..c1a083eb0dd5 100644
> --- a/include/linux/mempolicy.h
> +++ b/include/linux/mempolicy.h
> @@ -54,6 +54,11 @@ struct mempolicy {
> nodemask_t cpuset_mems_allowed; /* relative to these nodes */
> nodemask_t user_nodemask; /* nodemask passed by user */
> } w;
> +
> + /* Weighted interleave settings */
> + struct {
> + u8 cur_weight;
> + } wil;
> };
>
> /*
> diff --git a/include/uapi/linux/mempolicy.h b/include/uapi/linux/mempolicy.h
> index a8963f7ef4c2..1f9bb10d1a47 100644
> --- a/include/uapi/linux/mempolicy.h
> +++ b/include/uapi/linux/mempolicy.h
> @@ -23,6 +23,7 @@ enum {
> MPOL_INTERLEAVE,
> MPOL_LOCAL,
> MPOL_PREFERRED_MANY,
> + MPOL_WEIGHTED_INTERLEAVE,
> MPOL_MAX, /* always last member of enum */
> };
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0abd3a3394ef..a2b5d64b28e0 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -19,6 +19,13 @@
> * for anonymous memory. For process policy an process counter
> * is used.
> *
> + * weighted interleave
> + * Allocate memory interleaved over a set of nodes based on
> + * a set of weights (per-node), with normal fallback if it
> + * fails. Otherwise operates the same as interleave.
> + * Example: nodeset(0,1) & weights (2,1) - 2 pages allocated
> + * on node 0 for every 1 page allocated on node 1.
> + *
> * bind Only allocate memory on a specific set of nodes,
> * no fallback.
> * FIXME: memory is allocated starting with the first node
> @@ -327,6 +334,7 @@ static struct mempolicy *mpol_new(unsigned short mode, unsigned short flags,
> policy->mode = mode;
> policy->flags = flags;
> policy->home_node = NUMA_NO_NODE;
> + policy->wil.cur_weight = 0;
>
> return policy;
> }
> @@ -439,6 +447,10 @@ static const struct mempolicy_operations mpol_ops[MPOL_MAX] = {
> .create = mpol_new_nodemask,
> .rebind = mpol_rebind_preferred,
> },
> + [MPOL_WEIGHTED_INTERLEAVE] = {
> + .create = mpol_new_nodemask,
> + .rebind = mpol_rebind_nodemask,
> + },
> };
>
> static bool migrate_folio_add(struct folio *folio, struct list_head *foliolist,
> @@ -860,7 +872,8 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
>
> old = current->mempolicy;
> current->mempolicy = new;
> - if (new && new->mode == MPOL_INTERLEAVE)
> + if (new && (new->mode == MPOL_INTERLEAVE ||
> + new->mode == MPOL_WEIGHTED_INTERLEAVE))
> current->il_prev = MAX_NUMNODES-1;
> task_unlock(current);
> mpol_put(old);
> @@ -886,6 +899,7 @@ static void get_policy_nodemask(struct mempolicy *pol, nodemask_t *nodes)
> case MPOL_INTERLEAVE:
> case MPOL_PREFERRED:
> case MPOL_PREFERRED_MANY:
> + case MPOL_WEIGHTED_INTERLEAVE:
> *nodes = pol->nodes;
> break;
> case MPOL_LOCAL:
> @@ -970,6 +984,13 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
> } else if (pol == current->mempolicy &&
> pol->mode == MPOL_INTERLEAVE) {
> *policy = next_node_in(current->il_prev, pol->nodes);
> + } else if (pol == current->mempolicy &&
> + (pol->mode == MPOL_WEIGHTED_INTERLEAVE)) {
> + if (pol->wil.cur_weight)
> + *policy = current->il_prev;
> + else
> + *policy = next_node_in(current->il_prev,
> + pol->nodes);
> } else {
> err = -EINVAL;
> goto out;
> @@ -1799,7 +1820,8 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> pol = __get_vma_policy(vma, addr, ilx);
> if (!pol)
> pol = get_task_policy(current);
> - if (pol->mode == MPOL_INTERLEAVE) {
> + if (pol->mode == MPOL_INTERLEAVE ||
> + pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> *ilx += vma->vm_pgoff >> order;
> *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> }
> @@ -1849,6 +1871,28 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
> return zone >= dynamic_policy_zone;
> }
>
> +static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> +{
> + unsigned int next;
> + struct task_struct *me = current;
> + struct iw_table __rcu *table;
> +
> + next = next_node_in(me->il_prev, policy->nodes);
> + if (next == MAX_NUMNODES)
> + return next;
> +
> + rcu_read_lock();
> + table = rcu_dereference(iw_table);
> + if (!policy->wil.cur_weight)
> + policy->wil.cur_weight = table->weights[next];
> + rcu_read_unlock();
> +
> + policy->wil.cur_weight--;
> + if (!policy->wil.cur_weight)
> + me->il_prev = next;
> + return next;
> +}
> +
> /* Do dynamic interleaving for a process */
> static unsigned int interleave_nodes(struct mempolicy *policy)
> {
> @@ -1883,6 +1927,9 @@ unsigned int mempolicy_slab_node(void)
> case MPOL_INTERLEAVE:
> return interleave_nodes(policy);
>
> + case MPOL_WEIGHTED_INTERLEAVE:
> + return weighted_interleave_nodes(policy);
> +
> case MPOL_BIND:
> case MPOL_PREFERRED_MANY:
> {
> @@ -1921,6 +1968,39 @@ static unsigned int read_once_policy_nodemask(struct mempolicy *pol,
> return nodes_weight(*mask);
> }
>
> +static unsigned int weighted_interleave_nid(struct mempolicy *pol, pgoff_t ilx)
> +{
> + nodemask_t nodemask;
> + unsigned int target, nr_nodes;
> + struct iw_table __rcu *table;
> + unsigned int weight_total = 0;
> + u8 weight;
> + int nid;
> +
> + nr_nodes = read_once_policy_nodemask(pol, &nodemask);
> + if (!nr_nodes)
> + return numa_node_id();
> +
> + rcu_read_lock();
> + table = rcu_dereference(iw_table);
> + /* calculate the total weight */
> + for_each_node_mask(nid, nodemask)
> + weight_total += table->weights[nid];
> +
> + /* Calculate the node offset based on totals */
> + target = ilx % weight_total;
> + nid = first_node(nodemask);
> + while (target) {
> + weight = table->weights[nid];
> + if (target < weight)
> + break;
> + target -= weight;
> + nid = next_node_in(nid, nodemask);
> + }
> + rcu_read_unlock();
> + return nid;
> +}
> +
> /*
> * Do static interleaving for interleave index @ilx. Returns the ilx'th
> * node in pol->nodes (starting from ilx=0), wrapping around if ilx
> @@ -1981,6 +2061,11 @@ static nodemask_t *policy_nodemask(gfp_t gfp, struct mempolicy *pol,
> *nid = (ilx == NO_INTERLEAVE_INDEX) ?
> interleave_nodes(pol) : interleave_nid(pol, ilx);
> break;
> + case MPOL_WEIGHTED_INTERLEAVE:
> + *nid = (ilx == NO_INTERLEAVE_INDEX) ?
> + weighted_interleave_nodes(pol) :
> + weighted_interleave_nid(pol, ilx);
> + break;
> }
>
> return nodemask;
> @@ -2042,6 +2127,7 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask)
> case MPOL_PREFERRED_MANY:
> case MPOL_BIND:
> case MPOL_INTERLEAVE:
> + case MPOL_WEIGHTED_INTERLEAVE:
> *mask = mempolicy->nodes;
> break;
>
> @@ -2141,7 +2227,8 @@ struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
> * If the policy is interleave or does not allow the current
> * node in its nodemask, we allocate the standard way.
> */
> - if (pol->mode != MPOL_INTERLEAVE &&
> + if ((pol->mode != MPOL_INTERLEAVE &&
> + pol->mode != MPOL_WEIGHTED_INTERLEAVE) &&
> (!nodemask || node_isset(nid, *nodemask))) {
> /*
> * First, try to allocate THP only on local node, but
> @@ -2277,6 +2364,114 @@ static unsigned long alloc_pages_bulk_array_interleave(gfp_t gfp,
> return total_allocated;
> }
>
> +static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> + struct mempolicy *pol, unsigned long nr_pages,
> + struct page **page_array)
> +{
> + struct task_struct *me = current;
> + unsigned long total_allocated = 0;
> + unsigned long nr_allocated;
> + unsigned long rounds;
> + unsigned long node_pages, delta;
> + u8 weight;
> + struct iw_table __rcu *table;
> + u8 *weights;
> + unsigned int weight_total = 0;
> + unsigned long rem_pages = nr_pages;
> + nodemask_t nodes;
> + int nnodes, node, weight_nodes;
> + int prev_node = NUMA_NO_NODE;
> + int i;
> +
> + nnodes = read_once_policy_nodemask(pol, &nodes);
> + if (!nnodes)
> + return 0;
> +
> + /* Continue allocating from most recent node and adjust the nr_pages */
> + if (pol->wil.cur_weight) {
> + node = next_node_in(me->il_prev, nodes);
> + node_pages = pol->wil.cur_weight;
> + if (node_pages > rem_pages)
> + node_pages = rem_pages;
> + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> + NULL, page_array);
> + page_array += nr_allocated;
> + total_allocated += nr_allocated;
> + /* if that's all the pages, no need to interleave */
> + if (rem_pages <= pol->wil.cur_weight) {
> + pol->wil.cur_weight -= rem_pages;
> + return total_allocated;
> + }
> + /* Otherwise we adjust nr_pages down, and continue from there */
> + rem_pages -= pol->wil.cur_weight;
> + pol->wil.cur_weight = 0;
> + prev_node = node;
> + }
> +
> + /* fetch the weights for this operation and calculate total weight */
> + weights = kmalloc(nnodes, GFP_KERNEL);
> + if (!weights)
> + return total_allocated;
> +
> + rcu_read_lock();
> + table = rcu_dereference(iw_table);
> + weight_nodes = 0;
> + for_each_node_mask(node, nodes) {
> + weights[weight_nodes++] = table->weights[node];
> + weight_total += table->weights[node];
> + }
> + rcu_read_unlock();
> +
> + if (!weight_total) {
> + kfree(weights);
> + return total_allocated;
> + }
> +
> + /* Now we can continue allocating as if from 0 instead of an offset */
> + rounds = rem_pages / weight_total;
> + delta = rem_pages % weight_total;
> + for (i = 0; i < nnodes; i++) {
> + node = next_node_in(prev_node, nodes);
> + weight = weights[i];
> + node_pages = weight * rounds;
> + if (delta) {
> + if (delta > weight) {
> + node_pages += weight;
> + delta -= weight;
> + } else {
> + node_pages += delta;
> + delta = 0;
> + }
> + }
> + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> + NULL, page_array);
> + page_array += nr_allocated;
> + total_allocated += nr_allocated;
> + if (total_allocated == nr_pages)
> + break;
> + prev_node = node;
> + }
> +
> + /*
> + * Finally, we need to update me->il_prev and pol->wil.cur_weight
> + * if there were overflow pages, but not equivalent to the node
> + * weight, set the cur_weight to node_weight - delta and the
> + * me->il_prev to the previous node. Otherwise if it was perfect
> + * we can simply set il_prev to node and cur_weight to 0
> + */
> + if (node_pages) {
> + me->il_prev = prev_node;
> + node_pages %= weight;
> + pol->wil.cur_weight = weight - node_pages;
> + } else {
> + me->il_prev = node;
> + pol->wil.cur_weight = 0;
> + }
It appears that we should set me->il_prev and pol->wil.cur_weight when
delta becomes 0? That is, following allocation should start from there?
--
Best Regards,
Huang, Ying
> +
> + kfree(weights);
> + return total_allocated;
> +}
> +
> static unsigned long alloc_pages_bulk_array_preferred_many(gfp_t gfp, int nid,
> struct mempolicy *pol, unsigned long nr_pages,
> struct page **page_array)
> @@ -2317,6 +2512,11 @@ unsigned long alloc_pages_bulk_array_mempolicy(gfp_t gfp,
> return alloc_pages_bulk_array_interleave(gfp, pol,
> nr_pages, page_array);
>
> + if (pol->mode == MPOL_WEIGHTED_INTERLEAVE)
> + return alloc_pages_bulk_array_weighted_interleave(gfp, pol,
> + nr_pages,
> + page_array);
> +
> if (pol->mode == MPOL_PREFERRED_MANY)
> return alloc_pages_bulk_array_preferred_many(gfp,
> numa_node_id(), pol, nr_pages, page_array);
> @@ -2392,6 +2592,7 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
> case MPOL_INTERLEAVE:
> case MPOL_PREFERRED:
> case MPOL_PREFERRED_MANY:
> + case MPOL_WEIGHTED_INTERLEAVE:
> return !!nodes_equal(a->nodes, b->nodes);
> case MPOL_LOCAL:
> return true;
> @@ -2528,6 +2729,10 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
> polnid = interleave_nid(pol, ilx);
> break;
>
> + case MPOL_WEIGHTED_INTERLEAVE:
> + polnid = weighted_interleave_nid(pol, ilx);
> + break;
> +
> case MPOL_PREFERRED:
> if (node_isset(curnid, pol->nodes))
> goto out;
> @@ -2902,6 +3107,7 @@ static const char * const policy_modes[] =
> [MPOL_PREFERRED] = "prefer",
> [MPOL_BIND] = "bind",
> [MPOL_INTERLEAVE] = "interleave",
> + [MPOL_WEIGHTED_INTERLEAVE] = "weighted interleave",
> [MPOL_LOCAL] = "local",
> [MPOL_PREFERRED_MANY] = "prefer (many)",
> };
> @@ -2961,6 +3167,7 @@ int mpol_parse_str(char *str, struct mempolicy **mpol)
> }
> break;
> case MPOL_INTERLEAVE:
> + case MPOL_WEIGHTED_INTERLEAVE:
> /*
> * Default to online nodes with memory if no nodelist
> */
> @@ -3071,6 +3278,7 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
> case MPOL_PREFERRED_MANY:
> case MPOL_BIND:
> case MPOL_INTERLEAVE:
> + case MPOL_WEIGHTED_INTERLEAVE:
> nodes = pol->nodes;
> break;
> default:
^ permalink raw reply
* 回复: [PATCH v4] selftests/move_mount_set_group:Make tests build with old libc
From: Huyadi @ 2024-01-18 2:03 UTC (permalink / raw)
To: 'Mickaël Salaün', 'Christian Brauner',
Christian Brauner, serge@hallyn.com, Shuah Khan
Cc: jmorris@namei.org, serge@hallyn.com, shuah@kernel.org,
mathieu.desnoyers@efficios.com, linux-api@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kselftest@vger.kernel.org, 514118380@qq.com,
Christian Brauner, linux-fsdevel@vger.kernel.org, Shuah Khan
In-Reply-To: <20240111.mee0ohZie5he@digikod.net>
>On Thu, Jan 11, 2024 at 07:32:29PM +0800, Hu Yadi wrote:
>> From: "Hu.Yadi" <hu.yadi@h3c.com>
>>
>> Replace SYS_<syscall> with __NR_<syscall>. Using the __NR_<syscall>
>> notation, provided by UAPI, is useful to build tests on systems
>> without the SYS_<syscall> definitions.
>>
>> Replace SYS_move_mount with __NR_move_mount
>>
>> Similar changes: commit 87129ef13603 ("selftests/landlock: Make tests
>> build with old libc")
>>
>> Acked-by: Mickaël Salaün <mic@digikod.net>
>
>Sorry, it should have been Reviewed-by: Mickaël Salaün <mic@digikod.net>
>
>Also, this is maintained by the VFS maintainers. I CCed three relevant addresses.
Any progesss about this patch ?
Thanks
>
>> Signed-off-by: Hu.Yadi <hu.yadi@h3c.com>
>> Suggested-by: Jiao <jiaoxupo@h3c.com>
>> Reviewed-by: Berlin <berlin@h3c.com>
>> ---
>> Changes v4 -> v3:
>> - Adjust comments for consistent
>> - Add Acked-by
>> Changes v2 -> v3:
>> - Adjust comments
>> Changes v1 -> v2:
>> - Fix mail of Suggested-by and Reviewed-by
>>
>> .../move_mount_set_group/move_mount_set_group_test.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git
>> a/tools/testing/selftests/move_mount_set_group/move_mount_set_group_te
>> st.c
>> b/tools/testing/selftests/move_mount_set_group/move_mount_set_group_te
>> st.c index 50ed5d475dd1..bcf51d785a37 100644
>> ---
>> a/tools/testing/selftests/move_mount_set_group/move_mount_set_group_te
>> st.c
>> +++ b/tools/testing/selftests/move_mount_set_group/move_mount_set_grou
>> +++ p_test.c
>> @@ -218,7 +218,7 @@ static bool move_mount_set_group_supported(void)
>> if (mount(NULL, SET_GROUP_FROM, NULL, MS_SHARED, 0))
>> return -1;
>>
>> - ret = syscall(SYS_move_mount, AT_FDCWD, SET_GROUP_FROM,
>> + ret = syscall(__NR_move_mount, AT_FDCWD, SET_GROUP_FROM,
>> AT_FDCWD, SET_GROUP_TO, MOVE_MOUNT_SET_GROUP);
>> umount2("/tmp", MNT_DETACH);
>>
>> @@ -363,7 +363,7 @@ TEST_F(move_mount_set_group, complex_sharing_copying)
>> CLONE_VM | CLONE_FILES); ASSERT_GT(pid, 0);
>> ASSERT_EQ(wait_for_pid(pid), 0);
>>
>> - ASSERT_EQ(syscall(SYS_move_mount, ca_from.mntfd, "",
>> + ASSERT_EQ(syscall(__NR_move_mount, ca_from.mntfd, "",
>> ca_to.mntfd, "", MOVE_MOUNT_SET_GROUP
>> | MOVE_MOUNT_F_EMPTY_PATH | MOVE_MOUNT_T_EMPTY_PATH),
>> 0);
>> --
>> 2.23.0
>>
>>
^ permalink raw reply
* Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Huang, Ying @ 2024-01-18 1:28 UTC (permalink / raw)
To: Gregory Price
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <Zadm5r/23tonKeXB@memverge.com>
Gregory Price <gregory.price@memverge.com> writes:
> On Mon, Jan 15, 2024 at 01:47:31PM +0800, Huang, Ying wrote:
>> Gregory Price <gourry.memverge@gmail.com> writes:
>>
>> > + /* Continue allocating from most recent node and adjust the nr_pages */
>> > + if (pol->wil.cur_weight) {
>> > + node = next_node_in(me->il_prev, nodes);
>> > + node_pages = pol->wil.cur_weight;
>> > + if (node_pages > rem_pages)
>> > + node_pages = rem_pages;
>> > + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
>> > + NULL, page_array);
> ... snip ...
>> > + if (delta > weight) {
>> > + node_pages += weight;
>> > + delta -= weight;
>> > + } else {
>> > + node_pages += delta;
>> > + delta = 0;
>> > + }
>> > + }
>> > + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
>> > + NULL, page_array);
>>
>> Should we check nr_allocated here? Allocation may fail anyway.
>>
>
> I thought about this briefly in both situations.
>
> If you look at alloc_pages_bulk_array_interleave(), it does not fail if
> __alloc_pages_bulk() fails, instead it continues and attempts to
> allocate from the remaining nodes.
>
> Presumably, this is because the caller of the bulk allocator can accept
> a partial-failure and will go ahead and allocate the remaining pages on
> an extra slow path.
>
> Since alloc_pages_bulk_array_interleave() appears to be capable of
> failing in the exact same way, I considered this safe.
You are right. We should proceed with next node here.
--
Best Regards,
Huang, Ying
>> > + if (pol->mode == MPOL_WEIGHTED_INTERLEAVE)
>> > + return alloc_pages_bulk_array_weighted_interleave(gfp, pol,
>> > + nr_pages,
>> > + page_array);
>> > +
>>
>> Just nit-pick, may be better to be
>>
>> return alloc_pages_bulk_array_weighted_interleave(
>> gfp, pol, nr_pages, page_array);
>>
>
> Wasn't sure on style when names get this long lol, will make the change
> :]
>
>
>
> Probably v2 thursday or friday
>
> Regards
> ~Gregory
^ permalink raw reply
* Re: [PATCH 1/3] mm/mempolicy: implement the sysfs-based weighted_interleave interface
From: Gregory Price @ 2024-01-17 17:46 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <87o7dkzbsv.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Wed, Jan 17, 2024 at 02:58:08PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
>
> > We haven't had the discussion on how/when this should happen yet,
> > though, and there's some research to be done. (i.e. when should DRAM
> > weights be set? should the entire table be reweighted on hotplug? etc)
>
> Before that, I'm OK to remove default_iw_table and use hard coded "1" as
> default weight for now.
>
Can't quite do that. default_iw_table is a static structure because we
need a reliable default structure not subject to module initialization
failure. Otherwise we can end up in a situation where iw_table is NULL
during some allocation path if the sysfs structure fails to setup fully.
There's no good reason to fail allocations just because sysfs failed to
initialization for some reason. I'll leave default_iw_table with a size
of MAX_NUMNODES for now (nr_node_ids is set up at runtime per your
reference to `setup_nr_node_ids` below, so we can't use it for this).
> >
> >> u8 __rcu *iw_table;
> >>
> >> Then, we only need to allocate nr_node_ids elements now.
> >>
> >
> > We need nr_possible_nodes to handle hotplug correctly.
>
> nr_node_ids >= num_possible_nodes(). It's larger than any possible node
> ID.
>
nr_node_ids gets setup at runtime, while the default_iw_table needs
to be a static structure (see above). I can make default_iw_table
MAX_NUMNODES and subsequent allocations of iw_table be nr_node_ids,
but that makes iw_table a different size at any given time.
This *will* break if "true hotplug" ever shows up and possible_nodes !=
MAX_NUMNODES. But I can write it up if it's a sticking point for you.
Ultimately we're squabbling over, at most, about ~3kb of memory, just
keep that in mind. (I guess if you spawn 3000 threads and each tries a
concurrent write to sysfs/node1, you'd eat 3MB view briefly, but that
is a truly degenerate case and I can think of more denegerate things).
>
> When "true node hotplug" becomes reality, we can make nr_node_ids ==
> MAX_NUMNODES. So, it's safe to use it. Please take a look at
> setup_nr_node_ids().
>
~Gregory
^ permalink raw reply
* Re: [PATCH] selftests/filesystems:fix build error in overlayfs
From: Christian Brauner @ 2024-01-17 13:23 UTC (permalink / raw)
To: jmorris, serge, shuah, mathieu.desnoyers, mic, amir73il, avagin,
Hu Yadi
Cc: Christian Brauner, linux-api, linux-kernel, linux-security-module,
linux-kselftest, 514118380
In-Reply-To: <20240112074059.29673-1-hu.yadi@h3c.com>
On Fri, 12 Jan 2024 15:40:59 +0800, Hu Yadi wrote:
> One build issue comes up due to both mount.h included dev_in_maps.c
>
> In file included from dev_in_maps.c:10:
> /usr/include/sys/mount.h:35:3: error: expected identifier before numeric constant
> 35 | MS_RDONLY = 1, /* Mount read-only. */
> | ^~~~~~~~~
> In file included from dev_in_maps.c:13:
>
> [...]
Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc
[1/1] selftests/filesystems:fix build error in overlayfs
https://git.kernel.org/vfs/vfs/c/f67dae6ba174
^ permalink raw reply
* Re: [PATCH 1/1] socket.7: Mention CAP_NET_RAW on SO_MARK
From: Petr Vorel @ 2024-01-17 12:08 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-man, Maciej Żenczykowski, linux-kernel, linux-api,
Matthias Gerstner, Avinesh Kumar
In-Reply-To: <ZafBxnhOSWxBRWko@debian>
> Hi Petr,
> On Wed, Jan 17, 2024 at 10:14:52AM +0100, Petr Vorel wrote:
> > Hi Alex,
> > > Hi Petr,
> > > On Tue, Jan 16, 2024 at 12:04:18PM +0100, Petr Vorel wrote:
> > > > Added in 079925cce1d0 ("net: allow SO_MARK with CAP_NET_RAW") in v5.17.
> > > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > Patch applied. Thanks!
> > Thank you! BTW I don't see this patch in git tree [1], maybe you just haven't
> > push yet.
> That's it. I'm first pushing them to a 'contrib' branch (similar to
> Linux 'next') in my own server[1], and after one day or so I push to
> master. That buffering allows me to catch some mistakes.
Thanks a lot for info!
> [1]: <https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/log/?h=contrib>
Ah, maybe my patch to MAINTAINERS was wrong, please push the correct tree
https://lore.kernel.org/linux-man/20240117091903.2668916-1-pvorel@suse.cz/
> $ git log --all --graph --oneline -10
> * 0c118822f (alx/contrib, contrib) open.2: srcfix
> * 24347d22a syscalls.2: ffix
> * 9e04cb40f mremap.2: wfix
> * 3a271d5b8 fts.3, div_t.3type: ffix
> * 527f31b9f socket.7: Mention CAP_NET_RAW on SO_MARK
> | * 71c25f15d (alx/MR, MR) man*/: ffix (MR.sed)
> |/
> * d82ce270c (HEAD -> master, korg/master, alx/main, main) locale.5: tfix
> * f65154cba cpuid.4: Note which CPUs don't support CPUID and what happens
> * f98168310 sched_getcpu.3: tfix
> * 26cdb3ebb faccessat(2), utimensat(2): Document AT_EMPTY_PATH flag
> I'll push in a moment to master.
Thanks!
Kind regards,
Petr
> Have a lovely day,
> Alex
^ permalink raw reply
* Re: [PATCH 1/1] socket.7: Mention CAP_NET_RAW on SO_MARK
From: Alejandro Colomar @ 2024-01-17 12:02 UTC (permalink / raw)
To: Petr Vorel
Cc: linux-man, Maciej Żenczykowski, linux-kernel, linux-api,
Matthias Gerstner, Avinesh Kumar
In-Reply-To: <20240117091452.GB2665992@pevik>
[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]
Hi Petr,
On Wed, Jan 17, 2024 at 10:14:52AM +0100, Petr Vorel wrote:
> Hi Alex,
>
> > Hi Petr,
>
> > On Tue, Jan 16, 2024 at 12:04:18PM +0100, Petr Vorel wrote:
> > > Added in 079925cce1d0 ("net: allow SO_MARK with CAP_NET_RAW") in v5.17.
>
> > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
>
> > Patch applied. Thanks!
>
> Thank you! BTW I don't see this patch in git tree [1], maybe you just haven't
> push yet.
That's it. I'm first pushing them to a 'contrib' branch (similar to
Linux 'next') in my own server[1], and after one day or so I push to
master. That buffering allows me to catch some mistakes.
[1]: <https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/log/?h=contrib>
$ git log --all --graph --oneline -10
* 0c118822f (alx/contrib, contrib) open.2: srcfix
* 24347d22a syscalls.2: ffix
* 9e04cb40f mremap.2: wfix
* 3a271d5b8 fts.3, div_t.3type: ffix
* 527f31b9f socket.7: Mention CAP_NET_RAW on SO_MARK
| * 71c25f15d (alx/MR, MR) man*/: ffix (MR.sed)
|/
* d82ce270c (HEAD -> master, korg/master, alx/main, main) locale.5: tfix
* f65154cba cpuid.4: Note which CPUs don't support CPUID and what happens
* f98168310 sched_getcpu.3: tfix
* 26cdb3ebb faccessat(2), utimensat(2): Document AT_EMPTY_PATH flag
I'll push in a moment to master.
Have a lovely day,
Alex
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] socket.7: Mention CAP_NET_RAW on SO_MARK
From: Petr Vorel @ 2024-01-17 9:14 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-man, Maciej Żenczykowski, linux-kernel, linux-api,
Matthias Gerstner, Avinesh Kumar
In-Reply-To: <ZaadPmLFCI4rsGy_@debian>
Hi Alex,
> Hi Petr,
> On Tue, Jan 16, 2024 at 12:04:18PM +0100, Petr Vorel wrote:
> > Added in 079925cce1d0 ("net: allow SO_MARK with CAP_NET_RAW") in v5.17.
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> Patch applied. Thanks!
Thank you! BTW I don't see this patch in git tree [1], maybe you just haven't
push yet.
Kind regards,
Petr
[1] https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/
> Have a lovely day,
> Alex
> > ---
> > man7/socket.7 | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> > diff --git a/man7/socket.7 b/man7/socket.7
> > index 3ff8fe51b..3ebfc770b 100644
> > --- a/man7/socket.7
> > +++ b/man7/socket.7
> > @@ -614,7 +614,9 @@ Changing the mark can be used for mark-based
> > routing without netfilter or for packet filtering.
> > Setting this option requires the
> > .B CAP_NET_ADMIN
> > -capability.
> > +or
> > +.B CAP_NET_RAW
> > +(since Linux 5.17) capability.
> > .TP
> > .B SO_OOBINLINE
> > If this option is enabled,
> > --
> > 2.43.0
^ permalink raw reply
* Re: [PATCH 1/3] mm/mempolicy: implement the sysfs-based weighted_interleave interface
From: Huang, Ying @ 2024-01-17 6:58 UTC (permalink / raw)
To: Gregory Price
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <ZadkmWj3Rd483f68@memverge.com>
Gregory Price <gregory.price@memverge.com> writes:
> On Mon, Jan 15, 2024 at 11:18:00AM +0800, Huang, Ying wrote:
>> Gregory Price <gourry.memverge@gmail.com> writes:
>>
>> > +static struct iw_table default_iw_table;
>> > +/*
>> > + * iw_table is the sysfs-set interleave weight table, a value of 0
>> > + * denotes that the default_iw_table value should be used.
>> > + *
>> > + * iw_table is RCU protected
>> > + */
>> > +static struct iw_table __rcu *iw_table;
>> > +static DEFINE_MUTEX(iw_table_mtx);
>>
>> I greped "mtx" in kernel/*.c and mm/*.c and found nothing. To following
>> the existing coding convention, better to name this as iw_table_mutex or
>> iw_table_lock?
>>
>
> ack.
>
>> And, I think this is used to protect both iw_table and default_iw_table?
>> If so, it deserves some comments.
>>
>
> Right now default_iw_table cannot be updated, and so it is neither
> protected nor requires protection.
>
> I planned to add the protection comment in the next patch series, which
> would implement the kernel-side interface for updating the default
> weights during boot/hotplug.
>
> We haven't had the discussion on how/when this should happen yet,
> though, and there's some research to be done. (i.e. when should DRAM
> weights be set? should the entire table be reweighted on hotplug? etc)
Before that, I'm OK to remove default_iw_table and use hard coded "1" as
default weight for now.
>> > +static ssize_t node_store(struct kobject *kobj, struct kobj_attribute *attr,
>> > + const char *buf, size_t count)
>> > +{
>> > + struct iw_node_attr *node_attr;
>> > + struct iw_table __rcu *new;
>> > + struct iw_table __rcu *old;
>> > + u8 weight = 0;
>> > +
>> > + node_attr = container_of(attr, struct iw_node_attr, kobj_attr);
>> > + if (count == 0 || sysfs_streq(buf, ""))
>> > + weight = 0;
>> > + else if (kstrtou8(buf, 0, &weight))
>> > + return -EINVAL;
>> > +
>> > + new = kmalloc(sizeof(*new), GFP_KERNEL);
>> > + if (!new)
>> > + return -ENOMEM;
>> > +
>> > + mutex_lock(&iw_table_mtx);
>> > + old = rcu_dereference_protected(iw_table,
>> > + lockdep_is_held(&iw_table_mtx));
>> > + /* If value is 0, revert to default weight */
>> > + weight = weight ? weight : default_iw_table.weights[node_attr->nid];
>>
>> If we change the default weight in default_iw_table.weights[], how do we
>> identify whether the weight has been customized by users via sysfs? So,
>> I suggest to use 0 in iw_table for not-customized weight.
>>
>> And if so, we need to use RCU to access default_iw_table too.
>>
>
> Dumb simplification on my part, I'll walk this back and add the
>
> if (!weight) weight = default_iw_table[node]
>
> logic back into the allocator paths accordinly.
>
>> > + memcpy(&new->weights, &old->weights, sizeof(new->weights));
>> > + new->weights[node_attr->nid] = weight;
>> > + rcu_assign_pointer(iw_table, new);
>> > + mutex_unlock(&iw_table_mtx);
>> > + kfree_rcu(old, rcu);
>>
>> synchronize_rcu() should be OK here. It's fast enough in this cold
>> path. This make it good to define iw_table as
>>
> I'll take a look.
>
>> u8 __rcu *iw_table;
>>
>> Then, we only need to allocate nr_node_ids elements now.
>>
>
> We need nr_possible_nodes to handle hotplug correctly.
nr_node_ids >= num_possible_nodes(). It's larger than any possible node
ID.
> I decided to simplify this down to MAX_NUMNODES *juuuuuust in case*
> "true node hotplug" ever becomes a reality. If that happens, then
> only allocating space for possible nodes creates a much bigger
> headache on hotplug.
>
> For the sake of that simplification, it seemed better to just eat the
> 1KB. If you really want me to do that, I will, but the MAX_NUMNODES
> choice was an explicitly defensive choice.
When "true node hotplug" becomes reality, we can make nr_node_ids ==
MAX_NUMNODES. So, it's safe to use it. Please take a look at
setup_nr_node_ids().
>> > +static int __init mempolicy_sysfs_init(void)
>> > +{
>> > + /*
>> > + * if sysfs is not enabled MPOL_WEIGHTED_INTERLEAVE defaults to
>> > + * MPOL_INTERLEAVE behavior, but is still defined separately to
>> > + * allow task-local weighted interleave and system-defaults to
>> > + * operate as intended.
>> > + *
>> > + * In this scenario iw_table cannot (presently) change, so
>> > + * there's no need to set up RCU / cleanup code.
>> > + */
>> > + memset(&default_iw_table.weights, 1, sizeof(default_iw_table));
>>
>> This depends on sizeof(default_iw_table.weights[0]) == 1, I think it's
>> better to use explicit loop here to make the code more robust a little.
>>
>
> oh hm, you're right. rookie mistake on my part.
>
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-01-17 5:34 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <87bk9n172k.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Mon, Jan 15, 2024 at 01:47:31PM +0800, Huang, Ying wrote:
> Gregory Price <gourry.memverge@gmail.com> writes:
>
> > + /* Continue allocating from most recent node and adjust the nr_pages */
> > + if (pol->wil.cur_weight) {
> > + node = next_node_in(me->il_prev, nodes);
> > + node_pages = pol->wil.cur_weight;
> > + if (node_pages > rem_pages)
> > + node_pages = rem_pages;
> > + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> > + NULL, page_array);
... snip ...
> > + if (delta > weight) {
> > + node_pages += weight;
> > + delta -= weight;
> > + } else {
> > + node_pages += delta;
> > + delta = 0;
> > + }
> > + }
> > + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> > + NULL, page_array);
>
> Should we check nr_allocated here? Allocation may fail anyway.
>
I thought about this briefly in both situations.
If you look at alloc_pages_bulk_array_interleave(), it does not fail if
__alloc_pages_bulk() fails, instead it continues and attempts to
allocate from the remaining nodes.
Presumably, this is because the caller of the bulk allocator can accept
a partial-failure and will go ahead and allocate the remaining pages on
an extra slow path.
Since alloc_pages_bulk_array_interleave() appears to be capable of
failing in the exact same way, I considered this safe.
> > + if (pol->mode == MPOL_WEIGHTED_INTERLEAVE)
> > + return alloc_pages_bulk_array_weighted_interleave(gfp, pol,
> > + nr_pages,
> > + page_array);
> > +
>
> Just nit-pick, may be better to be
>
> return alloc_pages_bulk_array_weighted_interleave(
> gfp, pol, nr_pages, page_array);
>
Wasn't sure on style when names get this long lol, will make the change
:]
Probably v2 thursday or friday
Regards
~Gregory
^ permalink raw reply
* Re: [PATCH 2/3] mm/mempolicy: refactor a read-once mechanism into a function for re-use
From: Gregory Price @ 2024-01-17 5:26 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <87h6jf1bfx.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Mon, Jan 15, 2024 at 12:13:06PM +0800, Huang, Ying wrote:
> Gregory Price <gourry.memverge@gmail.com> writes:
>
> >
> > +static unsigned int read_once_policy_nodemask(struct mempolicy *pol,
> > + nodemask_t *mask)
>
> It may be more useful if we define this as memcpy_once(). That can be
> used not only for nodemask, but also other data structure.
>
Seemed better to do this is an entirely separate patch line to avoid
scope creep on reviews and such.
> > + barrier();
> > + __builtin_memcpy(mask, &pol->nodes, sizeof(nodemask_t));
>
> We don't use __builtin_memcpy() in kernel itself directly. Although it
> is used in kernel tools. So, I think it's better to use memcpy() here.
>
ack.
~Gregory
^ permalink raw reply
* Re: [PATCH 1/3] mm/mempolicy: implement the sysfs-based weighted_interleave interface
From: Gregory Price @ 2024-01-17 5:24 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <87le8r1dzr.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Mon, Jan 15, 2024 at 11:18:00AM +0800, Huang, Ying wrote:
> Gregory Price <gourry.memverge@gmail.com> writes:
>
> > +static struct iw_table default_iw_table;
> > +/*
> > + * iw_table is the sysfs-set interleave weight table, a value of 0
> > + * denotes that the default_iw_table value should be used.
> > + *
> > + * iw_table is RCU protected
> > + */
> > +static struct iw_table __rcu *iw_table;
> > +static DEFINE_MUTEX(iw_table_mtx);
>
> I greped "mtx" in kernel/*.c and mm/*.c and found nothing. To following
> the existing coding convention, better to name this as iw_table_mutex or
> iw_table_lock?
>
ack.
> And, I think this is used to protect both iw_table and default_iw_table?
> If so, it deserves some comments.
>
Right now default_iw_table cannot be updated, and so it is neither
protected nor requires protection.
I planned to add the protection comment in the next patch series, which
would implement the kernel-side interface for updating the default
weights during boot/hotplug.
We haven't had the discussion on how/when this should happen yet,
though, and there's some research to be done. (i.e. when should DRAM
weights be set? should the entire table be reweighted on hotplug? etc)
> > +static ssize_t node_store(struct kobject *kobj, struct kobj_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct iw_node_attr *node_attr;
> > + struct iw_table __rcu *new;
> > + struct iw_table __rcu *old;
> > + u8 weight = 0;
> > +
> > + node_attr = container_of(attr, struct iw_node_attr, kobj_attr);
> > + if (count == 0 || sysfs_streq(buf, ""))
> > + weight = 0;
> > + else if (kstrtou8(buf, 0, &weight))
> > + return -EINVAL;
> > +
> > + new = kmalloc(sizeof(*new), GFP_KERNEL);
> > + if (!new)
> > + return -ENOMEM;
> > +
> > + mutex_lock(&iw_table_mtx);
> > + old = rcu_dereference_protected(iw_table,
> > + lockdep_is_held(&iw_table_mtx));
> > + /* If value is 0, revert to default weight */
> > + weight = weight ? weight : default_iw_table.weights[node_attr->nid];
>
> If we change the default weight in default_iw_table.weights[], how do we
> identify whether the weight has been customized by users via sysfs? So,
> I suggest to use 0 in iw_table for not-customized weight.
>
> And if so, we need to use RCU to access default_iw_table too.
>
Dumb simplification on my part, I'll walk this back and add the
if (!weight) weight = default_iw_table[node]
logic back into the allocator paths accordinly.
> > + memcpy(&new->weights, &old->weights, sizeof(new->weights));
> > + new->weights[node_attr->nid] = weight;
> > + rcu_assign_pointer(iw_table, new);
> > + mutex_unlock(&iw_table_mtx);
> > + kfree_rcu(old, rcu);
>
> synchronize_rcu() should be OK here. It's fast enough in this cold
> path. This make it good to define iw_table as
>
I'll take a look.
> u8 __rcu *iw_table;
>
> Then, we only need to allocate nr_node_ids elements now.
>
We need nr_possible_nodes to handle hotplug correctly.
I decided to simplify this down to MAX_NUMNODES *juuuuuust in case*
"true node hotplug" ever becomes a reality. If that happens, then
only allocating space for possible nodes creates a much bigger
headache on hotplug.
For the sake of that simplification, it seemed better to just eat the
1KB. If you really want me to do that, I will, but the MAX_NUMNODES
choice was an explicitly defensive choice.
> > +static int __init mempolicy_sysfs_init(void)
> > +{
> > + /*
> > + * if sysfs is not enabled MPOL_WEIGHTED_INTERLEAVE defaults to
> > + * MPOL_INTERLEAVE behavior, but is still defined separately to
> > + * allow task-local weighted interleave and system-defaults to
> > + * operate as intended.
> > + *
> > + * In this scenario iw_table cannot (presently) change, so
> > + * there's no need to set up RCU / cleanup code.
> > + */
> > + memset(&default_iw_table.weights, 1, sizeof(default_iw_table));
>
> This depends on sizeof(default_iw_table.weights[0]) == 1, I think it's
> better to use explicit loop here to make the code more robust a little.
>
oh hm, you're right. rookie mistake on my part.
Thanks,
Gregory
^ permalink raw reply
* Re: [PATCH] selftests/filesystems:fix build error in overlayfs
From: Andrei Vagin @ 2024-01-17 1:43 UTC (permalink / raw)
To: Hu Yadi
Cc: jmorris, serge, shuah, mathieu.desnoyers, mic, amir73il, brauner,
linux-api, linux-kernel, linux-security-module, linux-kselftest,
514118380
In-Reply-To: <20240112074059.29673-1-hu.yadi@h3c.com>
On Thu, Jan 11, 2024 at 11:45 PM Hu Yadi <hu.yadi@h3c.com> wrote:
>
> One build issue comes up due to both mount.h included dev_in_maps.c
>
> In file included from dev_in_maps.c:10:
> /usr/include/sys/mount.h:35:3: error: expected identifier before numeric constant
> 35 | MS_RDONLY = 1, /* Mount read-only. */
> | ^~~~~~~~~
> In file included from dev_in_maps.c:13:
>
> Remove one of them to solve conflict, another error comes up:
>
> dev_in_maps.c:170:6: error: implicit declaration of function ‘mount’ [-Werror=implicit-function-declaration]
> 170 | if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) == -1) {
> | ^~~~~
> cc1: all warnings being treated as errors
>
> and then , add sys_mount definition to solve it
> After both above, dev_in_maps.c can be built correctly on my mache(gcc 10.2,glibc-2.32,kernel-5.10)
>
> Signed-off-by: Hu Yadi <hu.yadi@h3c.com>
Acked-by: Andrei Vagin <avagin@google.com>
^ permalink raw reply
* Re: [PATCH 1/1] socket.7: Mention CAP_NET_RAW on SO_MARK
From: Alejandro Colomar @ 2024-01-16 15:14 UTC (permalink / raw)
To: Petr Vorel
Cc: linux-man, Maciej Żenczykowski, linux-kernel, linux-api,
Matthias Gerstner, Avinesh Kumar
In-Reply-To: <20240116110418.2577798-1-pvorel@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 910 bytes --]
Hi Petr,
On Tue, Jan 16, 2024 at 12:04:18PM +0100, Petr Vorel wrote:
> Added in 079925cce1d0 ("net: allow SO_MARK with CAP_NET_RAW") in v5.17.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
Patch applied. Thanks!
Have a lovely day,
Alex
> ---
> man7/socket.7 | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/man7/socket.7 b/man7/socket.7
> index 3ff8fe51b..3ebfc770b 100644
> --- a/man7/socket.7
> +++ b/man7/socket.7
> @@ -614,7 +614,9 @@ Changing the mark can be used for mark-based
> routing without netfilter or for packet filtering.
> Setting this option requires the
> .B CAP_NET_ADMIN
> -capability.
> +or
> +.B CAP_NET_RAW
> +(since Linux 5.17) capability.
> .TP
> .B SO_OOBINLINE
> If this option is enabled,
> --
> 2.43.0
>
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/1] socket.7: Mention CAP_NET_RAW on SO_MARK
From: Petr Vorel @ 2024-01-16 11:04 UTC (permalink / raw)
To: linux-man
Cc: Petr Vorel, Maciej Żenczykowski, Alejandro Colomar,
linux-kernel, linux-api, Matthias Gerstner, Avinesh Kumar
Added in 079925cce1d0 ("net: allow SO_MARK with CAP_NET_RAW") in v5.17.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
man7/socket.7 | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/man7/socket.7 b/man7/socket.7
index 3ff8fe51b..3ebfc770b 100644
--- a/man7/socket.7
+++ b/man7/socket.7
@@ -614,7 +614,9 @@ Changing the mark can be used for mark-based
routing without netfilter or for packet filtering.
Setting this option requires the
.B CAP_NET_ADMIN
-capability.
+or
+.B CAP_NET_RAW
+(since Linux 5.17) capability.
.TP
.B SO_OOBINLINE
If this option is enabled,
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox