* [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths
@ 2026-08-29 1:59 Gregory Price
2026-08-29 1:59 ` [PATCH 1/2] mm/mempolicy: use SRCU for the weighted interleave state Gregory Price
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Gregory Price @ 2026-08-29 1:59 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, kernel-team, akpm, david, ziy, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
urezki, chenwandun
The interleave node selectors and bulk allocators take copies of
nodemasks and node weights (for weighted interleave) in the fault path.
Both of these copies can be entirely eliminated.
For node weights, use SRCU to pin the weights in place. This eliminates
a copy and a kmalloc from the bulk allocator path.
For nodemasks, we can operate directly on pol->nodes as long as we bounds
check the walk. A concurrent rebind can shrink the mask, or tear the read
of it so the mask appears empty.
- The interleave node selectors fall back to numa_node_id() when that
happens, which is what they already did when a copy came back empty.
- The bulk allocator simply returns what it managed to allocate.
The node count and weight totals are read separately from the nodemask
walk that consumes them - creating a time-of-check / time-of-use race.
Just clamp the walk to a single pass (number of nodes), and clamp each
bulk allocation chunk to the space left in the request.
The cost is distribution accuracy during a rebind. The copies never
corrected for that either - they only kept the code from dividing by
zero and overrunning the allocation request.
Gregory Price (2):
mm/mempolicy: use SRCU for the weighted interleave state
mm/mempolicy: stop copying the nodemask in the interleave paths
mm/mempolicy.c | 156 ++++++++++++++++++++++++++-----------------------
1 file changed, 83 insertions(+), 73 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] mm/mempolicy: use SRCU for the weighted interleave state 2026-08-29 1:59 [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths Gregory Price @ 2026-08-29 1:59 ` Gregory Price 2026-08-29 1:59 ` [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths Gregory Price 2026-08-29 23:18 ` [PATCH 0/2] mm/mempolicy: stop copying state " Andrew Morton 2 siblings, 0 replies; 8+ messages in thread From: Gregory Price @ 2026-08-29 1:59 UTC (permalink / raw) To: linux-mm Cc: linux-kernel, kernel-team, akpm, david, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, urezki, chenwandun, Matthew Wilcox alloc_pages_bulk_weighted_interleave() copies iw_table into a scratch array on every call so it can walk the weights outside of RCU. The copy exists only because the loop may sleep in the page allocator and so cannot hold rcu_read_lock(). Use SRCU to pin the global iw_table object and use it in-place instead. Retire through both flavors - call_srcu() for the sleeping readers, then kfree_rcu() for the reference-less ones - so writers no longer block on synchronize_rcu() either. Tested in a VM with KASAN, PROVE_LOCKING and DEBUG_OBJECTS_RCU_HEAD, with a udelay() injected into the read section to widen the race against concurrent sysfs weight writers, and placement checked against the configured weights. Every retired state reached its callback. Swapping the deferred free for a bare kfree() in the same test reports a use-after-free immediately. Suggested-by: Andrew Morton <akpm@linux-foundation.org> Suggested-by: Matthew Wilcox <willy@infradead.org> Assisted-by: Claude:claude-opus-5 Signed-off-by: Gregory Price (Meta) <gourry@gourry.net> --- mm/mempolicy.c | 70 ++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 060a0eb26917..2643915dc966 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -112,6 +112,7 @@ #include <linux/printk.h> #include <linux/leafops.h> #include <linux/gcd.h> +#include <linux/srcu.h> #include <asm/tlbflush.h> #include <asm/tlb.h> @@ -157,6 +158,7 @@ static const int weightiness = 32; */ struct weighted_interleave_state { bool mode_auto; + struct rcu_head rcu; u8 iw_table[]; }; static struct weighted_interleave_state __rcu *wi_state; @@ -168,6 +170,24 @@ static unsigned int *node_bw_table; */ static DEFINE_MUTEX(wi_state_lock); +/* Readers that sleep while walking iw_table hold this instead */ +DEFINE_STATIC_SRCU_FAST(wi_srcu); + +static void wi_state_free_rcu(struct rcu_head *head) +{ + struct weighted_interleave_state *state = + container_of(head, struct weighted_interleave_state, rcu); + + kfree_rcu(state, rcu); +} + +/* Retire through both flavors: sleeping readers use SRCU, the rest RCU */ +static void wi_state_retire(struct weighted_interleave_state *state) +{ + if (state) + call_srcu(&wi_srcu, &state->rcu, wi_state_free_rcu); +} + static u8 get_il_weight(int node) { struct weighted_interleave_state *state; @@ -266,10 +286,7 @@ int mempolicy_set_node_perf(unsigned int node, struct access_coordinate *coords) rcu_assign_pointer(wi_state, new_wi_state); mutex_unlock(&wi_state_lock); - if (old_wi_state) { - synchronize_rcu(); - kfree(old_wi_state); - } + wi_state_retire(old_wi_state); out: kfree(old_bw); return 0; @@ -2644,7 +2661,8 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, unsigned long nr_allocated = 0; unsigned long rounds; unsigned long node_pages, delta; - u8 *weights, weight; + struct srcu_ctr __percpu *scp; + u8 *table, weight; unsigned int weight_total = 0; unsigned long rem_pages = nr_pages; nodemask_t nodes; @@ -2688,25 +2706,14 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, me->il_weight = 0; prev_node = node; - /* create a local copy of node weights to operate on outside rcu */ - weights = kmalloc(nr_node_ids, gfp & GFP_RECLAIM_MASK); - if (!weights) - return total_allocated; - - rcu_read_lock(); - state = rcu_dereference(wi_state); - if (state) { - memcpy(weights, state->iw_table, nr_node_ids * sizeof(u8)); - rcu_read_unlock(); - } else { - rcu_read_unlock(); - for (i = 0; i < nr_node_ids; i++) - weights[i] = 1; - } + /* The page allocator may sleep, pin the weight table with SRCU */ + scp = srcu_read_lock_fast(&wi_srcu); + state = srcu_dereference(wi_state, &wi_srcu); + table = state ? state->iw_table : NULL; /* calculate total, detect system default usage */ for_each_node_mask(node, nodes) - weight_total += weights[node]; + weight_total += table ? table[node] : 1; /* * Calculate rounds/partial rounds to minimize __alloc_pages_bulk calls. @@ -2718,10 +2725,10 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, rounds = rem_pages / weight_total; delta = rem_pages % weight_total; resume_node = next_node_in(prev_node, nodes); - resume_weight = weights[resume_node]; + resume_weight = table ? table[resume_node] : 1; for (i = 0; i < nnodes; i++) { node = next_node_in(prev_node, nodes); - weight = weights[node]; + weight = table ? table[node] : 1; node_pages = weight * rounds; /* If a delta exists, add this node's portion of the delta */ if (delta > weight) { @@ -2747,7 +2754,7 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, } me->il_prev = resume_node; me->il_weight = resume_weight; - kfree(weights); + srcu_read_unlock_fast(&wi_srcu, scp); return total_allocated; } @@ -3673,10 +3680,7 @@ static ssize_t node_store(struct kobject *kobj, struct kobj_attribute *attr, rcu_assign_pointer(wi_state, new_wi_state); mutex_unlock(&wi_state_lock); - if (old_wi_state) { - synchronize_rcu(); - kfree(old_wi_state); - } + wi_state_retire(old_wi_state); return count; } @@ -3742,10 +3746,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj, update_wi_state: rcu_assign_pointer(wi_state, new_wi_state); mutex_unlock(&wi_state_lock); - if (old_wi_state) { - synchronize_rcu(); - kfree(old_wi_state); - } + wi_state_retire(old_wi_state); return count; } @@ -3789,10 +3790,7 @@ static void wi_state_free(void) rcu_assign_pointer(wi_state, NULL); mutex_unlock(&wi_state_lock); - if (old_wi_state) { - synchronize_rcu(); - kfree(old_wi_state); - } + wi_state_retire(old_wi_state); } static struct kobj_attribute wi_auto_attr = { -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths 2026-08-29 1:59 [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths Gregory Price 2026-08-29 1:59 ` [PATCH 1/2] mm/mempolicy: use SRCU for the weighted interleave state Gregory Price @ 2026-08-29 1:59 ` Gregory Price 2026-09-02 9:00 ` Rakie Kim 2026-08-29 23:18 ` [PATCH 0/2] mm/mempolicy: stop copying state " Andrew Morton 2 siblings, 1 reply; 8+ messages in thread From: Gregory Price @ 2026-08-29 1:59 UTC (permalink / raw) To: linux-mm Cc: linux-kernel, kernel-team, akpm, david, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, urezki, chenwandun The interleave node selectors copy pol->nodes onto the stack so the mask cannot change while they walk it. nodemask_t is 128 bytes at MAX_NUMNODES=1024, and two of the three run per folio fault. The copy only buys consistency between the node count and the walk. Drop the consistency and just bounds check the walk instead. If an empty nodelist or weight is perceived, fall back to numa_node_id(), which is what the functions already did when the copy came back empty. weighted_interleave_nid() counts the nodes as we sum the weights. We use that node count to limit the maximum skew a single node can host. interleave_nid() walks with next_node_in() rather than next_node(), so a mask that shrank mid-walk wraps to a node still in the policy. alloc_pages_bulk_weighted_interleave() derives per-node counts from a weight total summed over the mask, so a changing mask can make them exceed the request. Clamp each chunk to the space left in page_array. A cpuset cookie will not work here: two of these take VMA policies, which mpol_rebind_mm() rebinds under mmap_write_lock(), not mems_allowed_seq. Cost is distribution accuracy during a rebind - but the copy never corrected this anyway, it was just a safety mechanism to prevent div/0 and overrunning the alloc request buffer. Remove read_once_policy_nodemask(), now unused. -fstack-usage at MAX_NUMNODES=1024: weighted_interleave_nid 184 -> 56 interleave_nid 168 -> 32 alloc_pages_bulk_mempolicy_noprof 360 -> 136 Assisted-by: Claude:claude-opus-5 Signed-off-by: Gregory Price (Meta) <gourry@gourry.net> --- mm/mempolicy.c | 86 ++++++++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 37 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 2643915dc966..296129126109 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -2197,34 +2197,15 @@ 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); -} - static unsigned int weighted_interleave_nid(struct mempolicy *pol, pgoff_t ilx) { struct weighted_interleave_state *state; - nodemask_t nodemask; - unsigned int target, nr_nodes; + unsigned int target, nnodes = 0; u8 *table = NULL; unsigned int weight_total = 0; u8 weight; int nid = 0; - nr_nodes = read_once_policy_nodemask(pol, &nodemask); - if (!nr_nodes) - return numa_node_id(); - rcu_read_lock(); state = rcu_dereference(wi_state); @@ -2232,22 +2213,40 @@ static unsigned int weighted_interleave_nid(struct mempolicy *pol, pgoff_t ilx) if (state) table = state->iw_table; - /* calculate the total weight */ - for_each_node_mask(nid, nodemask) + /* calculate the total weight and the node count */ + for_each_node_mask(nid, pol->nodes) { weight_total += table ? table[nid] : 1; + nnodes++; + } + + /* the mask is empty */ + if (!weight_total) { + rcu_read_unlock(); + return numa_node_id(); + } /* Calculate the node offset based on totals */ target = ilx % weight_total; - nid = first_node(nodemask); - while (target) { + nid = first_node(pol->nodes); + + /* + * The target was calculated in a separate loop, and a concurrent + * rebind can change the total number of nodes. Clamp this loop to + * a single pass (nnodes) to keep the walk bounded by node count. + */ + while (target && nnodes-- && nid < MAX_NUMNODES) { /* detect system default usage */ weight = table ? table[nid] : 1; if (target < weight) break; target -= weight; - nid = next_node_in(nid, nodemask); + nid = next_node_in(nid, pol->nodes); } rcu_read_unlock(); + + /* the mask emptied under the walk */ + if (nid >= MAX_NUMNODES) + return numa_node_id(); return nid; } @@ -2258,18 +2257,21 @@ static unsigned int weighted_interleave_nid(struct mempolicy *pol, pgoff_t ilx) */ static unsigned int interleave_nid(struct mempolicy *pol, pgoff_t ilx) { - nodemask_t nodemask; unsigned int target, nnodes; int i; int nid; - nnodes = read_once_policy_nodemask(pol, &nodemask); + nnodes = nodes_weight(pol->nodes); if (!nnodes) return numa_node_id(); target = ilx % nnodes; - nid = first_node(nodemask); - for (i = 0; i < target; i++) - nid = next_node(nid, nodemask); + nid = first_node(pol->nodes); + for (i = 0; i < target && nid < MAX_NUMNODES; i++) + nid = next_node_in(nid, pol->nodes); + + /* the mask emptied under the walk */ + if (nid >= MAX_NUMNODES) + return numa_node_id(); return nid; } @@ -2665,7 +2667,6 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, u8 *table, 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; @@ -2675,10 +2676,10 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, if (!nr_pages) return 0; - /* read the nodes onto the stack, retry if done during rebind */ + /* count the nodes, retry if a rebind happened during the read */ do { cpuset_mems_cookie = read_mems_allowed_begin(); - nnodes = read_once_policy_nodemask(pol, &nodes); + nnodes = nodes_weight(pol->nodes); } while (read_mems_allowed_retry(cpuset_mems_cookie)); /* if the nodemask has become invalid, we cannot do anything */ @@ -2688,7 +2689,7 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, /* 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)) { + if (weight && node_isset(node, pol->nodes)) { node_pages = min(rem_pages, weight); nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages, page_array); @@ -2712,9 +2713,13 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, table = state ? state->iw_table : NULL; /* calculate total, detect system default usage */ - for_each_node_mask(node, nodes) + for_each_node_mask(node, pol->nodes) weight_total += table ? table[node] : 1; + /* the mask emptied since it was counted */ + if (!weight_total) + goto out; + /* * Calculate rounds/partial rounds to minimize __alloc_pages_bulk calls. * Track which node weighted interleave should resume from. @@ -2724,10 +2729,14 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, */ rounds = rem_pages / weight_total; delta = rem_pages % weight_total; - resume_node = next_node_in(prev_node, nodes); + resume_node = next_node_in(prev_node, pol->nodes); + if (resume_node >= MAX_NUMNODES) + goto out; resume_weight = table ? table[resume_node] : 1; for (i = 0; i < nnodes; i++) { - node = next_node_in(prev_node, nodes); + node = next_node_in(prev_node, pol->nodes); + if (node >= MAX_NUMNODES) + break; weight = table ? table[node] : 1; node_pages = weight * rounds; /* If a delta exists, add this node's portion of the delta */ @@ -2744,6 +2753,8 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, /* node_pages can be 0 if an allocation fails and rounds == 0 */ if (!node_pages) break; + /* a rebind can invalidate the counts: never overrun page_array */ + node_pages = min(node_pages, nr_pages - total_allocated); nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages, page_array); page_array += nr_allocated; @@ -2754,6 +2765,7 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, } me->il_prev = resume_node; me->il_weight = resume_weight; +out: srcu_read_unlock_fast(&wi_srcu, scp); return total_allocated; } -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths 2026-08-29 1:59 ` [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths Gregory Price @ 2026-09-02 9:00 ` Rakie Kim 2026-09-02 14:52 ` Gregory Price 0 siblings, 1 reply; 8+ messages in thread From: Rakie Kim @ 2026-09-02 9:00 UTC (permalink / raw) To: Gregory Price Cc: linux-kernel, kernel-team, akpm, david, ziy, matthew.brost, joshua.hahnjy, byungchul, ying.huang, apopple, urezki, chenwandun, linux-mm, kernel_team, Rakie Kim Hello Gregory, Thanks for the series. On Fri, 28 Aug 2026 21:59:43 -0400 Gregory Price <gourry@gourry.net> wrote: > The interleave node selectors copy pol->nodes onto the stack so the mask > cannot change while they walk it. nodemask_t is 128 bytes at > MAX_NUMNODES=1024, and two of the three run per folio fault. > > The copy only buys consistency between the node count and the walk. > Drop the consistency and just bounds check the walk instead. I went through both patches. Resolving the sleeping-allocation problem with SRCU rather than patching the allocation site, and removing the copies from the fault path along the way, looks like the right direction to me. > > If an empty nodelist or weight is perceived, fall back to numa_node_id(), > which is what the functions already did when the copy came back empty. > > weighted_interleave_nid() counts the nodes as we sum the weights. We use > that node count to limit the maximum skew a single node can host. > > interleave_nid() walks with next_node_in() rather than next_node(), so a > mask that shrank mid-walk wraps to a node still in the policy. > > alloc_pages_bulk_weighted_interleave() derives per-node counts from a > weight total summed over the mask, so a changing mask can make them exceed > the request. Clamp each chunk to the space left in page_array. > > A cpuset cookie will not work here: two of these take VMA policies, which > mpol_rebind_mm() rebinds under mmap_write_lock(), not mems_allowed_seq. > > Cost is distribution accuracy during a rebind - but the copy never > corrected this anyway, it was just a safety mechanism to prevent div/0 > and overrunning the alloc request buffer. > > Remove read_once_policy_nodemask(), now unused. > > -fstack-usage at MAX_NUMNODES=1024: > > weighted_interleave_nid 184 -> 56 > interleave_nid 168 -> 32 > alloc_pages_bulk_mempolicy_noprof 360 -> 136 > > Assisted-by: Claude:claude-opus-5 > Signed-off-by: Gregory Price (Meta) <gourry@gourry.net> > --- > mm/mempolicy.c | 86 ++++++++++++++++++++++++++++---------------------- > 1 file changed, 49 insertions(+), 37 deletions(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c [...snip...] > @@ -2675,10 +2676,10 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, > if (!nr_pages) > return 0; > > - /* read the nodes onto the stack, retry if done during rebind */ > + /* count the nodes, retry if a rebind happened during the read */ > do { > cpuset_mems_cookie = read_mems_allowed_begin(); > - nnodes = read_once_policy_nodemask(pol, &nodes); > + nnodes = nodes_weight(pol->nodes); > } while (read_mems_allowed_retry(cpuset_mems_cookie)); > > /* if the nodemask has become invalid, we cannot do anything */ [...snip...] > @@ -2712,9 +2713,13 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, > table = state ? state->iw_table : NULL; > > /* calculate total, detect system default usage */ > - for_each_node_mask(node, nodes) > + for_each_node_mask(node, pol->nodes) > weight_total += table ? table[node] : 1; I have a minor comment on this part. After this change, everything else reads pol->nodes fresh at the point of use - the weight sum and the walk both look at the current mask. Only nnodes is still the count from this earlier read. If the mask changes in between, the loop bound no longer matches the mask the loop is actually walking, so the walk can stop short of the pages the weight total planned for. Would it be better to count the nodes in the loop that sums the weights, the way weighted_interleave_nid() does it in this patch? nnodes = 0; for_each_node_mask(node, pol->nodes) { weight_total += table ? table[node] : 1; nnodes++; } [...snip...] Thanks again for your time. Rakie Kim ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths 2026-09-02 9:00 ` Rakie Kim @ 2026-09-02 14:52 ` Gregory Price 2026-09-04 8:01 ` Rakie Kim 0 siblings, 1 reply; 8+ messages in thread From: Gregory Price @ 2026-09-02 14:52 UTC (permalink / raw) To: Rakie Kim Cc: linux-kernel, kernel-team, akpm, david, ziy, matthew.brost, joshua.hahnjy, byungchul, ying.huang, apopple, urezki, chenwandun, linux-mm, kernel_team On Wed, Sep 02, 2026 at 06:00:44PM +0900, Rakie Kim wrote: > > > @@ -2712,9 +2713,13 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp, > > table = state ? state->iw_table : NULL; > > > > /* calculate total, detect system default usage */ > > - for_each_node_mask(node, nodes) > > + for_each_node_mask(node, pol->nodes) > > weight_total += table ? table[node] : 1; > > I have a minor comment on this part. After this change, everything > else reads pol->nodes fresh at the point of use - the weight sum > and the walk both look at the current mask. Only nnodes is still > the count from this earlier read. If the mask changes in between, > the loop bound no longer matches the mask the loop is actually > walking, so the walk can stop short of the pages the weight total > planned for. Would it be better to count the nodes in the loop > that sums the weights, the way weighted_interleave_nid() does it > in this patch? > I don't think this actually fixes anything? But basically the proposal is to widen the SRCU() window further to include the cpuset cookie entirely. e.g. SRCU() { cpuset_cookie() { for_each_node_mask(node, pol->nodes) weight_total += ... nnodes++; } /* ... snip - single node quick-exit ... */ /* ... actual multi-node bulk allocation ... */ for_each_node_mask(node, pol->nodes) { nr_allocated = __alloc_pages_bulk(gfp, node, ...); /* * At this point, due to a torn read from pol->nodes * we can visit a node that wasn't present previously * or we can skip a node that was present previously. * * In either case, weight_total is the wrong value for * the set of nodes being walked anyway - we are going * to skew in the distribution no matter what. */ } } I'm not sure widening the SRCU window is worth it here, it doesn't actually buy us anything. Also we'd be calculating the weight total every time even when there's a scenario where we quick-exit because the entire allocation fits in the first node in the mask. > nnodes = 0; > for_each_node_mask(node, pol->nodes) { > weight_total += table ? table[node] : 1; > nnodes++; > } > > [...snip...] > > Thanks again for your time. > > Rakie Kim ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths 2026-09-02 14:52 ` Gregory Price @ 2026-09-04 8:01 ` Rakie Kim 0 siblings, 0 replies; 8+ messages in thread From: Rakie Kim @ 2026-09-04 8:01 UTC (permalink / raw) To: Gregory Price Cc: linux-kernel, kernel-team, akpm, david, ziy, matthew.brost, joshua.hahnjy, byungchul, ying.huang, apopple, urezki, chenwandun, linux-mm, kernel_team, Rakie Kim On Wed, 2 Sep 2026 10:52:45 -0400 Gregory Price <gourry@gourry.net> wrote: > I don't think this actually fixes anything? > > But basically the proposal is to widen the SRCU() window further to > include the cpuset cookie entirely. > > e.g. > > SRCU() { > cpuset_cookie() { > for_each_node_mask(node, pol->nodes) > weight_total += ... > nnodes++; > } > > /* ... snip - single node quick-exit ... */ > > /* ... actual multi-node bulk allocation ... */ > for_each_node_mask(node, pol->nodes) { > nr_allocated = __alloc_pages_bulk(gfp, node, ...); > > /* > * At this point, due to a torn read from pol->nodes > * we can visit a node that wasn't present previously > * or we can skip a node that was present previously. > * > * In either case, weight_total is the wrong value for > * the set of nodes being walked anyway - we are going > * to skew in the distribution no matter what. > */ > } > } > > I'm not sure widening the SRCU window is worth it here, it doesn't > actually buy us anything. > > Also we'd be calculating the weight total every time even when there's a > scenario where we quick-exit because the entire allocation fits in the > first node in the mask. Sorry, I did not explain that well. I was not suggesting moving the sum into the cookie loop or widening the SRCU section - only adding the counter to the sum loop where it already is: /* calculate total, detect system default usage */ nnodes = 0; for_each_node_mask(node, pol->nodes) { weight_total += table ? table[node] : 1; nnodes++; } The order of the function does not change, so the quick-exit path still returns before this loop runs - the sum is not computed in that case either way. What I had in mind is the gap between the two reads. Say the policy starts with two nodes, every weight is 10, and 100 pages are requested: /* the mask is {0,1} here */ do { cpuset_mems_cookie = read_mems_allowed_begin(); nnodes = nodes_weight(pol->nodes); /* nnodes = 2 */ } while (read_mems_allowed_retry(cpuset_mems_cookie)); /* a rebind grows the mask to {0,1,2,3} at this point */ /* calculate total, detect system default usage */ for_each_node_mask(node, pol->nodes) weight_total += ...; /* 10 * 4 = 40 */ rounds = rem_pages / weight_total; /* 100 / 40 = 2 */ for (i = 0; i < nnodes; i++) /* bounded by 2 */ ... The distribution is planned from a total of 40, which spreads the 100 pages over four nodes, but the walk is still bounded by the two nodes counted earlier. It places 60 pages and returns, and the caller allocates the remaining 40 one page at a time. The mask does not have to change again during the walk for this to happen. You are right that a torn read during the walk still skews the distribution, and this does not change that. It only removes the case where the bound and the total start out inconsistent. If that is not worth the extra line, I am fine either way. Thanks for looking at it. Rakie Kim ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths 2026-08-29 1:59 [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths Gregory Price 2026-08-29 1:59 ` [PATCH 1/2] mm/mempolicy: use SRCU for the weighted interleave state Gregory Price 2026-08-29 1:59 ` [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths Gregory Price @ 2026-08-29 23:18 ` Andrew Morton 2026-08-30 16:39 ` Gregory Price 2 siblings, 1 reply; 8+ messages in thread From: Andrew Morton @ 2026-08-29 23:18 UTC (permalink / raw) To: Gregory Price Cc: linux-mm, linux-kernel, kernel-team, david, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, ying.huang, apopple, urezki, chenwandun On Fri, 28 Aug 2026 21:59:41 -0400 Gregory Price <gourry@gourry.net> wrote: > The interleave node selectors and bulk allocators take copies of > nodemasks and node weights (for weighted interleave) in the fault path. > Both of these copies can be entirely eliminated. > > For node weights, use SRCU to pin the weights in place. This eliminates > a copy and a kmalloc from the bulk allocator path. > > For nodemasks, we can operate directly on pol->nodes as long as we bounds > check the walk. A concurrent rebind can shrink the mask, or tear the read > of it so the mask appears empty. > > - The interleave node selectors fall back to numa_node_id() when that > happens, which is what they already did when a copy came back empty. > > - The bulk allocator simply returns what it managed to allocate. > > The node count and weight totals are read separately from the nodemask > walk that consumes them - creating a time-of-check / time-of-use race. > Just clamp the walk to a single pass (number of nodes), and clamp each > bulk allocation chunk to the space left in the request. > > The cost is distribution accuracy during a rebind. The copies never > corrected for that either - they only kept the code from dividing by > zero and overrunning the allocation request. Not very well, it seems. Sashiko thinks there's a div-by-zero in alloc_pages_bulk_interleave(). https://sashiko.dev/#/patchset/20260829015943.1258774-1-gourry@gourry.net > Gregory Price (2): > mm/mempolicy: use SRCU for the weighted interleave state > mm/mempolicy: stop copying the nodemask in the interleave paths Looks nice, thanks - I'll queue it for testing. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths 2026-08-29 23:18 ` [PATCH 0/2] mm/mempolicy: stop copying state " Andrew Morton @ 2026-08-30 16:39 ` Gregory Price 0 siblings, 0 replies; 8+ messages in thread From: Gregory Price @ 2026-08-30 16:39 UTC (permalink / raw) To: Andrew Morton Cc: linux-mm, linux-kernel, kernel-team, david, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, ying.huang, apopple, urezki, chenwandun On Sat, Aug 29, 2026 at 04:18:50PM -0700, Andrew Morton wrote: > On Fri, 28 Aug 2026 21:59:41 -0400 Gregory Price <gourry@gourry.net> wrote: > > > The interleave node selectors and bulk allocators take copies of > > nodemasks and node weights (for weighted interleave) in the fault path. > > Both of these copies can be entirely eliminated. > > > > For node weights, use SRCU to pin the weights in place. This eliminates > > a copy and a kmalloc from the bulk allocator path. > > > > For nodemasks, we can operate directly on pol->nodes as long as we bounds > > check the walk. A concurrent rebind can shrink the mask, or tear the read > > of it so the mask appears empty. > > > > - The interleave node selectors fall back to numa_node_id() when that > > happens, which is what they already did when a copy came back empty. > > > > - The bulk allocator simply returns what it managed to allocate. > > > > The node count and weight totals are read separately from the nodemask > > walk that consumes them - creating a time-of-check / time-of-use race. > > Just clamp the walk to a single pass (number of nodes), and clamp each > > bulk allocation chunk to the space left in the request. > > > > The cost is distribution accuracy during a rebind. The copies never > > corrected for that either - they only kept the code from dividing by > > zero and overrunning the allocation request. > > Not very well, it seems. Sashiko thinks there's a div-by-zero in > alloc_pages_bulk_interleave(). > > https://sashiko.dev/#/patchset/20260829015943.1258774-1-gourry@gourry.net > That's what this was for :] https://lore.kernel.org/linux-mm/20260828193111.1023497-1-gourry@gourry.net/ ~Gregory ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-04 8:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-29 1:59 [PATCH 0/2] mm/mempolicy: stop copying state in the interleave paths Gregory Price 2026-08-29 1:59 ` [PATCH 1/2] mm/mempolicy: use SRCU for the weighted interleave state Gregory Price 2026-08-29 1:59 ` [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths Gregory Price 2026-09-02 9:00 ` Rakie Kim 2026-09-02 14:52 ` Gregory Price 2026-09-04 8:01 ` Rakie Kim 2026-08-29 23:18 ` [PATCH 0/2] mm/mempolicy: stop copying state " Andrew Morton 2026-08-30 16:39 ` Gregory Price
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox