* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Andy Lutomirski @ 2024-01-31 19:24 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tycho Andersen, Christian Brauner, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <20240131191405.GF2609@redhat.com>
> On 01/31, Oleg Nesterov wrote:
> >
> > On 01/31, Andy Lutomirski wrote:
> > Please note
> >
> > /* TODO: respect PIDFD_THREAD */
> >
> > this patch adds into pidfd_send_signal().
> >
> > See also this part of discussion
> >
> > > > + /* TODO: respect PIDFD_THREAD */
> > >
> > > So I've been thinking about this at the end of last week. Do we need to
> > > give userspace a way to send a thread-group wide signal even when a
> > > PIDFD_THREAD pidfd is passed? Or should we just not worry about this
> > > right now and wait until someone needs this?
> >
> > I don't know. I am fine either way, but I think this needs a separate
> > patch and another discussion in any case. Anyway should be trivial,
> > pidfd_send_signal() has the "flags" argument.
> >
> > with Christian in https://lore.kernel.org/all/20240130112126.GA26108@redhat.com/
I missed that. Whoops.
On Wed, Jan 31, 2024 at 11:15 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> Forgot to mention...
>
> And I agree that pidfd_send_signal(flags => PGID/SID) can make
> some sense too.
>
> But this a) doesn't depend on PIDFD_THREAD, and b) needs another
> patch/discussion.
>
> But again, I am not sure I understood you correctly.
>
Hmm.
When one works with regular (non-fd) pids / pgids etc, one specifies
the signal domain at the time that one sends the signal. I don't know
what pidfds should do. It seems a bit inefficient for anything that
wants a pidfd and might send a signal in a different mode in the
future to have to hold on to multiple pidfds, so it probably should be
a pidfd_send_signal flag.
Which leaves the question of what the default should be. Should
pidfd_send_signal with flags = 0 on a PIDFD_THREAD signal the process
or the thread? I guess there are two reasonable solutions:
1. flags = 0 always means process. And maybe there's a special flag
to send a signal that matches the pidfd type, or maybe not.
2. flags = 0 does what the pidfd seems to imply, and a new
PIDFD_SIGNAL_PID flag overrides it to signal the whole PID even if the
pidfd is PIDFD_THREAD.
Do any of you have actual use cases in mind where one choice is
clearly better than the other choice?
--Andy
^ permalink raw reply
* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Christian Brauner @ 2024-01-31 19:46 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Oleg Nesterov, Tycho Andersen, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <CALCETrXTHsyiR6Bav7bXCCHny0Z2Bn90fTUL9__KTftESQ9=7w@mail.gmail.com>
On Wed, Jan 31, 2024 at 11:24:48AM -0800, Andy Lutomirski wrote:
> > On 01/31, Oleg Nesterov wrote:
> > >
> > > On 01/31, Andy Lutomirski wrote:
> > > Please note
> > >
> > > /* TODO: respect PIDFD_THREAD */
> > >
> > > this patch adds into pidfd_send_signal().
> > >
> > > See also this part of discussion
> > >
> > > > > + /* TODO: respect PIDFD_THREAD */
> > > >
> > > > So I've been thinking about this at the end of last week. Do we need to
> > > > give userspace a way to send a thread-group wide signal even when a
> > > > PIDFD_THREAD pidfd is passed? Or should we just not worry about this
> > > > right now and wait until someone needs this?
> > >
> > > I don't know. I am fine either way, but I think this needs a separate
> > > patch and another discussion in any case. Anyway should be trivial,
> > > pidfd_send_signal() has the "flags" argument.
> > >
> > > with Christian in https://lore.kernel.org/all/20240130112126.GA26108@redhat.com/
>
> I missed that. Whoops.
>
> On Wed, Jan 31, 2024 at 11:15 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > Forgot to mention...
> >
> > And I agree that pidfd_send_signal(flags => PGID/SID) can make
> > some sense too.
> >
> > But this a) doesn't depend on PIDFD_THREAD, and b) needs another
> > patch/discussion.
> >
> > But again, I am not sure I understood you correctly.
> >
>
> Hmm.
>
> When one works with regular (non-fd) pids / pgids etc, one specifies
> the signal domain at the time that one sends the signal. I don't know
> what pidfds should do. It seems a bit inefficient for anything that
> wants a pidfd and might send a signal in a different mode in the
> future to have to hold on to multiple pidfds, so it probably should be
> a pidfd_send_signal flag.
>
> Which leaves the question of what the default should be. Should
> pidfd_send_signal with flags = 0 on a PIDFD_THREAD signal the process
> or the thread? I guess there are two reasonable solutions:
>
> 1. flags = 0 always means process. And maybe there's a special flag
> to send a signal that matches the pidfd type, or maybe not.
>
> 2. flags = 0 does what the pidfd seems to imply, and a new
> PIDFD_SIGNAL_PID flag overrides it to signal the whole PID even if the
> pidfd is PIDFD_THREAD.
>
> Do any of you have actual use cases in mind where one choice is
> clearly better than the other choice?
So conceptually I think having the type of pidfd dictate the default
scope of the signal is the most elegant approach. And then very likely
we should just have:
PIDFD_SIGNAL_THREAD
PIDFD_SIGNAL_THREAD_GROUP
PIDFD_SIGNAL_PROCESS_GROUP
I think for userspace it doesn't really matter as long as we clearly
document what's going on.
Thoughts?
^ permalink raw reply
* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Andy Lutomirski @ 2024-01-31 19:50 UTC (permalink / raw)
To: Christian Brauner
Cc: Oleg Nesterov, Tycho Andersen, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <20240131-kerngesund-baumhaus-17a428b4aacb@brauner>
On Wed, Jan 31, 2024 at 11:46 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Jan 31, 2024 at 11:24:48AM -0800, Andy Lutomirski wrote:
> > > On 01/31, Oleg Nesterov wrote:
> > > >
> > > > On 01/31, Andy Lutomirski wrote:
> > > > Please note
> > > >
> > > > /* TODO: respect PIDFD_THREAD */
> > > >
> > > > this patch adds into pidfd_send_signal().
> > > >
> > > > See also this part of discussion
> > > >
> > > > > > + /* TODO: respect PIDFD_THREAD */
> > > > >
> > > > > So I've been thinking about this at the end of last week. Do we need to
> > > > > give userspace a way to send a thread-group wide signal even when a
> > > > > PIDFD_THREAD pidfd is passed? Or should we just not worry about this
> > > > > right now and wait until someone needs this?
> > > >
> > > > I don't know. I am fine either way, but I think this needs a separate
> > > > patch and another discussion in any case. Anyway should be trivial,
> > > > pidfd_send_signal() has the "flags" argument.
> > > >
> > > > with Christian in https://lore.kernel.org/all/20240130112126.GA26108@redhat.com/
> >
> > I missed that. Whoops.
> >
> > On Wed, Jan 31, 2024 at 11:15 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > Forgot to mention...
> > >
> > > And I agree that pidfd_send_signal(flags => PGID/SID) can make
> > > some sense too.
> > >
> > > But this a) doesn't depend on PIDFD_THREAD, and b) needs another
> > > patch/discussion.
> > >
> > > But again, I am not sure I understood you correctly.
> > >
> >
> > Hmm.
> >
> > When one works with regular (non-fd) pids / pgids etc, one specifies
> > the signal domain at the time that one sends the signal. I don't know
> > what pidfds should do. It seems a bit inefficient for anything that
> > wants a pidfd and might send a signal in a different mode in the
> > future to have to hold on to multiple pidfds, so it probably should be
> > a pidfd_send_signal flag.
> >
> > Which leaves the question of what the default should be. Should
> > pidfd_send_signal with flags = 0 on a PIDFD_THREAD signal the process
> > or the thread? I guess there are two reasonable solutions:
> >
> > 1. flags = 0 always means process. And maybe there's a special flag
> > to send a signal that matches the pidfd type, or maybe not.
> >
> > 2. flags = 0 does what the pidfd seems to imply, and a new
> > PIDFD_SIGNAL_PID flag overrides it to signal the whole PID even if the
> > pidfd is PIDFD_THREAD.
> >
> > Do any of you have actual use cases in mind where one choice is
> > clearly better than the other choice?
>
> So conceptually I think having the type of pidfd dictate the default
> scope of the signal is the most elegant approach. And then very likely
> we should just have:
>
> PIDFD_SIGNAL_THREAD
> PIDFD_SIGNAL_THREAD_GROUP
> PIDFD_SIGNAL_PROCESS_GROUP
>
> I think for userspace it doesn't really matter as long as we clearly
> document what's going on.
>
This seems reasonable unless we're likely to end up with a pidfd mode
that doesn't actually make sense in a send_signal context. But I'm
not immediately seeing any reason that that would happen.
--Andy
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Huang, Ying @ 2024-02-01 1:55 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: <ZbqDgaOsAeXnqRP2@memverge.com>
Gregory Price <gregory.price@memverge.com> writes:
> On Wed, Jan 31, 2024 at 05:19:51PM +0800, Huang, Ying wrote:
>> Gregory Price <gregory.price@memverge.com> writes:
>>
>> >
>> > I think this is handled already? It is definitely an explicit race
>> > condition that is documented elsewhere:
>> >
>> > /*
>> > * mpol_rebind_policy - Migrate a policy to a different set of nodes
>> > *
>> > * Per-vma policies are protected by mmap_lock. Allocations using per-task
>> > * policies are protected by task->mems_allowed_seq to prevent a premature
>> > * OOM/allocation failure due to parallel nodemask modification.
>> > */
>>
>> Thanks for pointing this out!
>>
>> If we use task->mems_allowed_seq reader side in
>> weighted_interleave_nodes() we can guarantee the consistency of
>> policy->nodes. That may be not deserved, because it's not a big deal to
>> allocate 1 page in a wrong node.
>>
>> It makes more sense to do that in
>> alloc_pages_bulk_array_weighted_interleave(), because a lot of pages may
>> be allocated there.
>>
>
> To save the versioning if there are issues, here are the 3 diffs that
> I have left. If you are good with these changes, I'll squash the first
> 2 into the third commit, keep the last one as a separate commit (it
> changes the interleave_nodes() logic too), and submit v5 w/ your
> reviewed tag on all of them.
>
>
> Fix one (pedantic?) warning from syzbot:
> ----------------------------------------
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index b1437396c357..dfd097009606 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2391,7 +2391,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> unsigned long nr_allocated = 0;
> unsigned long rounds;
> unsigned long node_pages, delta;
> - u8 __rcu *table, *weights, weight;
> + u8 __rcu *table, __rcu *weights, weight;
The __rcu usage can be checked with `sparse` directly. For example,
make C=1 mm/mempolicy.o
More details can be found in
https://www.kernel.org/doc/html/latest/dev-tools/sparse.html
Per my understanding, we shouldn't use "__rcu" here. Please search
"__rcu" in the following document.
https://www.kernel.org/doc/html/latest/RCU/checklist.html
> unsigned int weight_total = 0;
> unsigned long rem_pages = nr_pages;
> nodemask_t nodes;
>
>
>
> Simplifying resume_node/weight logic:
> -------------------------------------
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 2c1aef8eab70..b0ca9bcdd64c 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2405,15 +2405,9 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> page_array += nr_allocated;
> total_allocated += nr_allocated;
> /* if that's all the pages, no need to interleave */
> - if (rem_pages < weight) {
> - /* stay on current node, adjust il_weight */
> + if (rem_pages <= weight) {
> me->il_weight -= rem_pages;
> return total_allocated;
> - } else if (rem_pages == weight) {
> - /* move to next node / weight */
> - me->il_prev = next_node_in(node, nodes);
> - me->il_weight = get_il_weight(me->il_prev);
> - return total_allocated;
> }
> /* Otherwise we adjust remaining pages, continue from there */
> rem_pages -= weight;
> @@ -2460,17 +2454,10 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> node_pages += weight;
> delta -= weight;
> } else if (delta) {
> + /* when delta is deleted, resume from that node */
~~~~~~~
depleted?
> node_pages += delta;
> - /* delta may deplete on a boundary or w/ a remainder */
> - if (delta == weight) {
> - /* boundary: resume from next node/weight */
> - resume_node = next_node_in(node, nodes);
> - resume_weight = weights[resume_node];
> - } else {
> - /* remainder: resume this node w/ remainder */
> - resume_node = node;
> - resume_weight = weight - delta;
> - }
> + resume_node = node;
> + resume_weight = weight - delta;
> delta = 0;
> }
> /* node_pages can be 0 if an allocation fails and rounds == 0 */
>
>
>
>
>
> task->mems_allowed_seq protection (added as 4th patch)
> ------------------------------------------------------
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index b0ca9bcdd64c..b1437396c357 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1879,10 +1879,15 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
> static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> {
> unsigned int node = current->il_prev;
> + unsigned int cpuset_mems_cookie;
>
> +retry:
> + /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
> + cpuset_mems_cookie = read_mems_allowed_begin();
> if (!current->il_weight || !node_isset(node, policy->nodes)) {
> node = next_node_in(node, policy->nodes);
node will be changed in the loop. So we need to change the logic here.
> - /* can only happen if nodemask is being rebound */
> + if (read_mems_allowed_retry(cpuset_mems_cookie))
> + goto retry;
> if (node == MAX_NUMNODES)
> return node;
> current->il_prev = node;
> @@ -1896,10 +1901,17 @@ static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> static unsigned int interleave_nodes(struct mempolicy *policy)
> {
> unsigned int nid;
> + unsigned int cpuset_mems_cookie;
> +
> + /* to prevent miscount, use tsk->mems_allowed_seq to detect rebind */
> + do {
> + cpuset_mems_cookie = read_mems_allowed_begin();
> + nid = next_node_in(current->il_prev, policy->nodes);
> + } while (read_mems_allowed_retry(cpuset_mems_cookie));
>
> - nid = next_node_in(current->il_prev, policy->nodes);
> if (nid < MAX_NUMNODES)
> current->il_prev = nid;
> +
> return nid;
> }
>
> @@ -2374,6 +2386,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> struct page **page_array)
> {
> struct task_struct *me = current;
> + unsigned int cpuset_mems_cookie;
> unsigned long total_allocated = 0;
> unsigned long nr_allocated = 0;
> unsigned long rounds;
> @@ -2388,10 +2401,17 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> int prev_node;
> int i;
>
> +
Change by accident?
> if (!nr_pages)
> return 0;
>
> - nnodes = read_once_policy_nodemask(pol, &nodes);
> + /* read the nodes onto the stack, retry if done during rebind */
> + do {
> + cpuset_mems_cookie = read_mems_allowed_begin();
> + nnodes = read_once_policy_nodemask(pol, &nodes);
> + } while (read_mems_allowed_retry(cpuset_mems_cookie));
> +
> + /* if the nodemask has become invalid, we cannot do anything */
> if (!nnodes)
> return 0;
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-02-01 2:01 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: <871q9xeyo4.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Thu, Feb 01, 2024 at 09:55:07AM +0800, Huang, Ying wrote:
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index b1437396c357..dfd097009606 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -2391,7 +2391,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> > unsigned long nr_allocated = 0;
> > unsigned long rounds;
> > unsigned long node_pages, delta;
> > - u8 __rcu *table, *weights, weight;
> > + u8 __rcu *table, __rcu *weights, weight;
>
> The __rcu usage can be checked with `sparse` directly. For example,
>
> make C=1 mm/mempolicy.o
>
> More details can be found in
>
> https://www.kernel.org/doc/html/latest/dev-tools/sparse.html
>
> Per my understanding, we shouldn't use "__rcu" here. Please search
> "__rcu" in the following document.
>
> https://www.kernel.org/doc/html/latest/RCU/checklist.html
>
Thanks for this, I will sort this out and respond here with changes
before v5.
> > @@ -2460,17 +2454,10 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> > node_pages += weight;
> > delta -= weight;
> > } else if (delta) {
> > + /* when delta is deleted, resume from that node */
> ~~~~~~~
> depleted?
ack.
> > +retry:
> > + /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
> > + cpuset_mems_cookie = read_mems_allowed_begin();
> > if (!current->il_weight || !node_isset(node, policy->nodes)) {
> > node = next_node_in(node, policy->nodes);
>
> node will be changed in the loop. So we need to change the logic here.
>
Good catch, stupid mistake. ack.
> > @@ -2388,10 +2401,17 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> > int prev_node;
> > int i;
> >
> > +
>
> Change by accident?
>
ack.
~Gregory
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-02-01 2:18 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: <871q9xeyo4.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Thu, Feb 01, 2024 at 09:55:07AM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
> > - u8 __rcu *table, *weights, weight;
> > + u8 __rcu *table, __rcu *weights, weight;
>
> The __rcu usage can be checked with `sparse` directly. For example,
>
> make C=1 mm/mempolicy.o
>
fixed and squashed, all the __rcu usage i had except the global pointer
have been used. Thanks for the reference material, was struggling to
understand that.
> > task->mems_allowed_seq protection (added as 4th patch)
> > ------------------------------------------------------
> >
> > + cpuset_mems_cookie = read_mems_allowed_begin();
> > if (!current->il_weight || !node_isset(node, policy->nodes)) {
> > node = next_node_in(node, policy->nodes);
>
> node will be changed in the loop. So we need to change the logic here.
>
new patch, if it all looks good i'll ship it in v5
~Gregory
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d8cc3a577986..4e5a640d10b8 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1878,11 +1878,17 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
{
- unsigned int node = current->il_prev;
-
- if (!current->il_weight || !node_isset(node, policy->nodes)) {
- node = next_node_in(node, policy->nodes);
- /* can only happen if nodemask is being rebound */
+ unsigned int node;
+ unsigned int cpuset_mems_cookie;
+
+retry:
+ /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ if (!current->il_weight ||
+ !node_isset(current->il_prev, policy->nodes)) {
+ node = next_node_in(current->il_prev, policy->nodes);
+ if (read_mems_allowed_retry(cpuset_mems_cookie))
+ goto retry;
if (node == MAX_NUMNODES)
return node;
current->il_prev = node;
@@ -1896,8 +1902,14 @@ static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
static unsigned int interleave_nodes(struct mempolicy *policy)
{
unsigned int nid;
+ unsigned int cpuset_mems_cookie;
+
+ /* to prevent miscount, use tsk->mems_allowed_seq to detect rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nid = next_node_in(current->il_prev, policy->nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
- nid = next_node_in(current->il_prev, policy->nodes);
if (nid < MAX_NUMNODES)
current->il_prev = nid;
return nid;
@@ -2374,6 +2386,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
struct page **page_array)
{
struct task_struct *me = current;
+ unsigned int cpuset_mems_cookie;
unsigned long total_allocated = 0;
unsigned long nr_allocated = 0;
unsigned long rounds;
@@ -2391,7 +2404,13 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
if (!nr_pages)
return 0;
- nnodes = read_once_policy_nodemask(pol, &nodes);
+ /* read the nodes onto the stack, retry if done during rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nnodes = read_once_policy_nodemask(pol, &nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
+
+ /* if the nodemask has become invalid, we cannot do anything */
if (!nnodes)
return 0;
^ permalink raw reply related
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Huang, Ying @ 2024-02-01 3:02 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: <Zbr/iv3IfVqhOglE@memverge.com>
Gregory Price <gregory.price@memverge.com> writes:
> On Thu, Feb 01, 2024 at 09:55:07AM +0800, Huang, Ying wrote:
>> Gregory Price <gregory.price@memverge.com> writes:
>> > - u8 __rcu *table, *weights, weight;
>> > + u8 __rcu *table, __rcu *weights, weight;
>>
>> The __rcu usage can be checked with `sparse` directly. For example,
>>
>> make C=1 mm/mempolicy.o
>>
>
> fixed and squashed, all the __rcu usage i had except the global pointer
> have been used. Thanks for the reference material, was struggling to
> understand that.
>
>> > task->mems_allowed_seq protection (added as 4th patch)
>> > ------------------------------------------------------
>> >
>> > + cpuset_mems_cookie = read_mems_allowed_begin();
>> > if (!current->il_weight || !node_isset(node, policy->nodes)) {
>> > node = next_node_in(node, policy->nodes);
>>
>> node will be changed in the loop. So we need to change the logic here.
>>
>
> new patch, if it all looks good i'll ship it in v5
>
> ~Gregory
>
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index d8cc3a577986..4e5a640d10b8 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1878,11 +1878,17 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
>
> static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> {
> - unsigned int node = current->il_prev;
> -
> - if (!current->il_weight || !node_isset(node, policy->nodes)) {
> - node = next_node_in(node, policy->nodes);
> - /* can only happen if nodemask is being rebound */
> + unsigned int node;
IIUC, "node" may be used without initialization.
--
Best Regards,
Huang, Ying
> + unsigned int cpuset_mems_cookie;
> +
> +retry:
> + /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
> + cpuset_mems_cookie = read_mems_allowed_begin();
> + if (!current->il_weight ||
> + !node_isset(current->il_prev, policy->nodes)) {
> + node = next_node_in(current->il_prev, policy->nodes);
> + if (read_mems_allowed_retry(cpuset_mems_cookie))
> + goto retry;
> if (node == MAX_NUMNODES)
> return node;
> current->il_prev = node;
> @@ -1896,8 +1902,14 @@ static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> static unsigned int interleave_nodes(struct mempolicy *policy)
> {
> unsigned int nid;
> + unsigned int cpuset_mems_cookie;
> +
> + /* to prevent miscount, use tsk->mems_allowed_seq to detect rebind */
> + do {
> + cpuset_mems_cookie = read_mems_allowed_begin();
> + nid = next_node_in(current->il_prev, policy->nodes);
> + } while (read_mems_allowed_retry(cpuset_mems_cookie));
>
> - nid = next_node_in(current->il_prev, policy->nodes);
> if (nid < MAX_NUMNODES)
> current->il_prev = nid;
> return nid;
> @@ -2374,6 +2386,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> struct page **page_array)
> {
> struct task_struct *me = current;
> + unsigned int cpuset_mems_cookie;
> unsigned long total_allocated = 0;
> unsigned long nr_allocated = 0;
> unsigned long rounds;
> @@ -2391,7 +2404,13 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> if (!nr_pages)
> return 0;
>
> - nnodes = read_once_policy_nodemask(pol, &nodes);
> + /* read the nodes onto the stack, retry if done during rebind */
> + do {
> + cpuset_mems_cookie = read_mems_allowed_begin();
> + nnodes = read_once_policy_nodemask(pol, &nodes);
> + } while (read_mems_allowed_retry(cpuset_mems_cookie));
> +
> + /* if the nodemask has become invalid, we cannot do anything */
> if (!nnodes)
> return 0;
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-02-01 3:10 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: <87wmroevjc.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Thu, Feb 01, 2024 at 11:02:47AM +0800, Huang, Ying wrote:
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index d8cc3a577986..4e5a640d10b8 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -1878,11 +1878,17 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
> >
> > static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> > {
> > - unsigned int node = current->il_prev;
> > -
> > - if (!current->il_weight || !node_isset(node, policy->nodes)) {
> > - node = next_node_in(node, policy->nodes);
> > - /* can only happen if nodemask is being rebound */
> > + unsigned int node;
>
> IIUC, "node" may be used without initialization.
>
ok i should slow down lol. This should take care of it.
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d8cc3a577986..ed0d5d2d456a 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1878,11 +1878,17 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
{
- unsigned int node = current->il_prev;
-
- if (!current->il_weight || !node_isset(node, policy->nodes)) {
+ unsigned int node;
+ unsigned int cpuset_mems_cookie;
+
+retry:
+ /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ node = current->il_prev;
+ if (!node || !node_isset(node, policy->nodes)) {
node = next_node_in(node, policy->nodes);
- /* can only happen if nodemask is being rebound */
+ if (read_mems_allowed_retry(cpuset_mems_cookie))
+ goto retry;
if (node == MAX_NUMNODES)
return node;
current->il_prev = node;
@@ -1896,8 +1902,14 @@ static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
static unsigned int interleave_nodes(struct mempolicy *policy)
{
unsigned int nid;
+ unsigned int cpuset_mems_cookie;
+
+ /* to prevent miscount, use tsk->mems_allowed_seq to detect rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nid = next_node_in(current->il_prev, policy->nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
- nid = next_node_in(current->il_prev, policy->nodes);
if (nid < MAX_NUMNODES)
current->il_prev = nid;
return nid;
@@ -2374,6 +2386,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
struct page **page_array)
{
struct task_struct *me = current;
+ unsigned int cpuset_mems_cookie;
unsigned long total_allocated = 0;
unsigned long nr_allocated = 0;
unsigned long rounds;
@@ -2391,7 +2404,13 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
if (!nr_pages)
return 0;
- nnodes = read_once_policy_nodemask(pol, &nodes);
+ /* read the nodes onto the stack, retry if done during rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nnodes = read_once_policy_nodemask(pol, &nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
+
+ /* if the nodemask has become invalid, we cannot do anything */
if (!nnodes)
return 0;
^ permalink raw reply related
* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Christian Brauner @ 2024-02-01 13:30 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Oleg Nesterov, Tycho Andersen, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <CALCETrUh-DJ28W-LYZd3mACb4z-rmi4kmeUCitHjyufiN7U0sQ@mail.gmail.com>
On Wed, Jan 31, 2024 at 11:50:23AM -0800, Andy Lutomirski wrote:
> On Wed, Jan 31, 2024 at 11:46 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Wed, Jan 31, 2024 at 11:24:48AM -0800, Andy Lutomirski wrote:
> > > > On 01/31, Oleg Nesterov wrote:
> > > > >
> > > > > On 01/31, Andy Lutomirski wrote:
> > > > > Please note
> > > > >
> > > > > /* TODO: respect PIDFD_THREAD */
> > > > >
> > > > > this patch adds into pidfd_send_signal().
> > > > >
> > > > > See also this part of discussion
> > > > >
> > > > > > > + /* TODO: respect PIDFD_THREAD */
> > > > > >
> > > > > > So I've been thinking about this at the end of last week. Do we need to
> > > > > > give userspace a way to send a thread-group wide signal even when a
> > > > > > PIDFD_THREAD pidfd is passed? Or should we just not worry about this
> > > > > > right now and wait until someone needs this?
> > > > >
> > > > > I don't know. I am fine either way, but I think this needs a separate
> > > > > patch and another discussion in any case. Anyway should be trivial,
> > > > > pidfd_send_signal() has the "flags" argument.
> > > > >
> > > > > with Christian in https://lore.kernel.org/all/20240130112126.GA26108@redhat.com/
> > >
> > > I missed that. Whoops.
> > >
> > > On Wed, Jan 31, 2024 at 11:15 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > >
> > > > Forgot to mention...
> > > >
> > > > And I agree that pidfd_send_signal(flags => PGID/SID) can make
> > > > some sense too.
> > > >
> > > > But this a) doesn't depend on PIDFD_THREAD, and b) needs another
> > > > patch/discussion.
> > > >
> > > > But again, I am not sure I understood you correctly.
> > > >
> > >
> > > Hmm.
> > >
> > > When one works with regular (non-fd) pids / pgids etc, one specifies
> > > the signal domain at the time that one sends the signal. I don't know
> > > what pidfds should do. It seems a bit inefficient for anything that
> > > wants a pidfd and might send a signal in a different mode in the
> > > future to have to hold on to multiple pidfds, so it probably should be
> > > a pidfd_send_signal flag.
> > >
> > > Which leaves the question of what the default should be. Should
> > > pidfd_send_signal with flags = 0 on a PIDFD_THREAD signal the process
> > > or the thread? I guess there are two reasonable solutions:
> > >
> > > 1. flags = 0 always means process. And maybe there's a special flag
> > > to send a signal that matches the pidfd type, or maybe not.
> > >
> > > 2. flags = 0 does what the pidfd seems to imply, and a new
> > > PIDFD_SIGNAL_PID flag overrides it to signal the whole PID even if the
> > > pidfd is PIDFD_THREAD.
> > >
> > > Do any of you have actual use cases in mind where one choice is
> > > clearly better than the other choice?
> >
> > So conceptually I think having the type of pidfd dictate the default
> > scope of the signal is the most elegant approach. And then very likely
> > we should just have:
> >
> > PIDFD_SIGNAL_THREAD
> > PIDFD_SIGNAL_THREAD_GROUP
> > PIDFD_SIGNAL_PROCESS_GROUP
> >
> > I think for userspace it doesn't really matter as long as we clearly
> > document what's going on.
> >
>
> This seems reasonable unless we're likely to end up with a pidfd mode
> that doesn't actually make sense in a send_signal context. But I'm
> not immediately seeing any reason that that would happen.
Yeah, I think that's very unlikely and we could reject it obased on api
design considerations.
^ permalink raw reply
* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Christian Brauner @ 2024-02-01 13:39 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Oleg Nesterov, Tycho Andersen, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <20240201-laufleistung-gesessen-068ff127834d@brauner>
On Thu, Feb 01, 2024 at 02:30:46PM +0100, Christian Brauner wrote:
> On Wed, Jan 31, 2024 at 11:50:23AM -0800, Andy Lutomirski wrote:
> > On Wed, Jan 31, 2024 at 11:46 AM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Wed, Jan 31, 2024 at 11:24:48AM -0800, Andy Lutomirski wrote:
> > > > > On 01/31, Oleg Nesterov wrote:
> > > > > >
> > > > > > On 01/31, Andy Lutomirski wrote:
> > > > > > Please note
> > > > > >
> > > > > > /* TODO: respect PIDFD_THREAD */
> > > > > >
> > > > > > this patch adds into pidfd_send_signal().
> > > > > >
> > > > > > See also this part of discussion
> > > > > >
> > > > > > > > + /* TODO: respect PIDFD_THREAD */
> > > > > > >
> > > > > > > So I've been thinking about this at the end of last week. Do we need to
> > > > > > > give userspace a way to send a thread-group wide signal even when a
> > > > > > > PIDFD_THREAD pidfd is passed? Or should we just not worry about this
> > > > > > > right now and wait until someone needs this?
> > > > > >
> > > > > > I don't know. I am fine either way, but I think this needs a separate
> > > > > > patch and another discussion in any case. Anyway should be trivial,
> > > > > > pidfd_send_signal() has the "flags" argument.
> > > > > >
> > > > > > with Christian in https://lore.kernel.org/all/20240130112126.GA26108@redhat.com/
> > > >
> > > > I missed that. Whoops.
> > > >
> > > > On Wed, Jan 31, 2024 at 11:15 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > > >
> > > > > Forgot to mention...
> > > > >
> > > > > And I agree that pidfd_send_signal(flags => PGID/SID) can make
> > > > > some sense too.
> > > > >
> > > > > But this a) doesn't depend on PIDFD_THREAD, and b) needs another
> > > > > patch/discussion.
> > > > >
> > > > > But again, I am not sure I understood you correctly.
> > > > >
> > > >
> > > > Hmm.
> > > >
> > > > When one works with regular (non-fd) pids / pgids etc, one specifies
> > > > the signal domain at the time that one sends the signal. I don't know
> > > > what pidfds should do. It seems a bit inefficient for anything that
> > > > wants a pidfd and might send a signal in a different mode in the
> > > > future to have to hold on to multiple pidfds, so it probably should be
> > > > a pidfd_send_signal flag.
> > > >
> > > > Which leaves the question of what the default should be. Should
> > > > pidfd_send_signal with flags = 0 on a PIDFD_THREAD signal the process
> > > > or the thread? I guess there are two reasonable solutions:
> > > >
> > > > 1. flags = 0 always means process. And maybe there's a special flag
> > > > to send a signal that matches the pidfd type, or maybe not.
> > > >
> > > > 2. flags = 0 does what the pidfd seems to imply, and a new
> > > > PIDFD_SIGNAL_PID flag overrides it to signal the whole PID even if the
> > > > pidfd is PIDFD_THREAD.
> > > >
> > > > Do any of you have actual use cases in mind where one choice is
> > > > clearly better than the other choice?
> > >
> > > So conceptually I think having the type of pidfd dictate the default
> > > scope of the signal is the most elegant approach. And then very likely
> > > we should just have:
> > >
> > > PIDFD_SIGNAL_THREAD
> > > PIDFD_SIGNAL_THREAD_GROUP
> > > PIDFD_SIGNAL_PROCESS_GROUP
> > >
> > > I think for userspace it doesn't really matter as long as we clearly
> > > document what's going on.
> > >
> >
> > This seems reasonable unless we're likely to end up with a pidfd mode
> > that doesn't actually make sense in a send_signal context. But I'm
> > not immediately seeing any reason that that would happen.
>
> Yeah, I think that's very unlikely and we could reject it obased on api
> design considerations.
Ah, forgot to ask. Did you intend to send a patch for this?
^ permalink raw reply
* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Andy Lutomirski @ 2024-02-01 19:33 UTC (permalink / raw)
To: Christian Brauner
Cc: Oleg Nesterov, Tycho Andersen, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <20240201-flugzeit-modeschau-dab703fc8b6d@brauner>
On Thu, Feb 1, 2024 at 5:39 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, Feb 01, 2024 at 02:30:46PM +0100, Christian Brauner wrote:
> > On Wed, Jan 31, 2024 at 11:50:23AM -0800, Andy Lutomirski wrote:
> > > On Wed, Jan 31, 2024 at 11:46 AM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > On Wed, Jan 31, 2024 at 11:24:48AM -0800, Andy Lutomirski wrote:
> > > > > > On 01/31, Oleg Nesterov wrote:
> > > > > > >
> > > > > > > On 01/31, Andy Lutomirski wrote:
> > > > > > > Please note
> > > > > > >
> > > > > > > /* TODO: respect PIDFD_THREAD */
> > > > > > >
> > > > > > > this patch adds into pidfd_send_signal().
> > > > > > >
> > > > > > > See also this part of discussion
> > > > > > >
> > > > > > > > > + /* TODO: respect PIDFD_THREAD */
> > > > > > > >
> > > > > > > > So I've been thinking about this at the end of last week. Do we need to
> > > > > > > > give userspace a way to send a thread-group wide signal even when a
> > > > > > > > PIDFD_THREAD pidfd is passed? Or should we just not worry about this
> > > > > > > > right now and wait until someone needs this?
> > > > > > >
> > > > > > > I don't know. I am fine either way, but I think this needs a separate
> > > > > > > patch and another discussion in any case. Anyway should be trivial,
> > > > > > > pidfd_send_signal() has the "flags" argument.
> > > > > > >
> > > > > > > with Christian in https://lore.kernel.org/all/20240130112126.GA26108@redhat.com/
> > > > >
> > > > > I missed that. Whoops.
> > > > >
> > > > > On Wed, Jan 31, 2024 at 11:15 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > > > >
> > > > > > Forgot to mention...
> > > > > >
> > > > > > And I agree that pidfd_send_signal(flags => PGID/SID) can make
> > > > > > some sense too.
> > > > > >
> > > > > > But this a) doesn't depend on PIDFD_THREAD, and b) needs another
> > > > > > patch/discussion.
> > > > > >
> > > > > > But again, I am not sure I understood you correctly.
> > > > > >
> > > > >
> > > > > Hmm.
> > > > >
> > > > > When one works with regular (non-fd) pids / pgids etc, one specifies
> > > > > the signal domain at the time that one sends the signal. I don't know
> > > > > what pidfds should do. It seems a bit inefficient for anything that
> > > > > wants a pidfd and might send a signal in a different mode in the
> > > > > future to have to hold on to multiple pidfds, so it probably should be
> > > > > a pidfd_send_signal flag.
> > > > >
> > > > > Which leaves the question of what the default should be. Should
> > > > > pidfd_send_signal with flags = 0 on a PIDFD_THREAD signal the process
> > > > > or the thread? I guess there are two reasonable solutions:
> > > > >
> > > > > 1. flags = 0 always means process. And maybe there's a special flag
> > > > > to send a signal that matches the pidfd type, or maybe not.
> > > > >
> > > > > 2. flags = 0 does what the pidfd seems to imply, and a new
> > > > > PIDFD_SIGNAL_PID flag overrides it to signal the whole PID even if the
> > > > > pidfd is PIDFD_THREAD.
> > > > >
> > > > > Do any of you have actual use cases in mind where one choice is
> > > > > clearly better than the other choice?
> > > >
> > > > So conceptually I think having the type of pidfd dictate the default
> > > > scope of the signal is the most elegant approach. And then very likely
> > > > we should just have:
> > > >
> > > > PIDFD_SIGNAL_THREAD
> > > > PIDFD_SIGNAL_THREAD_GROUP
> > > > PIDFD_SIGNAL_PROCESS_GROUP
> > > >
> > > > I think for userspace it doesn't really matter as long as we clearly
> > > > document what's going on.
> > > >
> > >
> > > This seems reasonable unless we're likely to end up with a pidfd mode
> > > that doesn't actually make sense in a send_signal context. But I'm
> > > not immediately seeing any reason that that would happen.
> >
> > Yeah, I think that's very unlikely and we could reject it obased on api
> > design considerations.
>
> Ah, forgot to ask. Did you intend to send a patch for this?
I can try to get to it tomorrow. Currently trying to madly line up a
whole bunch of stuff in time for a maintenance window.
^ permalink raw reply
* Re: [net-next 0/3] Per epoll context busy poll support
From: Joe Damato @ 2024-02-02 3:28 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: Eric Dumazet, netdev, linux-kernel, chuck.lever, jlayton,
linux-api, brauner, davem, alexander.duyck, kuba, Wei Wang,
Amritha Nambiar
In-Reply-To: <5faf88de-5063-421f-ad78-ad24d931fd17@intel.com>
On Tue, Jan 30, 2024 at 12:54:50PM -0600, Samudrala, Sridhar wrote:
>
>
> On 1/24/2024 2:20 AM, Eric Dumazet wrote:
> >On Wed, Jan 24, 2024 at 3:54 AM Joe Damato <jdamato@fastly.com> wrote:
> >>
> >>Greetings:
> >>
> >>TL;DR This builds on commit bf3b9f6372c4 ("epoll: Add busy poll support to
> >>epoll with socket fds.") by allowing user applications to enable
> >>epoll-based busy polling and set a busy poll packet budget on a per epoll
> >>context basis.
> >>
> >>To allow for this, two ioctls have been added for epoll contexts for
> >>getting and setting a new struct, struct epoll_params.
> >>
> >>This makes epoll-based busy polling much more usable for user
> >>applications than the current system-wide sysctl and hardcoded budget.
>
> Agree. looking forward to see this patch series accepted soon.
>
> >>
> >>Longer explanation:
> >>
> >>Presently epoll has support for a very useful form of busy poll based on
> >>the incoming NAPI ID (see also: SO_INCOMING_NAPI_ID [1]).
> >>
> >>This form of busy poll allows epoll_wait to drive NAPI packet processing
> >>which allows for a few interesting user application designs which can
> >>reduce latency and also potentially improve L2/L3 cache hit rates by
> >>deferring NAPI until userland has finished its work.
> >>
> >>The documentation available on this is, IMHO, a bit confusing so please
> >>allow me to explain how one might use this:
> >>
> >>1. Ensure each application thread has its own epoll instance mapping
> >>1-to-1 with NIC RX queues. An n-tuple filter would likely be used to
> >>direct connections with specific dest ports to these queues.
> >>
> >>2. Optionally: Setup IRQ coalescing for the NIC RX queues where busy
> >>polling will occur. This can help avoid the userland app from being
> >>pre-empted by a hard IRQ while userland is running. Note this means that
> >>userland must take care to call epoll_wait and not take too long in
> >>userland since it now drives NAPI via epoll_wait.
> >>
> >>3. Ensure that all incoming connections added to an epoll instance
> >>have the same NAPI ID. This can be done with a BPF filter when
> >>SO_REUSEPORT is used or getsockopt + SO_INCOMING_NAPI_ID when a single
> >>accept thread is used which dispatches incoming connections to threads.
> >>
> >>4. Lastly, busy poll must be enabled via a sysctl
> >>(/proc/sys/net/core/busy_poll).
> >>
> >>The unfortunate part about step 4 above is that this enables busy poll
> >>system-wide which affects all user applications on the system,
> >>including epoll-based network applications which were not intended to
> >>be used this way or applications where increased CPU usage for lower
> >>latency network processing is unnecessary or not desirable.
> >>
> >>If the user wants to run one low latency epoll-based server application
> >>with epoll-based busy poll, but would like to run the rest of the
> >>applications on the system (which may also use epoll) without busy poll,
> >>this system-wide sysctl presents a significant problem.
> >>
> >>This change preserves the system-wide sysctl, but adds a mechanism (via
> >>ioctl) to enable or disable busy poll for epoll contexts as needed by
> >>individual applications, making epoll-based busy poll more usable.
> >>
> >
> >I think this description missed the napi_defer_hard_irqs and
> >gro_flush_timeout settings ?
> >
> >I would think that if an application really wants to make sure its
> >thread is the only one
> >eventually calling napi->poll(), we must make sure NIC interrupts stay masked.
> >
> >Current implementations of busy poll always release NAPI_STATE_SCHED bit when
> >returning to user space.
> >
> >It seems you want to make sure the application and only the
> >application calls the napi->poll()
> >at chosen times.
> >
> >Some kind of contract is needed, and the presence of the hrtimer
> >(currently only driven from dev->@gro_flush_timeout)
> >would allow to do that correctly.
> >
> >Whenever we 'trust' user space to perform the napi->poll shortly, we
> >also want to arm the hrtimer to eventually detect
> >the application took too long, to restart the other mechanisms (NIC irq based)
> >
> >Note that we added the kthread based napi polling, and we are working
> >to add a busy polling feature to these kthreads.
> >allowing to completely mask NIC interrupts and further reduce latencies.
>
>
> Good to know that you are looking into enabling busy polling for napi
> kthreads.
> We have something similar in our ice OOT driver that is implemented and we
> call it 'independent pollers' as in this mode, busy polling will not be app
> dependent or triggered by an application.
> Here is a link to the slides we presented at netdev 0x16 driver workshop.
> https://netdevconf.info/0x16/slides/48/netdev0x16_driver_workshop_ADQ.pdf
>
> We haven't yet submitted the patches upstream as there is no kernel
> interface to configure napi specific timeouts.
> With the recent per-queue and per-napi netlink APIs that are accepted
> upstream
> https://lore.kernel.org/netdev/170147307026.5260.9300080745237900261.stgit@anambiarhost.jf.intel.com/
>
> we are thinking of making timeout as a per-napi parameter and can be used as
> an interface to enable napi kthread based busy polling.
I know I am replying to a stale thread on the patches I've submit (there is
a v5 now [1]), but I just looked at your message - sorry I didn't reply
sooner.
The per-queue and per-napi netlink APIs look extremely useful, thanks for
pointing this out.
In my development tree, I had added SIOCGIFNAME_BY_NAPI_ID which works
similar to SIOCGIFNAME: it takes a NAPI ID and returns the IF name. This is
useful on machines with multiple NICs where each NIC could be located in
one of many different NUMA zones.
The idea was that apps would use SO_INCOMING_NAPI_ID, distribute the NAPI
ID to a worker thread which could then use SIOCGIFNAME_BY_NAPI_ID to
compute which NIC the connection came in on. The app would then (via
configuration) know where to pin that worker thread; ideally somewhere NUMA
local to the NIC.
I had assumed that such a change would be rejected, but I figured I'd send
an RFC for it after the per epoll context stuff was done and see if anyone
thought SIOCGIFNAME_BY_NAPI_ID would be useful for them, as well.
> I think even the per-device napi_defer_hard_irqs and gro_flush_timeout
> should become per-napi parameters.
I agree.
I had been contemplating implementing this until I tried a different method
similar to an academic paper I was reading [2][3]. I think per-device
defer_hard_irqs and gro_flush_timeout would be extremely useful and a
better approach than the one I'm currently using.
Is this something you are currently working? I may try implementing this,
but didn't want to duplicate effort if you are already working on this.
Thanks,
Joe
[1]: https://lore.kernel.org/all/20240131180811.23566-1-jdamato@fastly.com/
[2]: https://dl.acm.org/doi/pdf/10.1145/3626780
[3]: https://gitlab.uwaterloo.ca/p5cai/netstack-exp/-/blob/master/kernel-polling-5.15.79-base.patch?ref_type=heads
^ permalink raw reply
* [PATCH 0/3] pidfd_poll: report POLLHUP when pid_task() == NULL
From: Oleg Nesterov @ 2024-02-02 13:11 UTC (permalink / raw)
To: Christian Brauner
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
3/3 is the unrelated "while at it" change.
Oleg.
^ permalink raw reply
* [PATCH 1/3] pidfd_poll: report POLLHUP when pid_task() == NULL
From: Oleg Nesterov @ 2024-02-02 13:12 UTC (permalink / raw)
To: Christian Brauner
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
In-Reply-To: <20240202131147.GA25988@redhat.com>
Add another wake_up_all(wait_pidfd) into __change_pid() and change
pidfd_poll() to include EPOLLHUP if task == NULL.
This allows to wait until the target process/thread is reaped.
TODO: change do_notify_pidfd() to use the keyed wakeups.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/fork.c | 22 +++++++---------------
kernel/pid.c | 5 +++++
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index b8c6ec9a08dd..8d08a2d1b095 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2071,20 +2071,6 @@ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
-static bool pidfd_task_exited(struct pid *pid, bool thread)
-{
- struct task_struct *task;
- bool exited;
-
- rcu_read_lock();
- task = pid_task(pid, PIDTYPE_PID);
- exited = !task ||
- (READ_ONCE(task->exit_state) && (thread || thread_group_empty(task)));
- rcu_read_unlock();
-
- return exited;
-}
-
/*
* Poll support for process exit notification.
*/
@@ -2092,6 +2078,7 @@ static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts)
{
struct pid *pid = file->private_data;
bool thread = file->f_flags & PIDFD_THREAD;
+ struct task_struct *task;
__poll_t poll_flags = 0;
poll_wait(file, &pid->wait_pidfd, pts);
@@ -2099,8 +2086,13 @@ static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts)
* Depending on PIDFD_THREAD, inform pollers when the thread
* or the whole thread-group exits.
*/
- if (pidfd_task_exited(pid, thread))
+ rcu_read_lock();
+ task = pid_task(pid, PIDTYPE_PID);
+ if (!task)
+ poll_flags = EPOLLIN | EPOLLRDNORM | EPOLLHUP;
+ else if (task->exit_state && (thread || thread_group_empty(task)))
poll_flags = EPOLLIN | EPOLLRDNORM;
+ rcu_read_unlock();
return poll_flags;
}
diff --git a/kernel/pid.c b/kernel/pid.c
index e11144466828..62461c7c82b8 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -349,6 +349,11 @@ static void __change_pid(struct task_struct *task, enum pid_type type,
hlist_del_rcu(&task->pid_links[type]);
*pid_ptr = new;
+ if (type == PIDTYPE_PID) {
+ WARN_ON_ONCE(pid_has_task(pid, PIDTYPE_PID));
+ wake_up_all(&pid->wait_pidfd);
+ }
+
for (tmp = PIDTYPE_MAX; --tmp >= 0; )
if (pid_has_task(pid, tmp))
return;
--
2.25.1.362.g51ebf55
^ permalink raw reply related
* [PATCH 2/3] pidfd: kill the no longer needed do_notify_pidfd() in de_thread()
From: Oleg Nesterov @ 2024-02-02 13:12 UTC (permalink / raw)
To: Christian Brauner
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
In-Reply-To: <20240202131147.GA25988@redhat.com>
Now that __change_pid() does wake_up_all(&pid->wait_pidfd) we can kill
do_notify_pidfd(leader) in de_thread(), it calls release_task(leader)
right after that and this implies detach_pid(leader, PIDTYPE_PID).
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/exec.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 0fd7e668c477..acd466f92998 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1143,11 +1143,6 @@ static int de_thread(struct task_struct *tsk)
BUG_ON(leader->exit_state != EXIT_ZOMBIE);
leader->exit_state = EXIT_DEAD;
- /*
- * leader and tsk exhanged their pids, the old pid dies,
- * wake up the PIDFD_THREAD waiters.
- */
- do_notify_pidfd(leader);
/*
* We are going to release_task()->ptrace_unlink() silently,
* the tracer can sleep in do_wait(). EXIT_DEAD guarantees
--
2.25.1.362.g51ebf55
^ permalink raw reply related
* [PATCH 3/3] pid: kill the obsolete PIDTYPE_PID code in transfer_pid()
From: Oleg Nesterov @ 2024-02-02 13:12 UTC (permalink / raw)
To: Christian Brauner
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
In-Reply-To: <20240202131147.GA25988@redhat.com>
transfer_pid() must be never called with pid == PIDTYPE_PID,
new_leader->thread_pid should be changed by exchange_tids().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/pid.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/pid.c b/kernel/pid.c
index 62461c7c82b8..de0bf2f8d18b 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -396,8 +396,7 @@ void exchange_tids(struct task_struct *left, struct task_struct *right)
void transfer_pid(struct task_struct *old, struct task_struct *new,
enum pid_type type)
{
- if (type == PIDTYPE_PID)
- new->thread_pid = old->thread_pid;
+ WARN_ON_ONCE(type == PIDTYPE_PID);
hlist_replace_rcu(&old->pid_links[type], &new->pid_links[type]);
}
--
2.25.1.362.g51ebf55
^ permalink raw reply related
* Re: [PATCH 1/3] pidfd_poll: report POLLHUP when pid_task() == NULL
From: Christian Brauner @ 2024-02-02 14:44 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
In-Reply-To: <20240202131226.GA26018@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 104 bytes --]
> TODO: change do_notify_pidfd() to use the keyed wakeups.
How does the following appended patch look?
[-- Attachment #2: 0001-pidfd-convert-to-wake_up_poll.patch --]
[-- Type: text/x-diff, Size: 3286 bytes --]
From d8ef35f3f151a758cb2503a51dfaf7263480cfbd Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Fri, 2 Feb 2024 15:18:16 +0100
Subject: [PATCH] pidfd: convert to wake_up_poll()
* Rename do_notify_pidfd() pidfd_wake_up_poll()
* Pass in a poll mask to enable epoll to avoid spurious wakeups
* Use poll_table typedef
* Warn if caller doesn't pass in appropriate poll mask
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
include/linux/pid.h | 2 +-
kernel/exit.c | 2 +-
kernel/fork.c | 4 ++--
kernel/signal.c | 10 ++++------
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index 8124d57752b9..a3666b2ae877 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -74,7 +74,7 @@ struct pid *pidfd_pid(const struct file *file);
struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags);
struct task_struct *pidfd_get_task(int pidfd, unsigned int *flags);
int pidfd_prepare(struct pid *pid, unsigned int flags, struct file **ret);
-void do_notify_pidfd(struct task_struct *task);
+void pidfd_wake_up_poll(struct task_struct *task, __poll_t mask);
static inline struct pid *get_pid(struct pid *pid)
{
diff --git a/kernel/exit.c b/kernel/exit.c
index c038d10dfb38..b4b39709b046 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -744,7 +744,7 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
* PIDFD_THREAD waiters.
*/
if (!thread_group_empty(tsk))
- do_notify_pidfd(tsk);
+ pidfd_wake_up_poll(tsk, EPOLLIN | EPOLLRDNORM | EPOLLHUP);
if (unlikely(tsk->ptrace)) {
int sig = thread_group_leader(tsk) &&
diff --git a/kernel/fork.c b/kernel/fork.c
index aa08193d124f..7b882e66448b 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2074,14 +2074,14 @@ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f)
/*
* Poll support for process exit notification.
*/
-static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts)
+static __poll_t pidfd_poll(struct file *file, poll_table *wait)
{
struct pid *pid = file->private_data;
bool thread = file->f_flags & PIDFD_THREAD;
struct task_struct *task;
__poll_t poll_flags = 0;
- poll_wait(file, &pid->wait_pidfd, pts);
+ poll_wait(file, &pid->wait_pidfd, wait);
/*
* Depending on PIDFD_THREAD, inform pollers when the thread
* or the whole thread-group exits.
diff --git a/kernel/signal.c b/kernel/signal.c
index 9b40109f0c56..ef330a7f1b51 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2019,13 +2019,11 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type)
return ret;
}
-void do_notify_pidfd(struct task_struct *task)
+void pidfd_wake_up_poll(struct task_struct *task, __poll_t mask)
{
- struct pid *pid;
-
WARN_ON(task->exit_state == 0);
- pid = task_pid(task);
- wake_up_all(&pid->wait_pidfd);
+ WARN_ON(mask == 0);
+ wake_up_poll(&task_pid(task)->wait_pidfd, mask);
}
/*
@@ -2055,7 +2053,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
* non-PIDFD_THREAD waiters.
*/
if (thread_group_empty(tsk))
- do_notify_pidfd(tsk);
+ pidfd_wake_up_poll(tsk, EPOLLIN | EPOLLRDNORM);
if (sig != SIGCHLD) {
/*
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 1/3] pidfd_poll: report POLLHUP when pid_task() == NULL
From: Christian Brauner @ 2024-02-02 15:16 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
In-Reply-To: <20240202-arbeit-fruchtig-26880564a21a@brauner>
On Fri, Feb 02, 2024 at 03:44:45PM +0100, Christian Brauner wrote:
> > TODO: change do_notify_pidfd() to use the keyed wakeups.
>
> How does the following appended patch look?
I missed the conversion in detach_pid() which should also just become:
diff --git a/kernel/pid.c b/kernel/pid.c
index de0bf2f8d18b..1bfcfa195226 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -351,7 +351,8 @@ static void __change_pid(struct task_struct *task, enum pid_type type,
if (type == PIDTYPE_PID) {
WARN_ON_ONCE(pid_has_task(pid, PIDTYPE_PID));
- wake_up_all(&pid->wait_pidfd);
+ pidfd_wake_up_poll(&pid->wait_pidfd,
+ EPOLLIN | EPOLLRDNORM | EPOLLHUP);
}
for (tmp = PIDTYPE_MAX; --tmp >= 0; )
^ permalink raw reply related
* Re: [PATCH 0/3] pidfd_poll: report POLLHUP when pid_task() == NULL
From: Christian Brauner @ 2024-02-02 15:49 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Christian Brauner, Andy Lutomirski, Eric W. Biederman,
Tycho Andersen, linux-api, linux-kernel
In-Reply-To: <20240202131147.GA25988@redhat.com>
On Fri, 02 Feb 2024 14:11:47 +0100, Oleg Nesterov wrote:
> 3/3 is the unrelated "while at it" change.
>
> Oleg.
>
Applied to the vfs.pidfd branch of the vfs/vfs.git tree.
Patches in the vfs.pidfd 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.pidfd
[1/3] pidfd_poll: report POLLHUP when pid_task() == NULL
https://git.kernel.org/vfs/vfs/c/0ab0783b0160
[2/3] pidfd: kill the no longer needed do_notify_pidfd() in de_thread()
https://git.kernel.org/vfs/vfs/c/afe79af3b522
[3/3] pid: kill the obsolete PIDTYPE_PID code in transfer_pid()
https://git.kernel.org/vfs/vfs/c/082d11c164ae
^ permalink raw reply
* Re: [PATCH 1/3] pidfd_poll: report POLLHUP when pid_task() == NULL
From: Oleg Nesterov @ 2024-02-02 16:07 UTC (permalink / raw)
To: Christian Brauner
Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
linux-kernel
In-Reply-To: <20240202-arbeit-fruchtig-26880564a21a@brauner>
On 02/02, Christian Brauner wrote:
>
> > TODO: change do_notify_pidfd() to use the keyed wakeups.
>
> How does the following appended patch look?
No, no.
I think we need a simpler patch. I was going to send it as 4/4, but I'd
like to think more, _perhaps_ we can also discriminate the PIDFD_THREAD
and non-PIDFD_THREAD waiters. I'll try to make the patch(es) tomorrow or
at least provided more info.
3 notes for now:
1. we can't use wake_up_poll(), it passes nr_exclusive => 1
2. exit_notify() should not pass EPOLLHUP to wake_up, we do
not want to wake up the { .events = POLLHUP } waiters.
3. we do not need to change __change_pid().
Well, _perhaps_ it can/should use __wake_up_pollfree(), but
I need to check if fs/select.c use "autoremove" or not.
> -static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts)
> +static __poll_t pidfd_poll(struct file *file, poll_table *wait)
> {
> struct pid *pid = file->private_data;
> bool thread = file->f_flags & PIDFD_THREAD;
> struct task_struct *task;
> __poll_t poll_flags = 0;
>
> - poll_wait(file, &pid->wait_pidfd, pts);
> + poll_wait(file, &pid->wait_pidfd, wait);
This is correct but only cosemtic and has nothing to do with what
we discuss?
Oleg.
^ permalink raw reply
* [PATCH v5 0/4] mm/mempolicy: weighted interleave mempolicy and sysfs extension
From: Gregory Price @ 2024-02-02 17:02 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linux-fsdevel, linux-api, corbet, akpm,
gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji, mhocko,
ying.huang, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Hasan Al Maruf, Hao Wang, Michal Hocko, Zhongkun He,
Frank van der Linden, John Groves, Jonathan Cameron, Andi Kleen
(v5: style, retry interleave w/ mems_allowed cookie
fix sparse warnings, style, review tags)
-----
Weighted interleave is a new interleave policy intended to make
use of heterogeneous memory environments appearing with CXL.
The existing interleave mechanism does an even round-robin
distribution of memory across all nodes in a nodemask, while
weighted interleave distributes memory across nodes according
to a provided weight. (Weight = # of page allocations per round)
Weighted interleave is intended to reduce average latency when
bandwidth is pressured - therefore increasing total throughput.
In other words: It allows greater use of the total available
bandwidth in a heterogeneous hardware environment (different
hardware provides different bandwidth capacity).
As bandwidth is pressured, latency increases - first linearly
and then exponentially. By keeping bandwidth usage distributed
according to available bandwidth, we therefore can reduce the
average latency of a cacheline fetch.
A good explanation of the bandwidth vs latency response curve:
https://mahmoudhatem.wordpress.com/2017/11/07/memory-bandwidth-vs-latency-response-curve/
From the article:
```
Constant region:
The latency response is fairly constant for the first 40%
of the sustained bandwidth.
Linear region:
In between 40% to 80% of the sustained bandwidth, the
latency response increases almost linearly with the bandwidth
demand of the system due to contention overhead by numerous
memory requests.
Exponential region:
Between 80% to 100% of the sustained bandwidth, the memory
latency is dominated by the contention latency which can be
as much as twice the idle latency or more.
Maximum sustained bandwidth :
Is 65% to 75% of the theoretical maximum bandwidth.
```
As a general rule of thumb:
* If bandwidth usage is low, latency does not increase. It is
optimal to place data in the nearest (lowest latency) device.
* If bandwidth usage is high, latency increases. It is optimal
to place data such that bandwidth use is optimized per-device.
This is the top line goal: Provide a user a mechanism to target using
the "maximum sustained bandwidth" of each hardware component in a
heterogenous memory system.
For example, the stream benchmark demonstrates that 1:1 (default)
interleave is actively harmful, while weighted interleave can be
beneficial. Default interleave distributes data such that too much
pressure is placed on devices with lower available bandwidth.
Stream Benchmark (vs DRAM, 1 Socket + 1 CXL Device)
Default interleave : -78% (slower than DRAM)
Global weighting : -6% to +4% (workload dependant)
Targeted weights : +2.5% to +4% (consistently better than DRAM)
Global means the task-policy was set (set_mempolicy), while targeted
means VMA policies were set (mbind2). We see weighted interleave
is not always beneficial when applied globally, but is always
beneficial when applied to bandwidth-driving memory regions.
There are 4 patches in this set:
1) Implement system-global interleave weights as sysfs extension
in mm/mempolicy.c. These weights are RCU protected, and a
default weight set is provided (all weights are 1 by default).
In future work, we intend to expose an interface for HMAT/CDAT
code to set reasonable default values based on the memory
configuration of the system discovered at boot/hotplug.
2) A mild refactor of some interleave-logic for re-use in the
new weighted interleave logic.
3) MPOL_WEIGHTED_INTERLEAVE extension for set_mempolicy/mbind
4) Protect interleave logic (weighted and normal) with the
mems_allowed seq cookie. If the nodemask changes while
accessing it during a rebind, just retry the access.
Included below are some performance and LTP test information,
and a sample numactl branch which can be used for testing.
= Performance summary =
(tests may have different configurations, see extended info below)
1) MLC (W2) : +38% over DRAM. +264% over default interleave.
MLC (W5) : +40% over DRAM. +226% over default interleave.
2) Stream : -6% to +4% over DRAM, +430% over default interleave.
3) XSBench : +19% over DRAM. +47% over default interleave.
= LTP Testing Summary =
existing mempolicy & mbind tests: pass
mempolicy & mbind + weighted interleave (global weights): pass
= version history
v5:
- style fixes
- mems_allowed cookie protection to detect rebind issues,
prevents spurious allocation failures and/or mis-allocations
- sparse warning fixes related to __rcu on local variables
=====================================================================
Performance tests - MLC
From - Ravi Jonnalagadda <ravis.opensrc@micron.com>
Hardware: Single-socket, multiple CXL memory expanders.
Workload: W2
Data Signature: 2:1 read:write
DRAM only bandwidth (GBps): 298.8
DRAM + CXL (default interleave) (GBps): 113.04
DRAM + CXL (weighted interleave)(GBps): 412.5
Gain over DRAM only: 1.38x
Gain over default interleave: 2.64x
Workload: W5
Data Signature: 1:1 read:write
DRAM only bandwidth (GBps): 273.2
DRAM + CXL (default interleave) (GBps): 117.23
DRAM + CXL (weighted interleave)(GBps): 382.7
Gain over DRAM only: 1.4x
Gain over default interleave: 2.26x
=====================================================================
Performance test - Stream
From - Gregory Price <gregory.price@memverge.com>
Hardware: Single socket, single CXL expander
numactl extension: https://github.com/gmprice/numactl/tree/weighted_interleave_master
Summary: 64 threads, ~18GB workload, 3GB per array, executed 100 times
Default interleave : -78% (slower than DRAM)
Global weighting : -6% to +4% (workload dependant)
mbind2 weights : +2.5% to +4% (consistently better than DRAM)
dram only:
numactl --cpunodebind=1 --membind=1 ./stream_c.exe --ntimes 100 --array-size 400M --malloc
Function Direction BestRateMBs AvgTime MinTime MaxTime
Copy: 0->0 200923.2 0.032662 0.031853 0.033301
Scale: 0->0 202123.0 0.032526 0.031664 0.032970
Add: 0->0 208873.2 0.047322 0.045961 0.047884
Triad: 0->0 208523.8 0.047262 0.046038 0.048414
CXL-only:
numactl --cpunodebind=1 -w --membind=2 ./stream_c.exe --ntimes 100 --array-size 400M --malloc
Copy: 0->0 22209.7 0.288661 0.288162 0.289342
Scale: 0->0 22288.2 0.287549 0.287147 0.288291
Add: 0->0 24419.1 0.393372 0.393135 0.393735
Triad: 0->0 24484.6 0.392337 0.392083 0.394331
Based on the above, the optimal weights are ~9:1
echo 9 > /sys/kernel/mm/mempolicy/weighted_interleave/node1
echo 1 > /sys/kernel/mm/mempolicy/weighted_interleave/node2
default interleave:
numactl --cpunodebind=1 --interleave=1,2 ./stream_c.exe --ntimes 100 --array-size 400M --malloc
Copy: 0->0 44666.2 0.143671 0.143285 0.144174
Scale: 0->0 44781.6 0.143256 0.142916 0.143713
Add: 0->0 48600.7 0.197719 0.197528 0.197858
Triad: 0->0 48727.5 0.197204 0.197014 0.197439
global weighted interleave:
numactl --cpunodebind=1 -w --interleave=1,2 ./stream_c.exe --ntimes 100 --array-size 400M --malloc
Copy: 0->0 190085.9 0.034289 0.033669 0.034645
Scale: 0->0 207677.4 0.031909 0.030817 0.033061
Add: 0->0 202036.8 0.048737 0.047516 0.053409
Triad: 0->0 217671.5 0.045819 0.044103 0.046755
targted regions w/ global weights (modified stream to mbind2 malloc'd regions))
numactl --cpunodebind=1 --membind=1 ./stream_c.exe -b --ntimes 100 --array-size 400M --malloc
Copy: 0->0 205827.0 0.031445 0.031094 0.031984
Scale: 0->0 208171.8 0.031320 0.030744 0.032505
Add: 0->0 217352.0 0.045087 0.044168 0.046515
Triad: 0->0 216884.8 0.045062 0.044263 0.046982
=====================================================================
Performance tests - XSBench
From - Hyeongtak Ji <hyeongtak.ji@sk.com>
Hardware: Single socket, Single CXL memory Expander
NUMA node 0: 56 logical cores, 128 GB memory
NUMA node 2: 96 GB CXL memory
Threads: 56
Lookups: 170,000,000
Summary: +19% over DRAM. +47% over default interleave.
Performance tests - XSBench
1. dram only
$ numactl -m 0 ./XSBench -s XL –p 5000000
Runtime: 36.235 seconds
Lookups/s: 4,691,618
2. default interleave
$ numactl –i 0,2 ./XSBench –s XL –p 5000000
Runtime: 55.243 seconds
Lookups/s: 3,077,293
3. weighted interleave
numactl –w –i 0,2 ./XSBench –s XL –p 5000000
Runtime: 29.262 seconds
Lookups/s: 5,809,513
=====================================================================
LTP Tests: https://github.com/gmprice/ltp/tree/mempolicy2
= Existing tests
set_mempolicy, get_mempolicy, mbind
MPOL_WEIGHTED_INTERLEAVE added manually to test basic functionality
but did not adjust tests for weighting. Basically the weights were
set to 1, which is the default, and it should behave the same as
MPOL_INTERLEAVE if logic is correct.
== set_mempolicy01 : passed 18, failed 0
== set_mempolicy02 : passed 10, failed 0
== set_mempolicy03 : passed 64, failed 0
== set_mempolicy04 : passed 32, failed 0
== set_mempolicy05 - n/a on non-x86
== set_mempolicy06 : passed 10, failed 0
this is set_mempolicy02 + MPOL_WEIGHTED_INTERLEAVE
== set_mempolicy07 : passed 32, failed 0
set_mempolicy04 + MPOL_WEIGHTED_INTERLEAVE
== get_mempolicy01 : passed 12, failed 0
change: added MPOL_WEIGHTED_INTERLEAVE
== get_mempolicy02 : passed 2, failed 0
== mbind01 : passed 15, failed 0
added MPOL_WEIGHTED_INTERLEAVE
== mbind02 : passed 4, failed 0
added MPOL_WEIGHTED_INTERLEAVE
== mbind03 : passed 16, failed 0
added MPOL_WEIGHTED_INTERLEAVE
== mbind04 : passed 48, failed 0
added MPOL_WEIGHTED_INTERLEAVE
=====================================================================
numactl (set_mempolicy) w/ global weighting test
numactl fork: https://github.com/gmprice/numactl/tree/weighted_interleave_master
command: numactl -w --interleave=0,1 ./eatmem
result (weights 1:1):
0176a000 weighted interleave:0-1 heap anon=65793 dirty=65793 active=0 N0=32897 N1=32896 kernelpagesize_kB=4
7fceeb9ff000 weighted interleave:0-1 anon=65537 dirty=65537 active=0 N0=32768 N1=32769 kernelpagesize_kB=4
50% distribution is correct
result (weights 5:1):
01b14000 weighted interleave:0-1 heap anon=65793 dirty=65793 active=0 N0=54828 N1=10965 kernelpagesize_kB=4
7f47a1dff000 weighted interleave:0-1 anon=65537 dirty=65537 active=0 N0=54614 N1=10923 kernelpagesize_kB=4
16.666% distribution is correct
result (weights 1:5):
01f07000 weighted interleave:0-1 heap anon=65793 dirty=65793 active=0 N0=10966 N1=54827 kernelpagesize_kB=4
7f17b1dff000 weighted interleave:0-1 anon=65537 dirty=65537 active=0 N0=10923 N1=54614 kernelpagesize_kB=4
16.666% distribution is correct
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main (void)
{
char* mem = malloc(1024*1024*256);
memset(mem, 1, 1024*1024*256);
for (int i = 0; i < ((1024*1024*256)/4096); i++)
{
mem = malloc(4096);
mem[0] = 1;
}
printf("done\n");
getchar();
return 0;
}
=====================================================================
Suggested-by: Gregory Price <gregory.price@memverge.com>
Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Suggested-by: Hasan Al Maruf <hasanalmaruf@fb.com>
Suggested-by: Hao Wang <haowang3@fb.com>
Suggested-by: Ying Huang <ying.huang@intel.com>
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Suggested-by: Michal Hocko <mhocko@suse.com>
Suggested-by: Zhongkun He <hezhongkun.hzk@bytedance.com>
Suggested-by: Frank van der Linden <fvdl@google.com>
Suggested-by: John Groves <john@jagalactic.com>
Suggested-by: Vinicius Tavares Petrucci <vtavarespetr@micron.com>
Suggested-by: Srinivasulu Thanneeru <sthanneeru@micron.com>
Suggested-by: Ravi Jonnalagadda <ravis.opensrc@micron.com>
Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Suggested-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
Suggested-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Gregory Price <gregory.price@memverge.com>
Gregory Price (3):
mm/mempolicy: refactor a read-once mechanism into a function for
re-use
mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted
interleaving
mm/mempolicy: protect task interleave functions with
tsk->mems_allowed_seq
Rakie Kim (1):
mm/mempolicy: implement the sysfs-based weighted_interleave interface
.../ABI/testing/sysfs-kernel-mm-mempolicy | 4 +
...fs-kernel-mm-mempolicy-weighted-interleave | 25 +
.../admin-guide/mm/numa_memory_policy.rst | 9 +
include/linux/sched.h | 1 +
include/uapi/linux/mempolicy.h | 1 +
mm/mempolicy.c | 488 +++++++++++++++++-
6 files changed, 513 insertions(+), 15 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-kernel-mm-mempolicy
create mode 100644 Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave
--
2.39.1
^ permalink raw reply
* [PATCH v5 1/4] mm/mempolicy: implement the sysfs-based weighted_interleave interface
From: Gregory Price @ 2024-02-02 17:02 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linux-fsdevel, linux-api, corbet, akpm,
gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji, mhocko,
ying.huang, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <20240202170238.90004-1-gregory.price@memverge.com>
From: Rakie Kim <rakie.kim@sk.com>
This patch provides a way to set interleave weight information under
sysfs at /sys/kernel/mm/mempolicy/weighted_interleave/nodeN
The sysfs structure is designed as follows.
$ tree /sys/kernel/mm/mempolicy/
/sys/kernel/mm/mempolicy/ [1]
└── weighted_interleave [2]
├── node0 [3]
└── node1
Each file above can be explained as follows.
[1] mm/mempolicy: configuration interface for mempolicy subsystem
[2] weighted_interleave/: config interface for weighted interleave policy
[3] weighted_interleave/nodeN: weight for nodeN
If a node value is set to `0`, the system-default value will be used.
As of this patch, the system-default for all nodes is always 1.
Suggested-by: "Huang, Ying" <ying.huang@intel.com>
Signed-off-by: Rakie Kim <rakie.kim@sk.com>
Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
Co-developed-by: Gregory Price <gregory.price@memverge.com>
Signed-off-by: Gregory Price <gregory.price@memverge.com>
Co-developed-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
Signed-off-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
Reviewed-by: "Huang, Ying" <ying.huang@intel.com>
---
.../ABI/testing/sysfs-kernel-mm-mempolicy | 4 +
...fs-kernel-mm-mempolicy-weighted-interleave | 25 ++
mm/mempolicy.c | 223 ++++++++++++++++++
3 files changed, 252 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-kernel-mm-mempolicy
create mode 100644 Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave
diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy
new file mode 100644
index 000000000000..8ac327fd7fb6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy
@@ -0,0 +1,4 @@
+What: /sys/kernel/mm/mempolicy/
+Date: January 2024
+Contact: Linux memory management mailing list <linux-mm@kvack.org>
+Description: Interface for Mempolicy
diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave
new file mode 100644
index 000000000000..0b7972de04e9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave
@@ -0,0 +1,25 @@
+What: /sys/kernel/mm/mempolicy/weighted_interleave/
+Date: January 2024
+Contact: Linux memory management mailing list <linux-mm@kvack.org>
+Description: Configuration Interface for the Weighted Interleave policy
+
+What: /sys/kernel/mm/mempolicy/weighted_interleave/nodeN
+Date: January 2024
+Contact: Linux memory management mailing list <linux-mm@kvack.org>
+Description: Weight configuration interface for nodeN
+
+ The interleave weight for a memory node (N). These weights are
+ utilized by tasks which have set their mempolicy to
+ MPOL_WEIGHTED_INTERLEAVE.
+
+ These weights only affect new allocations, and changes at runtime
+ will not cause migrations on already allocated pages.
+
+ The minimum weight for a node is always 1.
+
+ Minimum weight: 1
+ Maximum weight: 255
+
+ Writing an empty string or `0` will reset the weight to the
+ system default. The system default may be set by the kernel
+ or drivers at boot or during hotplug events.
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 10a590ee1c89..41e58c4c0d01 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -131,6 +131,32 @@ static struct mempolicy default_policy = {
static struct mempolicy preferred_node_policy[MAX_NUMNODES];
+/*
+ * iw_table is the sysfs-set interleave weight table, a value of 0 denotes
+ * system-default value should be used. A NULL iw_table also denotes that
+ * system-default values should be used. Until the system-default table
+ * is implemented, the system-default is always 1.
+ *
+ * iw_table is RCU protected
+ */
+static u8 __rcu *iw_table;
+static DEFINE_MUTEX(iw_table_lock);
+
+static u8 get_il_weight(int node)
+{
+ u8 *table;
+ u8 weight;
+
+ rcu_read_lock();
+ table = rcu_dereference(iw_table);
+ /* if no iw_table, use system default */
+ weight = table ? table[node] : 1;
+ /* if value in iw_table is 0, use system default */
+ weight = weight ? weight : 1;
+ rcu_read_unlock();
+ return weight;
+}
+
/**
* numa_nearest_node - Find nearest node by state
* @node: Node id to start the search
@@ -3067,3 +3093,200 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
p += scnprintf(p, buffer + maxlen - p, ":%*pbl",
nodemask_pr_args(&nodes));
}
+
+#ifdef CONFIG_SYSFS
+struct iw_node_attr {
+ struct kobj_attribute kobj_attr;
+ int nid;
+};
+
+static ssize_t node_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ struct iw_node_attr *node_attr;
+ u8 weight;
+
+ node_attr = container_of(attr, struct iw_node_attr, kobj_attr);
+ weight = get_il_weight(node_attr->nid);
+ return sysfs_emit(buf, "%d\n", weight);
+}
+
+static ssize_t node_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct iw_node_attr *node_attr;
+ u8 *new;
+ u8 *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 = kzalloc(nr_node_ids, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ mutex_lock(&iw_table_lock);
+ old = rcu_dereference_protected(iw_table,
+ lockdep_is_held(&iw_table_lock));
+ if (old)
+ memcpy(new, old, nr_node_ids);
+ new[node_attr->nid] = weight;
+ rcu_assign_pointer(iw_table, new);
+ mutex_unlock(&iw_table_lock);
+ synchronize_rcu();
+ kfree(old);
+ return count;
+}
+
+static struct iw_node_attr **node_attrs;
+
+static void sysfs_wi_node_release(struct iw_node_attr *node_attr,
+ struct kobject *parent)
+{
+ if (!node_attr)
+ return;
+ sysfs_remove_file(parent, &node_attr->kobj_attr.attr);
+ kfree(node_attr->kobj_attr.attr.name);
+ kfree(node_attr);
+}
+
+static void sysfs_wi_release(struct kobject *wi_kobj)
+{
+ int i;
+
+ for (i = 0; i < nr_node_ids; i++)
+ sysfs_wi_node_release(node_attrs[i], wi_kobj);
+ kobject_put(wi_kobj);
+}
+
+static const struct kobj_type wi_ktype = {
+ .sysfs_ops = &kobj_sysfs_ops,
+ .release = sysfs_wi_release,
+};
+
+static int add_weight_node(int nid, struct kobject *wi_kobj)
+{
+ struct iw_node_attr *node_attr;
+ char *name;
+
+ node_attr = kzalloc(sizeof(*node_attr), GFP_KERNEL);
+ if (!node_attr)
+ return -ENOMEM;
+
+ name = kasprintf(GFP_KERNEL, "node%d", nid);
+ if (!name) {
+ kfree(node_attr);
+ return -ENOMEM;
+ }
+
+ sysfs_attr_init(&node_attr->kobj_attr.attr);
+ node_attr->kobj_attr.attr.name = name;
+ node_attr->kobj_attr.attr.mode = 0644;
+ node_attr->kobj_attr.show = node_show;
+ node_attr->kobj_attr.store = node_store;
+ node_attr->nid = nid;
+
+ if (sysfs_create_file(wi_kobj, &node_attr->kobj_attr.attr)) {
+ kfree(node_attr->kobj_attr.attr.name);
+ kfree(node_attr);
+ pr_err("failed to add attribute to weighted_interleave\n");
+ return -ENOMEM;
+ }
+
+ node_attrs[nid] = node_attr;
+ return 0;
+}
+
+static int add_weighted_interleave_group(struct kobject *root_kobj)
+{
+ struct kobject *wi_kobj;
+ int nid, err;
+
+ wi_kobj = kzalloc(sizeof(struct kobject), GFP_KERNEL);
+ if (!wi_kobj)
+ return -ENOMEM;
+
+ err = kobject_init_and_add(wi_kobj, &wi_ktype, root_kobj,
+ "weighted_interleave");
+ if (err) {
+ kfree(wi_kobj);
+ return err;
+ }
+
+ for_each_node_state(nid, N_POSSIBLE) {
+ err = add_weight_node(nid, wi_kobj);
+ if (err) {
+ pr_err("failed to add sysfs [node%d]\n", nid);
+ break;
+ }
+ }
+ if (err)
+ kobject_put(wi_kobj);
+ return 0;
+}
+
+static void mempolicy_kobj_release(struct kobject *kobj)
+{
+ u8 *old;
+
+ mutex_lock(&iw_table_lock);
+ old = rcu_dereference_protected(iw_table,
+ lockdep_is_held(&iw_table_lock));
+ rcu_assign_pointer(iw_table, NULL);
+ mutex_unlock(&iw_table_lock);
+ synchronize_rcu();
+ kfree(old);
+ kfree(node_attrs);
+ kfree(kobj);
+}
+
+static const struct kobj_type mempolicy_ktype = {
+ .release = mempolicy_kobj_release
+};
+
+static int __init mempolicy_sysfs_init(void)
+{
+ int err;
+ static struct kobject *mempolicy_kobj;
+
+ mempolicy_kobj = kzalloc(sizeof(*mempolicy_kobj), GFP_KERNEL);
+ if (!mempolicy_kobj) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+
+ node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *),
+ GFP_KERNEL);
+ if (!node_attrs) {
+ err = -ENOMEM;
+ goto mempol_out;
+ }
+
+ err = kobject_init_and_add(mempolicy_kobj, &mempolicy_ktype, mm_kobj,
+ "mempolicy");
+ if (err)
+ goto node_out;
+
+ err = add_weighted_interleave_group(mempolicy_kobj);
+ if (err) {
+ pr_err("mempolicy sysfs structure failed to initialize\n");
+ kobject_put(mempolicy_kobj);
+ return err;
+ }
+
+ return err;
+node_out:
+ kfree(node_attrs);
+mempol_out:
+ kfree(mempolicy_kobj);
+err_out:
+ pr_err("failed to add mempolicy kobject to the system\n");
+ return err;
+}
+
+late_initcall(mempolicy_sysfs_init);
+#endif /* CONFIG_SYSFS */
--
2.39.1
^ permalink raw reply related
* [PATCH v5 2/4] mm/mempolicy: refactor a read-once mechanism into a function for re-use
From: Gregory Price @ 2024-02-02 17:02 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linux-fsdevel, linux-api, corbet, akpm,
gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji, mhocko,
ying.huang, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <20240202170238.90004-1-gregory.price@memverge.com>
move the use of barrier() to force policy->nodemask onto the stack into
a function `read_once_policy_nodemask` so that it may be re-used.
Suggested-by: "Huang, Ying" <ying.huang@intel.com>
Signed-off-by: Gregory Price <gregory.price@memverge.com>
Reviewed-by: "Huang, Ying" <ying.huang@intel.com>
---
mm/mempolicy.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 41e58c4c0d01..697f2a791c24 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1909,6 +1909,20 @@ unsigned int mempolicy_slab_node(void)
}
}
+static unsigned int read_once_policy_nodemask(struct mempolicy *pol,
+ nodemask_t *mask)
+{
+ /*
+ * barrier stabilizes the nodemask locally so that it can be iterated
+ * over safely without concern for changes. Allocators validate node
+ * selection does not violate mems_allowed, so this is safe.
+ */
+ barrier();
+ memcpy(mask, &pol->nodes, sizeof(nodemask_t));
+ barrier();
+ return nodes_weight(*mask);
+}
+
/*
* Do static interleaving for interleave index @ilx. Returns the ilx'th
* node in pol->nodes (starting from ilx=0), wrapping around if ilx
@@ -1916,20 +1930,12 @@ unsigned int mempolicy_slab_node(void)
*/
static unsigned int interleave_nid(struct mempolicy *pol, pgoff_t ilx)
{
- nodemask_t nodemask = pol->nodes;
+ nodemask_t nodemask;
unsigned int target, nnodes;
int i;
int nid;
- /*
- * The barrier will stabilize the nodemask in a register or on
- * the stack so that it will stop changing under the code.
- *
- * Between first_node() and next_node(), pol->nodes could be changed
- * by other threads. So we put pol->nodes in a local stack.
- */
- barrier();
- nnodes = nodes_weight(nodemask);
+ nnodes = read_once_policy_nodemask(pol, &nodemask);
if (!nnodes)
return numa_node_id();
target = ilx % nnodes;
--
2.39.1
^ permalink raw reply related
* [PATCH v5 3/4] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-02-02 17:02 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linux-fsdevel, linux-api, corbet, akpm,
gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji, mhocko,
ying.huang, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <20240202170238.90004-1-gregory.price@memverge.com>
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/
For now, the default value of all nodes will be `1`, which matches
the behavior of standard 1:1 round-robin interleave. An extension
will be added in the future to allow default values to be registered
at kernel and device bringup time.
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).
Some high level notes about the pieces of weighted interleave:
current->il_prev:
Tracks the node previously allocated from.
current->il_weight:
The active weight of the current node (current->il_prev)
When this reaches 0, current->il_prev is set to the next node
and current->il_weight is set to the next weight.
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. Operates only on task->mempolicy.
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.
Operates on VMA policies.
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 will be allocated first.
Operates only on the task->mempolicy.
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. If a call
to alloc_pages_mpol() were made with a weighted-interleave policy and
ilx set to NO_INTERLEAVE_INDEX, weighted_interleave_nodes() would
operate on a VMA policy - violating the description above.
An inspection of all callers of alloc_pages_mpol() shows that all
external callers set ilx to `0`, an index value, or will call
get_vma_policy() to acquire the ilx.
For example, mm/shmem.c may call into alloc_pages_mpol. The call stacks
all set (pgoff_t ilx) or end up in `get_vma_policy()`. This enforces
the `weighted_interleave_nodes()` and `weighted_interleave_nid()`
policy requirements (task/vma respectively).
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>
Reviewed-by: "Huang, Ying" <ying.huang@intel.com>
---
.../admin-guide/mm/numa_memory_policy.rst | 9 +
include/linux/sched.h | 1 +
include/uapi/linux/mempolicy.h | 1 +
mm/mempolicy.c | 218 +++++++++++++++++-
4 files changed, 225 insertions(+), 4 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/sched.h b/include/linux/sched.h
index ffe8f618ab86..b9ce285d8c9c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1259,6 +1259,7 @@ struct task_struct {
/* Protected by alloc_lock: */
struct mempolicy *mempolicy;
short il_prev;
+ u8 il_weight;
short pref_node_fork;
#endif
#ifdef CONFIG_NUMA_BALANCING
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 697f2a791c24..d8cc3a577986 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
@@ -441,6 +448,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,
@@ -862,8 +873,11 @@ 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;
+ current->il_weight = 0;
+ }
task_unlock(current);
mpol_put(old);
ret = 0;
@@ -888,6 +902,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:
@@ -972,6 +987,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 (current->il_weight)
+ *policy = current->il_prev;
+ else
+ *policy = next_node_in(current->il_prev,
+ pol->nodes);
} else {
err = -EINVAL;
goto out;
@@ -1336,7 +1358,8 @@ static long do_mbind(unsigned long start, unsigned long len,
* VMAs, the nodes will still be interleaved from the targeted
* nodemask, but one by one may be selected differently.
*/
- if (new->mode == MPOL_INTERLEAVE) {
+ if (new->mode == MPOL_INTERLEAVE ||
+ new->mode == MPOL_WEIGHTED_INTERLEAVE) {
struct page *page;
unsigned int order;
unsigned long addr = -EFAULT;
@@ -1784,7 +1807,8 @@ struct mempolicy *__get_vma_policy(struct vm_area_struct *vma,
* @vma: virtual memory area whose policy is sought
* @addr: address in @vma for shared policy lookup
* @order: 0, or appropriate huge_page_order for interleaving
- * @ilx: interleave index (output), for use only when MPOL_INTERLEAVE
+ * @ilx: interleave index (output), for use only when MPOL_INTERLEAVE or
+ * MPOL_WEIGHTED_INTERLEAVE
*
* Returns effective policy for a VMA at specified address.
* Falls back to current->mempolicy or system default policy, as necessary.
@@ -1801,7 +1825,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);
}
@@ -1851,6 +1876,22 @@ 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 node = current->il_prev;
+
+ if (!current->il_weight || !node_isset(node, policy->nodes)) {
+ node = next_node_in(node, policy->nodes);
+ /* can only happen if nodemask is being rebound */
+ if (node == MAX_NUMNODES)
+ return node;
+ current->il_prev = node;
+ current->il_weight = get_il_weight(node);
+ }
+ current->il_weight--;
+ return node;
+}
+
/* Do dynamic interleaving for a process */
static unsigned int interleave_nodes(struct mempolicy *policy)
{
@@ -1885,6 +1926,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:
{
@@ -1923,6 +1967,45 @@ 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;
+ u8 *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) {
+ /* detect system default usage */
+ weight = table ? table[nid] : 1;
+ weight = weight ? weight : 1;
+ weight_total += weight;
+ }
+
+ /* Calculate the node offset based on totals */
+ target = ilx % weight_total;
+ nid = first_node(nodemask);
+ while (target) {
+ /* detect system default usage */
+ weight = table ? table[nid] : 1;
+ weight = weight ? weight : 1;
+ 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
@@ -1983,6 +2066,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;
@@ -2044,6 +2132,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;
@@ -2144,6 +2233,7 @@ struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
* node in its nodemask, we allocate the standard way.
*/
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
@@ -2279,6 +2369,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 = 0;
+ unsigned long rounds;
+ unsigned long node_pages, delta;
+ u8 *table, *weights, weight;
+ unsigned int weight_total = 0;
+ unsigned long rem_pages = nr_pages;
+ nodemask_t nodes;
+ int nnodes, node;
+ int resume_node = MAX_NUMNODES - 1;
+ u8 resume_weight = 0;
+ int prev_node;
+ int i;
+
+ if (!nr_pages)
+ return 0;
+
+ nnodes = read_once_policy_nodemask(pol, &nodes);
+ if (!nnodes)
+ return 0;
+
+ /* Continue allocating from most recent node and adjust the nr_pages */
+ node = me->il_prev;
+ weight = me->il_weight;
+ if (weight && node_isset(node, nodes)) {
+ node_pages = min(rem_pages, weight);
+ 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 <= weight) {
+ me->il_weight -= rem_pages;
+ return total_allocated;
+ }
+ /* Otherwise we adjust remaining pages, continue from there */
+ rem_pages -= weight;
+ }
+ /* clear active weight in case of an allocation failure */
+ me->il_weight = 0;
+ prev_node = node;
+
+ /* create a local copy of node weights to operate on outside rcu */
+ weights = kzalloc(nr_node_ids, GFP_KERNEL);
+ if (!weights)
+ return total_allocated;
+
+ rcu_read_lock();
+ table = rcu_dereference(iw_table);
+ if (table)
+ memcpy(weights, table, nr_node_ids);
+ rcu_read_unlock();
+
+ /* calculate total, detect system default usage */
+ for_each_node_mask(node, nodes) {
+ if (!weights[node])
+ weights[node] = 1;
+ weight_total += weights[node];
+ }
+
+ /*
+ * Calculate rounds/partial rounds to minimize __alloc_pages_bulk calls.
+ * Track which node weighted interleave should resume from.
+ *
+ * if (rounds > 0) and (delta == 0), resume_node will always be
+ * the node following prev_node and its weight.
+ */
+ rounds = rem_pages / weight_total;
+ delta = rem_pages % weight_total;
+ resume_node = next_node_in(prev_node, nodes);
+ resume_weight = weights[resume_node];
+ for (i = 0; i < nnodes; i++) {
+ node = next_node_in(prev_node, nodes);
+ weight = weights[node];
+ node_pages = weight * rounds;
+ /* If a delta exists, add this node's portion of the delta */
+ if (delta > weight) {
+ node_pages += weight;
+ delta -= weight;
+ } else if (delta) {
+ /* when delta is depleted, resume from that node */
+ node_pages += delta;
+ resume_node = node;
+ resume_weight = weight - delta;
+ delta = 0;
+ }
+ /* node_pages can be 0 if an allocation fails and rounds == 0 */
+ if (!node_pages)
+ break;
+ 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;
+ }
+ me->il_prev = resume_node;
+ me->il_weight = resume_weight;
+ 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)
@@ -2319,6 +2517,10 @@ 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);
@@ -2394,6 +2596,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;
@@ -2530,6 +2733,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;
@@ -2904,6 +3111,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)",
};
@@ -2963,6 +3171,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
*/
@@ -3073,6 +3282,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:
--
2.39.1
^ permalink raw reply related
* [PATCH v5 4/4] mm/mempolicy: protect task interleave functions with tsk->mems_allowed_seq
From: Gregory Price @ 2024-02-02 17:02 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linux-fsdevel, linux-api, corbet, akpm,
gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji, mhocko,
ying.huang, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams
In-Reply-To: <20240202170238.90004-1-gregory.price@memverge.com>
In the event of rebind, pol->nodemask can change at the same time as an
allocation occurs. We can detect this with tsk->mems_allowed_seq and
prevent a miscount or an allocation failure from occurring.
The same thing happens in the allocators to detect failure, but this
can prevent spurious failures in a much smaller critical section.
Suggested-by: "Huang, Ying" <ying.huang@intel.com>
Signed-off-by: Gregory Price <gregory.price@memverge.com>
---
mm/mempolicy.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d8cc3a577986..ed0d5d2d456a 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1878,11 +1878,17 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
{
- unsigned int node = current->il_prev;
-
- if (!current->il_weight || !node_isset(node, policy->nodes)) {
+ unsigned int node;
+ unsigned int cpuset_mems_cookie;
+
+retry:
+ /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ node = current->il_prev;
+ if (!node || !node_isset(node, policy->nodes)) {
node = next_node_in(node, policy->nodes);
- /* can only happen if nodemask is being rebound */
+ if (read_mems_allowed_retry(cpuset_mems_cookie))
+ goto retry;
if (node == MAX_NUMNODES)
return node;
current->il_prev = node;
@@ -1896,8 +1902,14 @@ static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
static unsigned int interleave_nodes(struct mempolicy *policy)
{
unsigned int nid;
+ unsigned int cpuset_mems_cookie;
+
+ /* to prevent miscount, use tsk->mems_allowed_seq to detect rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nid = next_node_in(current->il_prev, policy->nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
- nid = next_node_in(current->il_prev, policy->nodes);
if (nid < MAX_NUMNODES)
current->il_prev = nid;
return nid;
@@ -2374,6 +2386,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
struct page **page_array)
{
struct task_struct *me = current;
+ unsigned int cpuset_mems_cookie;
unsigned long total_allocated = 0;
unsigned long nr_allocated = 0;
unsigned long rounds;
@@ -2391,7 +2404,13 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
if (!nr_pages)
return 0;
- nnodes = read_once_policy_nodemask(pol, &nodes);
+ /* read the nodes onto the stack, retry if done during rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nnodes = read_once_policy_nodemask(pol, &nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
+
+ /* if the nodemask has become invalid, we cannot do anything */
if (!nnodes)
return 0;
--
2.39.1
^ 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;
as well as URLs for NNTP newsgroup(s).