* [PATCH] percpu: Fix hint invariant breakage
@ 2026-03-20 11:52 Joonwon Kang
2026-03-20 19:08 ` Andrew Morton
2026-03-21 17:09 ` Dennis Zhou
0 siblings, 2 replies; 6+ messages in thread
From: Joonwon Kang @ 2026-03-20 11:52 UTC (permalink / raw)
To: dennis, tj, cl; +Cc: akpm, linux-mm, linux-kernel, Joonwon Kang
The invariant "scan_hint_start > contig_hint_start if and only if
scan_hint == contig_hint" should be kept for hint management. However,
it could be broken in some cases:
- if (new contig == contig_hint == scan_hint) && (contig_hint_start <
scan_hint_start < new contig start) && the new contig is to become a
new contig_hint due to its better alignment, then scan_hint should
be invalidated instead of keeping it.
- if (new contig == contig_hint > scan_hint) && (start <
contig_hint_start) && the new contig is not to become a new
contig_hint, then scan_hint should be invalidated instead of being
updated to the new contig.
This commit fixes this invariant breakage and also optimizes scan_hint
by keeping it or updating it when acceptable:
- if (new contig > contig_hint > scan_hint) && (scan_hint_start < new
contig start < contig_hint_start), then keep scan_hint instead of
invalidating it.
- if (new contig > contig_hint == scan_hint) && (contig_hint_start <
new contig start < scan_hint_start), then update scan_hint to the
old contig_hint instead of invalidating it.
- if (new contig == contig_hint > scan_hint) && (new contig start <
contig_hint_start) && the new contig is to become a new contig_hint
due to its better alignment, then update scan_hint to the old
contig_hint instead of invalidating or keeping it.
Signed-off-by: Joonwon Kang <joonwonkang@google.com>
---
mm/percpu.c | 60 ++++++++++++++++++++++++++++++++++-------------------
1 file changed, 39 insertions(+), 21 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index 81462ce5866e..a0e4f8acb7c2 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -641,19 +641,13 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
if (contig > block->contig_hint) {
/* promote the old contig_hint to be the new scan_hint */
if (start > block->contig_hint_start) {
- if (block->contig_hint > block->scan_hint) {
+ if (block->contig_hint > block->scan_hint ||
+ start < block->scan_hint_start) {
block->scan_hint_start =
block->contig_hint_start;
block->scan_hint = block->contig_hint;
- } else if (start < block->scan_hint_start) {
- /*
- * The old contig_hint == scan_hint. But, the
- * new contig is larger so hold the invariant
- * scan_hint_start < contig_hint_start.
- */
- block->scan_hint = 0;
}
- } else {
+ } else if (start < block->scan_hint_start) {
block->scan_hint = 0;
}
block->contig_hint_start = start;
@@ -662,20 +656,44 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
if (block->contig_hint_start &&
(!start ||
__ffs(start) > __ffs(block->contig_hint_start))) {
+ if (block->contig_hint > block->scan_hint) {
+ if (start < block->contig_hint_start) {
+ block->scan_hint = block->contig_hint;
+ block->scan_hint_start = block->contig_hint_start;
+ }
+ } else if (start > block->scan_hint_start) {
+ /*
+ * old contig_hint == old scan_hint == contig.
+ * But, the new contig is farther than the old
+ * scan_hint so hold the invariant
+ * scan_hint_start > contig_hint_start iff
+ * scan_hint == contig_hint.
+ */
+ block->scan_hint = 0;
+ }
+
/* start has a better alignment so use it */
block->contig_hint_start = start;
- if (start < block->scan_hint_start &&
- block->contig_hint > block->scan_hint)
- block->scan_hint = 0;
- } else if (start > block->scan_hint_start ||
- block->contig_hint > block->scan_hint) {
- /*
- * Knowing contig == contig_hint, update the scan_hint
- * if it is farther than or larger than the current
- * scan_hint.
- */
- block->scan_hint_start = start;
- block->scan_hint = contig;
+ } else {
+ if (block->contig_hint > block->scan_hint) {
+ if (start < block->contig_hint_start) {
+ /*
+ * old scan_hint < contig == old
+ * contig_hint. But, the new contig is
+ * before the old contig_hint so hold
+ * the invariant
+ * scan_hint_start > contig_hint_start
+ * iff scan_hint == contig_hint.
+ */
+ block->scan_hint = 0;
+ } else {
+ block->scan_hint_start = start;
+ block->scan_hint = contig;
+ }
+ } else if (start > block->scan_hint_start) {
+ block->scan_hint_start = start;
+ block->scan_hint = contig;
+ }
}
} else {
/*
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] percpu: Fix hint invariant breakage
2026-03-20 11:52 [PATCH] percpu: Fix hint invariant breakage Joonwon Kang
@ 2026-03-20 19:08 ` Andrew Morton
2026-03-23 12:02 ` Joonwon Kang
2026-03-21 17:09 ` Dennis Zhou
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2026-03-20 19:08 UTC (permalink / raw)
To: Joonwon Kang; +Cc: dennis, tj, cl, linux-mm, linux-kernel
On Fri, 20 Mar 2026 11:52:14 +0000 Joonwon Kang <joonwonkang@google.com> wrote:
> The invariant "scan_hint_start > contig_hint_start if and only if
> scan_hint == contig_hint" should be kept for hint management. However,
> it could be broken in some cases:
>
> - if (new contig == contig_hint == scan_hint) && (contig_hint_start <
> scan_hint_start < new contig start) && the new contig is to become a
> new contig_hint due to its better alignment, then scan_hint should
> be invalidated instead of keeping it.
>
> - if (new contig == contig_hint > scan_hint) && (start <
> contig_hint_start) && the new contig is not to become a new
> contig_hint, then scan_hint should be invalidated instead of being
> updated to the new contig.
>
> This commit fixes this invariant breakage and also optimizes scan_hint
> by keeping it or updating it when acceptable:
>
> - if (new contig > contig_hint > scan_hint) && (scan_hint_start < new
> contig start < contig_hint_start), then keep scan_hint instead of
> invalidating it.
>
> - if (new contig > contig_hint == scan_hint) && (contig_hint_start <
> new contig start < scan_hint_start), then update scan_hint to the
> old contig_hint instead of invalidating it.
>
> - if (new contig == contig_hint > scan_hint) && (new contig start <
> contig_hint_start) && the new contig is to become a new contig_hint
> due to its better alignment, then update scan_hint to the old
> contig_hint instead of invalidating or keeping it.
>
Thanks. Does the change have any observable runtime effects?
Was this patch inspired by code inspection?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] percpu: Fix hint invariant breakage
2026-03-20 11:52 [PATCH] percpu: Fix hint invariant breakage Joonwon Kang
2026-03-20 19:08 ` Andrew Morton
@ 2026-03-21 17:09 ` Dennis Zhou
2026-03-23 14:05 ` Joonwon Kang
1 sibling, 1 reply; 6+ messages in thread
From: Dennis Zhou @ 2026-03-21 17:09 UTC (permalink / raw)
To: Joonwon Kang; +Cc: tj, cl, akpm, linux-mm, linux-kernel
Hello,
On Fri, Mar 20, 2026 at 11:52:14AM +0000, Joonwon Kang wrote:
> The invariant "scan_hint_start > contig_hint_start if and only if
> scan_hint == contig_hint" should be kept for hint management. However,
> it could be broken in some cases:
>
First I'd just like to apologize. I spent an hour yesterday trying to
remember why the invariant exists and the reality is this code is more
clever than it needs to be.
As Andrew asked, how did you come across this? It's pretty obscure so
thank you for taking the time to look at it.
> - if (new contig == contig_hint == scan_hint) && (contig_hint_start <
> scan_hint_start < new contig start) && the new contig is to become a
> new contig_hint due to its better alignment, then scan_hint should
> be invalidated instead of keeping it.
>
> - if (new contig == contig_hint > scan_hint) && (start <
> contig_hint_start) && the new contig is not to become a new
> contig_hint, then scan_hint should be invalidated instead of being
> updated to the new contig.
>
> This commit fixes this invariant breakage and also optimizes scan_hint
> by keeping it or updating it when acceptable:
>
> - if (new contig > contig_hint > scan_hint) && (scan_hint_start < new
> contig start < contig_hint_start), then keep scan_hint instead of
> invalidating it.
>
> - if (new contig > contig_hint == scan_hint) && (contig_hint_start <
> new contig start < scan_hint_start), then update scan_hint to the
> old contig_hint instead of invalidating it.
>
> - if (new contig == contig_hint > scan_hint) && (new contig start <
> contig_hint_start) && the new contig is to become a new contig_hint
> due to its better alignment, then update scan_hint to the old
> contig_hint instead of invalidating or keeping it.
>
> Signed-off-by: Joonwon Kang <joonwonkang@google.com>
> ---
> mm/percpu.c | 60 ++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 81462ce5866e..a0e4f8acb7c2 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -641,19 +641,13 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
> if (contig > block->contig_hint) {
> /* promote the old contig_hint to be the new scan_hint */
> if (start > block->contig_hint_start) {
> - if (block->contig_hint > block->scan_hint) {
> + if (block->contig_hint > block->scan_hint ||
> + start < block->scan_hint_start) {
I think this should be <=.
Given hints as [hint_start, size].
contig_hint = [64, 64]
scan_hint = [160, 64]
Free [224, 32].
Without <=, we don't promote the contig_hint and leave the stale
scan_hint.
> block->scan_hint_start =
> block->contig_hint_start;
> block->scan_hint = block->contig_hint;
> - } else if (start < block->scan_hint_start) {
> - /*
> - * The old contig_hint == scan_hint. But, the
> - * new contig is larger so hold the invariant
> - * scan_hint_start < contig_hint_start.
> - */
> - block->scan_hint = 0;
> }
> - } else {
> + } else if (start < block->scan_hint_start) {
I think this too should be <=.
scan_hint = [16, 8]
contig_hint = [32, 96]
free [24, 8]
scan_hint stays [16, 8] instead of being cleared.
> block->scan_hint = 0;
> }
> block->contig_hint_start = start;
> @@ -662,20 +656,44 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
> if (block->contig_hint_start &&
> (!start ||
> __ffs(start) > __ffs(block->contig_hint_start))) {
> + if (block->contig_hint > block->scan_hint) {
> + if (start < block->contig_hint_start) {
> + block->scan_hint = block->contig_hint;
> + block->scan_hint_start = block->contig_hint_start;
> + }
> + } else if (start > block->scan_hint_start) {
> + /*
> + * old contig_hint == old scan_hint == contig.
> + * But, the new contig is farther than the old
> + * scan_hint so hold the invariant
> + * scan_hint_start > contig_hint_start iff
> + * scan_hint == contig_hint.
> + */
> + block->scan_hint = 0;
> + }
> +
> /* start has a better alignment so use it */
> block->contig_hint_start = start;
> - if (start < block->scan_hint_start &&
> - block->contig_hint > block->scan_hint)
> - block->scan_hint = 0;
> - } else if (start > block->scan_hint_start ||
> - block->contig_hint > block->scan_hint) {
> - /*
> - * Knowing contig == contig_hint, update the scan_hint
> - * if it is farther than or larger than the current
> - * scan_hint.
> - */
> - block->scan_hint_start = start;
> - block->scan_hint = contig;
> + } else {
> + if (block->contig_hint > block->scan_hint) {
> + if (start < block->contig_hint_start) {
> + /*
> + * old scan_hint < contig == old
> + * contig_hint. But, the new contig is
> + * before the old contig_hint so hold
> + * the invariant
> + * scan_hint_start > contig_hint_start
> + * iff scan_hint == contig_hint.
> + */
> + block->scan_hint = 0;
> + } else {
> + block->scan_hint_start = start;
> + block->scan_hint = contig;
> + }
> + } else if (start > block->scan_hint_start) {
> + block->scan_hint_start = start;
> + block->scan_hint = contig;
> + }
> }
> } else {
> /*
> --
> 2.53.0.1018.g2bb0e51243-goog
>
Ultimately as I re-read this code, it might be nice to rewrite it so
that scan_hint can be kept separately. The code is a little too clever
with trying to avoid stating new_region overlaps scan_hint or
contig_hint.
I recently started shimming out the bitmap code in userspace so
hopefully I can test it for performance / correctness more rigorously.
Thanks,
Dennis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] percpu: Fix hint invariant breakage
2026-03-20 19:08 ` Andrew Morton
@ 2026-03-23 12:02 ` Joonwon Kang
0 siblings, 0 replies; 6+ messages in thread
From: Joonwon Kang @ 2026-03-23 12:02 UTC (permalink / raw)
To: akpm; +Cc: cl, dennis, joonwonkang, linux-kernel, linux-mm, tj
> On Fri, 20 Mar 2026 11:52:14 +0000 Joonwon Kang <joonwonkang@google.com> wrote:
>
> > The invariant "scan_hint_start > contig_hint_start if and only if
> > scan_hint == contig_hint" should be kept for hint management. However,
> > it could be broken in some cases:
> >
> > - if (new contig == contig_hint == scan_hint) && (contig_hint_start <
> > scan_hint_start < new contig start) && the new contig is to become a
> > new contig_hint due to its better alignment, then scan_hint should
> > be invalidated instead of keeping it.
> >
> > - if (new contig == contig_hint > scan_hint) && (start <
> > contig_hint_start) && the new contig is not to become a new
> > contig_hint, then scan_hint should be invalidated instead of being
> > updated to the new contig.
> >
> > This commit fixes this invariant breakage and also optimizes scan_hint
> > by keeping it or updating it when acceptable:
> >
> > - if (new contig > contig_hint > scan_hint) && (scan_hint_start < new
> > contig start < contig_hint_start), then keep scan_hint instead of
> > invalidating it.
> >
> > - if (new contig > contig_hint == scan_hint) && (contig_hint_start <
> > new contig start < scan_hint_start), then update scan_hint to the
> > old contig_hint instead of invalidating it.
> >
> > - if (new contig == contig_hint > scan_hint) && (new contig start <
> > contig_hint_start) && the new contig is to become a new contig_hint
> > due to its better alignment, then update scan_hint to the old
> > contig_hint instead of invalidating or keeping it.
> >
>
> Thanks. Does the change have any observable runtime effects?
>
> Was this patch inspired by code inspection?
Yes, this change basically comes from manual code review. While there is
no benchmark result or others backing up the effects of this change,
Dennis may help on that later as he suggested in another review. The main
point of this patch should rather be fixing the "theoretical" invariant
breakage cases.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] percpu: Fix hint invariant breakage
2026-03-21 17:09 ` Dennis Zhou
@ 2026-03-23 14:05 ` Joonwon Kang
2026-04-09 18:09 ` Dennis Zhou
0 siblings, 1 reply; 6+ messages in thread
From: Joonwon Kang @ 2026-03-23 14:05 UTC (permalink / raw)
To: dennis; +Cc: akpm, cl, joonwonkang, linux-kernel, linux-mm, tj, dodam
> Hello,
>
> On Fri, Mar 20, 2026 at 11:52:14AM +0000, Joonwon Kang wrote:
> > The invariant "scan_hint_start > contig_hint_start if and only if
> > scan_hint == contig_hint" should be kept for hint management. However,
> > it could be broken in some cases:
> >
>
> First I'd just like to apologize. I spent an hour yesterday trying to
> remember why the invariant exists and the reality is this code is more
> clever than it needs to be.
Thanks for taking time for this and sharing more context. While you are at
it, I have a fundamental question on the invariant. I had deliberation and
discussion on what benefits the invariant gets to the percpu allocator by
its existence. My understanding is that if we put contig_hint before
scan_hint when they are the same, it is more likely that contig_hint is
broken by a future allocation, which leads to a linear scan after the
scan_hint for hints update, although we could save scanning upto scan_hint
when contig_hint is not broken. On the other hand, if we put scan_hint
before contig_hint instead, it is more likely that scan_hint is broken
while keeping contig_hint, which does not lead to the linear scan for
hints update, although we could not save the scanning that could be saved
in the other case.
In other words, if contig_hint breaking allocations occur a lot in general
with the current invariant, the performance may more suffer than without
the invariant. I also think that there would be no strict reason of having
the invariant.
So, could you clarify the necessity of the invariant? If there is no must
reason, then I could post another spin-off patch to remove the invariant
at all so that we could simplify the code and experiment the result. How
do you think?
>
> As Andrew asked, how did you come across this? It's pretty obscure so
> thank you for taking the time to look at it.
I came across this issue by manual code review and thanks for saying that.
>
>
> > - if (new contig == contig_hint == scan_hint) && (contig_hint_start <
> > scan_hint_start < new contig start) && the new contig is to become a
> > new contig_hint due to its better alignment, then scan_hint should
> > be invalidated instead of keeping it.
> >
> > - if (new contig == contig_hint > scan_hint) && (start <
> > contig_hint_start) && the new contig is not to become a new
> > contig_hint, then scan_hint should be invalidated instead of being
> > updated to the new contig.
> >
> > This commit fixes this invariant breakage and also optimizes scan_hint
> > by keeping it or updating it when acceptable:
> >
> > - if (new contig > contig_hint > scan_hint) && (scan_hint_start < new
> > contig start < contig_hint_start), then keep scan_hint instead of
> > invalidating it.
> >
> > - if (new contig > contig_hint == scan_hint) && (contig_hint_start <
> > new contig start < scan_hint_start), then update scan_hint to the
> > old contig_hint instead of invalidating it.
> >
> > - if (new contig == contig_hint > scan_hint) && (new contig start <
> > contig_hint_start) && the new contig is to become a new contig_hint
> > due to its better alignment, then update scan_hint to the old
> > contig_hint instead of invalidating or keeping it.
> >
> > Signed-off-by: Joonwon Kang <joonwonkang@google.com>
> > ---
> > mm/percpu.c | 60 ++++++++++++++++++++++++++++++++++-------------------
> > 1 file changed, 39 insertions(+), 21 deletions(-)
> >
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index 81462ce5866e..a0e4f8acb7c2 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -641,19 +641,13 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
> > if (contig > block->contig_hint) {
> > /* promote the old contig_hint to be the new scan_hint */
> > if (start > block->contig_hint_start) {
> > - if (block->contig_hint > block->scan_hint) {
> > + if (block->contig_hint > block->scan_hint ||
> > + start < block->scan_hint_start) {
>
> I think this should be <=.
> Given hints as [hint_start, size].
>
> contig_hint = [64, 64]
> scan_hint = [160, 64]
>
> Free [224, 32].
>
> Without <=, we don't promote the contig_hint and leave the stale
> scan_hint.
Ah, I missed the fact that the new contig could be given overlapping with
other hints. Will reconsider the cases and fix it. Thanks.
>
> > block->scan_hint_start =
> > block->contig_hint_start;
> > block->scan_hint = block->contig_hint;
> > - } else if (start < block->scan_hint_start) {
> > - /*
> > - * The old contig_hint == scan_hint. But, the
> > - * new contig is larger so hold the invariant
> > - * scan_hint_start < contig_hint_start.
> > - */
> > - block->scan_hint = 0;
> > }
> > - } else {
> > + } else if (start < block->scan_hint_start) {
>
> I think this too should be <=.
>
> scan_hint = [16, 8]
> contig_hint = [32, 96]
>
> free [24, 8]
>
> scan_hint stays [16, 8] instead of being cleared.
Will fix it, thanks.
>
>
> > block->scan_hint = 0;
> > }
> > block->contig_hint_start = start;
> > @@ -662,20 +656,44 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
> > if (block->contig_hint_start &&
> > (!start ||
> > __ffs(start) > __ffs(block->contig_hint_start))) {
> > + if (block->contig_hint > block->scan_hint) {
> > + if (start < block->contig_hint_start) {
> > + block->scan_hint = block->contig_hint;
> > + block->scan_hint_start = block->contig_hint_start;
> > + }
> > + } else if (start > block->scan_hint_start) {
> > + /*
> > + * old contig_hint == old scan_hint == contig.
> > + * But, the new contig is farther than the old
> > + * scan_hint so hold the invariant
> > + * scan_hint_start > contig_hint_start iff
> > + * scan_hint == contig_hint.
> > + */
> > + block->scan_hint = 0;
> > + }
> > +
> > /* start has a better alignment so use it */
> > block->contig_hint_start = start;
> > - if (start < block->scan_hint_start &&
> > - block->contig_hint > block->scan_hint)
> > - block->scan_hint = 0;
> > - } else if (start > block->scan_hint_start ||
> > - block->contig_hint > block->scan_hint) {
> > - /*
> > - * Knowing contig == contig_hint, update the scan_hint
> > - * if it is farther than or larger than the current
> > - * scan_hint.
> > - */
> > - block->scan_hint_start = start;
> > - block->scan_hint = contig;
> > + } else {
> > + if (block->contig_hint > block->scan_hint) {
> > + if (start < block->contig_hint_start) {
> > + /*
> > + * old scan_hint < contig == old
> > + * contig_hint. But, the new contig is
> > + * before the old contig_hint so hold
> > + * the invariant
> > + * scan_hint_start > contig_hint_start
> > + * iff scan_hint == contig_hint.
> > + */
> > + block->scan_hint = 0;
> > + } else {
> > + block->scan_hint_start = start;
> > + block->scan_hint = contig;
> > + }
> > + } else if (start > block->scan_hint_start) {
> > + block->scan_hint_start = start;
> > + block->scan_hint = contig;
> > + }
> > }
> > } else {
> > /*
> > --
> > 2.53.0.1018.g2bb0e51243-goog
> >
>
> Ultimately as I re-read this code, it might be nice to rewrite it so
> that scan_hint can be kept separately. The code is a little too clever
> with trying to avoid stating new_region overlaps scan_hint or
> contig_hint.
>
> I recently started shimming out the bitmap code in userspace so
> hopefully I can test it for performance / correctness more rigorously.
Thanks for letting me know of this. It would be great if we could test
this subtle change around the invariant and the hints.
>
> Thanks,
> Dennis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] percpu: Fix hint invariant breakage
2026-03-23 14:05 ` Joonwon Kang
@ 2026-04-09 18:09 ` Dennis Zhou
0 siblings, 0 replies; 6+ messages in thread
From: Dennis Zhou @ 2026-04-09 18:09 UTC (permalink / raw)
To: Joonwon Kang; +Cc: dennis, akpm, cl, linux-kernel, linux-mm, tj, dodam
Hello,
Sorry for the delay, I've been a bit sick.
On Mon, Mar 23, 2026 at 02:05:14PM +0000, Joonwon Kang wrote:
> > Hello,
> >
> > On Fri, Mar 20, 2026 at 11:52:14AM +0000, Joonwon Kang wrote:
> > > The invariant "scan_hint_start > contig_hint_start if and only if
> > > scan_hint == contig_hint" should be kept for hint management. However,
> > > it could be broken in some cases:
> > >
> >
> > First I'd just like to apologize. I spent an hour yesterday trying to
> > remember why the invariant exists and the reality is this code is more
> > clever than it needs to be.
>
> Thanks for taking time for this and sharing more context. While you are at
> it, I have a fundamental question on the invariant. I had deliberation and
> discussion on what benefits the invariant gets to the percpu allocator by
> its existence. My understanding is that if we put contig_hint before
> scan_hint when they are the same, it is more likely that contig_hint is
> broken by a future allocation, which leads to a linear scan after the
> scan_hint for hints update, although we could save scanning upto scan_hint
> when contig_hint is not broken. On the other hand, if we put scan_hint
> before contig_hint instead, it is more likely that scan_hint is broken
> while keeping contig_hint, which does not lead to the linear scan for
> hints update, although we could not save the scanning that could be saved
> in the other case.
>
> In other words, if contig_hint breaking allocations occur a lot in general
> with the current invariant, the performance may more suffer than without
> the invariant. I also think that there would be no strict reason of having
> the invariant.
>
I think the original premise is that percpu memory is quite expensive, 1
allocation costs nr_cpus * sizeof(allocation). So we do our best to bin
pack at the cost of faster allocations. We could always just break the
contig_hint but then over time we could cause more fragmentation.
The case that triggered this was netdev needing 8 byte objects with 16
byte alignment [1].
> So, could you clarify the necessity of the invariant? If there is no must
> reason, then I could post another spin-off patch to remove the invariant
> at all so that we could simplify the code and experiment the result. How
> do you think?
>
I can't really recall the exact reasoning for the invariant, but it was
probably along the lines of wanting to not lose information if possible.
Say an earlier area becomes free that is the same size as the
contig_hint but with better alignment, we ant to use that as the
contig_hint but then we either have to lose the scan_hint or keep it
with the invariant. Given the premise above, I believe we want to
continue bin packing, I think the general idea of scanning next time
around isn't the worst thing.
Sadly because it's already there, and has worked for quite some time,
it's kind of on us today to provide data / reasoning to delete it. I'd
wager that some upcoming work is going to change how percpu gives out
objects either through some sort of slab caching that we can revisit
this more in that context.
Thanks,
Dennis
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-09 18:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-20 11:52 [PATCH] percpu: Fix hint invariant breakage Joonwon Kang
2026-03-20 19:08 ` Andrew Morton
2026-03-23 12:02 ` Joonwon Kang
2026-03-21 17:09 ` Dennis Zhou
2026-03-23 14:05 ` Joonwon Kang
2026-04-09 18:09 ` Dennis Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox