After xas_load(), xas->index could point to mid of found multi-index entry
and xas->index's bits under node->shift maybe non-zero. The afterward
xas_pause() will move forward xas->index with xa->node->shift with bits
under node->shift un-masked and thus skip some index unexpectedly.
Consider following case:
Assume XA_CHUNK_SHIFT is 4.
xa_store_range(xa, 16, 31, ...)
xa_store(xa, 32, ...)
XA_STATE(xas, xa, 17);
xas_for_each(&xas,...)
xas_load(&xas)
/* xas->index = 17, xas->xa_offset = 1, xas->xa_node->xa_shift = 4 */
xas_pause()
/* xas->index = 33, xas->xa_offset = 2, xas->xa_node->xa_shift = 4 */
As we can see, index of 32 is skipped unexpectedly.
Fix this by mask bit under node->xa_shift when move forward index in
xas_pause().
For now, this will not cause serious problems. Only minor problem
like cachestat return less number of page status could happen.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
lib/test_xarray.c | 35 +++++++++++++++++++++++++++++++++++
lib/xarray.c | 1 +
2 files changed, 36 insertions(+)
diff --git a/lib/test_xarray.c b/lib/test_xarray.c
index d5c5cbba33ed..6932a26f4927 100644
--- a/lib/test_xarray.c
+++ b/lib/test_xarray.c
@@ -1448,6 +1448,41 @@ static noinline void check_pause(struct xarray *xa)
XA_BUG_ON(xa, count != order_limit);
xa_destroy(xa);
+
+ index = 0;
+ for (order = XA_CHUNK_SHIFT; order > 0; order--) {
+ XA_BUG_ON(xa, xa_store_order(xa, index, order,
+ xa_mk_index(index), GFP_KERNEL));
+ index += 1UL << order;
+ }
+
+ index = 0;
+ count = 0;
+ xas_set(&xas, 0);
+ rcu_read_lock();
+ xas_for_each(&xas, entry, ULONG_MAX) {
+ XA_BUG_ON(xa, entry != xa_mk_index(index));
+ index += 1UL << (XA_CHUNK_SHIFT - count);
+ count++;
+ }
+ rcu_read_unlock();
+ XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
+
+ index = 0;
+ count = 0;
+ xas_set(&xas, XA_CHUNK_SIZE / 2 + 1);
+ rcu_read_lock();
+ xas_for_each(&xas, entry, ULONG_MAX) {
+ XA_BUG_ON(xa, entry != xa_mk_index(index));
+ index += 1UL << (XA_CHUNK_SHIFT - count);
+ count++;
+ xas_pause(&xas);
+ }
+ rcu_read_unlock();
+ XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
+
+ xa_destroy(xa);
+
}
static noinline void check_move_tiny(struct xarray *xa)
diff --git a/lib/xarray.c b/lib/xarray.c
index fa87949719a0..d0732c5b8403 100644
--- a/lib/xarray.c
+++ b/lib/xarray.c
@@ -1147,6 +1147,7 @@ void xas_pause(struct xa_state *xas)
if (!xa_is_sibling(xa_entry(xas->xa, node, offset)))
break;
}
+ xas->xa_index &= ~0UL << node->shift;
xas->xa_index += (offset - xas->xa_offset) << node->shift;
if (xas->xa_index == 0)
xas->xa_node = XAS_BOUNDS;
--
2.30.0Hi Kemeng,
On Wed, 18 Dec 2024 at 07:58, Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> After xas_load(), xas->index could point to mid of found multi-index entry
> and xas->index's bits under node->shift maybe non-zero. The afterward
> xas_pause() will move forward xas->index with xa->node->shift with bits
> under node->shift un-masked and thus skip some index unexpectedly.
>
> Consider following case:
> Assume XA_CHUNK_SHIFT is 4.
> xa_store_range(xa, 16, 31, ...)
> xa_store(xa, 32, ...)
> XA_STATE(xas, xa, 17);
> xas_for_each(&xas,...)
> xas_load(&xas)
> /* xas->index = 17, xas->xa_offset = 1, xas->xa_node->xa_shift = 4 */
> xas_pause()
> /* xas->index = 33, xas->xa_offset = 2, xas->xa_node->xa_shift = 4 */
> As we can see, index of 32 is skipped unexpectedly.
>
> Fix this by mask bit under node->xa_shift when move forward index in
> xas_pause().
>
> For now, this will not cause serious problems. Only minor problem
> like cachestat return less number of page status could happen.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Thanks for your patch, which is now commit c9ba5249ef8b080c ("Xarray:
move forward index correctly in xas_pause()") upstream.
> --- a/lib/test_xarray.c
> +++ b/lib/test_xarray.c
> @@ -1448,6 +1448,41 @@ static noinline void check_pause(struct xarray *xa)
> XA_BUG_ON(xa, count != order_limit);
>
> xa_destroy(xa);
> +
> + index = 0;
> + for (order = XA_CHUNK_SHIFT; order > 0; order--) {
> + XA_BUG_ON(xa, xa_store_order(xa, index, order,
> + xa_mk_index(index), GFP_KERNEL));
> + index += 1UL << order;
> + }
> +
> + index = 0;
> + count = 0;
> + xas_set(&xas, 0);
> + rcu_read_lock();
> + xas_for_each(&xas, entry, ULONG_MAX) {
> + XA_BUG_ON(xa, entry != xa_mk_index(index));
> + index += 1UL << (XA_CHUNK_SHIFT - count);
> + count++;
> + }
> + rcu_read_unlock();
> + XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
> +
> + index = 0;
> + count = 0;
> + xas_set(&xas, XA_CHUNK_SIZE / 2 + 1);
> + rcu_read_lock();
> + xas_for_each(&xas, entry, ULONG_MAX) {
> + XA_BUG_ON(xa, entry != xa_mk_index(index));
> + index += 1UL << (XA_CHUNK_SHIFT - count);
> + count++;
> + xas_pause(&xas);
> + }
> + rcu_read_unlock();
> + XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
> +
> + xa_destroy(xa);
> +
> }
On m68k, the last four XA_BUG_ON() checks above are triggered when
running the test. With extra debug prints added:
entry = 00000002 xa_mk_index(index) = 000000c1
entry = 00000002 xa_mk_index(index) = 000000e1
entry = 00000002 xa_mk_index(index) = 000000f1
...
entry = 000000e2 xa_mk_index(index) = fffff0ff
entry = 000000f9 xa_mk_index(index) = fffff8ff
entry = 000000f2 xa_mk_index(index) = fffffcff
count = 63 XA_CHUNK_SHIFT = 6
entry = 00000081 xa_mk_index(index) = 00000001
entry = 00000002 xa_mk_index(index) = 00000081
entry = 00000002 xa_mk_index(index) = 000000c1
...
entry = 000000e2 xa_mk_index(index) = ffffe0ff
entry = 000000f9 xa_mk_index(index) = fffff0ff
entry = 000000f2 xa_mk_index(index) = fffff8ff
count = 62 XA_CHUNK_SHIFT = 6
On arm32, the test succeeds, so it's probably not a 32-vs-64-bit issue.
Perhaps a big-endian or alignment issue (alignof(int/long) = 2)?
> --- a/lib/xarray.c
> +++ b/lib/xarray.c
> @@ -1147,6 +1147,7 @@ void xas_pause(struct xa_state *xas)
> if (!xa_is_sibling(xa_entry(xas->xa, node, offset)))
> break;
> }
> + xas->xa_index &= ~0UL << node->shift;
> xas->xa_index += (offset - xas->xa_offset) << node->shift;
> if (xas->xa_index == 0)
> xas->xa_node = XAS_BOUNDS;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
on 1/28/2025 12:21 AM, Geert Uytterhoeven wrote:
> Hi Kemeng,
>
> On Wed, 18 Dec 2024 at 07:58, Kemeng Shi <shikemeng@huaweicloud.com> wrote:
>> After xas_load(), xas->index could point to mid of found multi-index entry
>> and xas->index's bits under node->shift maybe non-zero. The afterward
>> xas_pause() will move forward xas->index with xa->node->shift with bits
>> under node->shift un-masked and thus skip some index unexpectedly.
>>
>> Consider following case:
>> Assume XA_CHUNK_SHIFT is 4.
>> xa_store_range(xa, 16, 31, ...)
>> xa_store(xa, 32, ...)
>> XA_STATE(xas, xa, 17);
>> xas_for_each(&xas,...)
>> xas_load(&xas)
>> /* xas->index = 17, xas->xa_offset = 1, xas->xa_node->xa_shift = 4 */
>> xas_pause()
>> /* xas->index = 33, xas->xa_offset = 2, xas->xa_node->xa_shift = 4 */
>> As we can see, index of 32 is skipped unexpectedly.
>>
>> Fix this by mask bit under node->xa_shift when move forward index in
>> xas_pause().
>>
>> For now, this will not cause serious problems. Only minor problem
>> like cachestat return less number of page status could happen.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
>
> Thanks for your patch, which is now commit c9ba5249ef8b080c ("Xarray:
> move forward index correctly in xas_pause()") upstream.
>
>> --- a/lib/test_xarray.c
>> +++ b/lib/test_xarray.c
>> @@ -1448,6 +1448,41 @@ static noinline void check_pause(struct xarray *xa)
>> XA_BUG_ON(xa, count != order_limit);
>>
>> xa_destroy(xa);
>> +
>> + index = 0;
>> + for (order = XA_CHUNK_SHIFT; order > 0; order--) {
>> + XA_BUG_ON(xa, xa_store_order(xa, index, order,
>> + xa_mk_index(index), GFP_KERNEL));
>> + index += 1UL << order;
>> + }
>> +
>> + index = 0;
>> + count = 0;
>> + xas_set(&xas, 0);
>> + rcu_read_lock();
>> + xas_for_each(&xas, entry, ULONG_MAX) {
>> + XA_BUG_ON(xa, entry != xa_mk_index(index));
>> + index += 1UL << (XA_CHUNK_SHIFT - count);
>> + count++;
>> + }
>> + rcu_read_unlock();
>> + XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
>> +
>> + index = 0;
>> + count = 0;
>> + xas_set(&xas, XA_CHUNK_SIZE / 2 + 1);
>> + rcu_read_lock();
>> + xas_for_each(&xas, entry, ULONG_MAX) {
>> + XA_BUG_ON(xa, entry != xa_mk_index(index));
>> + index += 1UL << (XA_CHUNK_SHIFT - count);
>> + count++;
>> + xas_pause(&xas);
>> + }
>> + rcu_read_unlock();
>> + XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
>> +
>> + xa_destroy(xa);
>> +
>> }
>
> On m68k, the last four XA_BUG_ON() checks above are triggered when
> running the test. With extra debug prints added:
>
> entry = 00000002 xa_mk_index(index) = 000000c1
> entry = 00000002 xa_mk_index(index) = 000000e1
> entry = 00000002 xa_mk_index(index) = 000000f1
> ...
> entry = 000000e2 xa_mk_index(index) = fffff0ff
> entry = 000000f9 xa_mk_index(index) = fffff8ff
> entry = 000000f2 xa_mk_index(index) = fffffcff
> count = 63 XA_CHUNK_SHIFT = 6
> entry = 00000081 xa_mk_index(index) = 00000001
> entry = 00000002 xa_mk_index(index) = 00000081
> entry = 00000002 xa_mk_index(index) = 000000c1
> ...
> entry = 000000e2 xa_mk_index(index) = ffffe0ff
> entry = 000000f9 xa_mk_index(index) = fffff0ff
> entry = 000000f2 xa_mk_index(index) = fffff8ff
> count = 62 XA_CHUNK_SHIFT = 6
>
> On arm32, the test succeeds, so it's probably not a 32-vs-64-bit issue.
> Perhaps a big-endian or alignment issue (alignof(int/long) = 2)?
Hi Geert,
Sorry for late reply. After check the debug info and the code, I think
the test is failed because CONFIG_XARRAY_MULTI is off. I guess
CONFIG_XARRAY_MULTI is on arm32 and is off on m68k so the test result
diffs. Luckly it's only a problem of of test code.
I will send patch to correct the test code soon. Thanks!
Kemeng
>
>> --- a/lib/xarray.c
>> +++ b/lib/xarray.c
>> @@ -1147,6 +1147,7 @@ void xas_pause(struct xa_state *xas)
>> if (!xa_is_sibling(xa_entry(xas->xa, node, offset)))
>> break;
>> }
>> + xas->xa_index &= ~0UL << node->shift;
>> xas->xa_index += (offset - xas->xa_offset) << node->shift;
>> if (xas->xa_index == 0)
>> xas->xa_node = XAS_BOUNDS;
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
Hi Kemeng,
On Thu, 6 Feb 2025 at 07:13, Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> on 1/28/2025 12:21 AM, Geert Uytterhoeven wrote:
> > On Wed, 18 Dec 2024 at 07:58, Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> >> After xas_load(), xas->index could point to mid of found multi-index entry
> >> and xas->index's bits under node->shift maybe non-zero. The afterward
> >> xas_pause() will move forward xas->index with xa->node->shift with bits
> >> under node->shift un-masked and thus skip some index unexpectedly.
> >>
> >> Consider following case:
> >> Assume XA_CHUNK_SHIFT is 4.
> >> xa_store_range(xa, 16, 31, ...)
> >> xa_store(xa, 32, ...)
> >> XA_STATE(xas, xa, 17);
> >> xas_for_each(&xas,...)
> >> xas_load(&xas)
> >> /* xas->index = 17, xas->xa_offset = 1, xas->xa_node->xa_shift = 4 */
> >> xas_pause()
> >> /* xas->index = 33, xas->xa_offset = 2, xas->xa_node->xa_shift = 4 */
> >> As we can see, index of 32 is skipped unexpectedly.
> >>
> >> Fix this by mask bit under node->xa_shift when move forward index in
> >> xas_pause().
> >>
> >> For now, this will not cause serious problems. Only minor problem
> >> like cachestat return less number of page status could happen.
> >>
> >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> >
> > Thanks for your patch, which is now commit c9ba5249ef8b080c ("Xarray:
> > move forward index correctly in xas_pause()") upstream.
> >
> >> --- a/lib/test_xarray.c
> >> +++ b/lib/test_xarray.c
> >> @@ -1448,6 +1448,41 @@ static noinline void check_pause(struct xarray *xa)
> >> XA_BUG_ON(xa, count != order_limit);
> >>
> >> xa_destroy(xa);
> >> +
> >> + index = 0;
> >> + for (order = XA_CHUNK_SHIFT; order > 0; order--) {
> >> + XA_BUG_ON(xa, xa_store_order(xa, index, order,
> >> + xa_mk_index(index), GFP_KERNEL));
> >> + index += 1UL << order;
> >> + }
> >> +
> >> + index = 0;
> >> + count = 0;
> >> + xas_set(&xas, 0);
> >> + rcu_read_lock();
> >> + xas_for_each(&xas, entry, ULONG_MAX) {
> >> + XA_BUG_ON(xa, entry != xa_mk_index(index));
> >> + index += 1UL << (XA_CHUNK_SHIFT - count);
> >> + count++;
> >> + }
> >> + rcu_read_unlock();
> >> + XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
> >> +
> >> + index = 0;
> >> + count = 0;
> >> + xas_set(&xas, XA_CHUNK_SIZE / 2 + 1);
> >> + rcu_read_lock();
> >> + xas_for_each(&xas, entry, ULONG_MAX) {
> >> + XA_BUG_ON(xa, entry != xa_mk_index(index));
> >> + index += 1UL << (XA_CHUNK_SHIFT - count);
> >> + count++;
> >> + xas_pause(&xas);
> >> + }
> >> + rcu_read_unlock();
> >> + XA_BUG_ON(xa, count != XA_CHUNK_SHIFT);
> >> +
> >> + xa_destroy(xa);
> >> +
> >> }
> >
> > On m68k, the last four XA_BUG_ON() checks above are triggered when
> > running the test. With extra debug prints added:
> >
> > entry = 00000002 xa_mk_index(index) = 000000c1
> > entry = 00000002 xa_mk_index(index) = 000000e1
> > entry = 00000002 xa_mk_index(index) = 000000f1
> > ...
> > entry = 000000e2 xa_mk_index(index) = fffff0ff
> > entry = 000000f9 xa_mk_index(index) = fffff8ff
> > entry = 000000f2 xa_mk_index(index) = fffffcff
> > count = 63 XA_CHUNK_SHIFT = 6
> > entry = 00000081 xa_mk_index(index) = 00000001
> > entry = 00000002 xa_mk_index(index) = 00000081
> > entry = 00000002 xa_mk_index(index) = 000000c1
> > ...
> > entry = 000000e2 xa_mk_index(index) = ffffe0ff
> > entry = 000000f9 xa_mk_index(index) = fffff0ff
> > entry = 000000f2 xa_mk_index(index) = fffff8ff
> > count = 62 XA_CHUNK_SHIFT = 6
> >
> > On arm32, the test succeeds, so it's probably not a 32-vs-64-bit issue.
> > Perhaps a big-endian or alignment issue (alignof(int/long) = 2)?
> Hi Geert,
> Sorry for late reply. After check the debug info and the code, I think
> the test is failed because CONFIG_XARRAY_MULTI is off. I guess
> CONFIG_XARRAY_MULTI is on arm32 and is off on m68k so the test result
> diffs. Luckly it's only a problem of of test code.
> I will send patch to correct the test code soon. Thanks!
You are right: CONFIG_XARRAY_MULTI is enabled in my arm32 build,
but not in my m68k build.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
© 2016 - 2026 Red Hat, Inc.