| Message ID | 20250208011852.31434-4-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | may miss to set node dead on destroy | expand |
* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: > Original code may get a stall value when overwriting the whole range on a ^^^^^- stale value. I thought you were saying it was stalling which did not make sense. > maple tree with only root node. The reason is we didn't set the only > root node dead during destroy. > > Add a test case to verify this is not recreated. > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > CC: Liam R. Howlett <Liam.Howlett@Oracle.com> > --- > tools/testing/radix-tree/maple.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c > index bc30050227fd..1e293e4d856d 100644 > --- a/tools/testing/radix-tree/maple.c > +++ b/tools/testing/radix-tree/maple.c > @@ -35256,6 +35256,30 @@ static noinline void __init check_rcu_simulated(struct maple_tree *mt) > MT_BUG_ON(mt, mas_prev(&mas_reader, 0) != xa_mk_value(val)); > rcu_read_unlock(); > > + /* Clear out tree & create one with only root node */ > + mas_lock(&mas_writer); > + mas_set_range(&mas_writer, 0, ULONG_MAX); > + mas_store_gfp(&mas_writer, NULL, GFP_KERNEL); > + mas_set_range(&mas_writer, 0, 0); > + for (i = 0; i <= 5; i++) { > + mas_writer.index = i * 10; > + mas_writer.last = i * 10 + 5; > + mas_store_gfp(&mas_writer, xa_mk_value(i), GFP_KERNEL); > + } > + mas_unlock(&mas_writer); > + target = 10; > + mas_set_range(&mas_reader, target, target); > + rcu_read_lock(); > + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(target/10)); > + > + /* Overwrite the whole range */ > + mas_lock(&mas_writer); > + mas_set_range(&mas_writer, 0, ULONG_MAX); > + mas_store_gfp(&mas_writer, xa_mk_value(val), GFP_KERNEL); > + mas_unlock(&mas_writer); > + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(val)); > + rcu_read_unlock(); > + > rcu_unregister_thread(); > } > > -- > 2.34.1 >
On Mon, Feb 10, 2025 at 09:18:53AM -0500, Liam R. Howlett wrote: >* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: >> Original code may get a stall value when overwriting the whole range on a > ^^^^^- stale value. I thought you were saying > it was stalling which did not make sense. > I want to say we don't get the new value as we expect. How about: Original code may not get the new value after overwriting the whole range on a maple tree with only root node. >> maple tree with only root node. The reason is we didn't set the only >> root node dead during destroy. >> >> Add a test case to verify this is not recreated. >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> >> --- >> tools/testing/radix-tree/maple.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c >> index bc30050227fd..1e293e4d856d 100644 >> --- a/tools/testing/radix-tree/maple.c >> +++ b/tools/testing/radix-tree/maple.c >> @@ -35256,6 +35256,30 @@ static noinline void __init check_rcu_simulated(struct maple_tree *mt) >> MT_BUG_ON(mt, mas_prev(&mas_reader, 0) != xa_mk_value(val)); >> rcu_read_unlock(); >> >> + /* Clear out tree & create one with only root node */ >> + mas_lock(&mas_writer); >> + mas_set_range(&mas_writer, 0, ULONG_MAX); >> + mas_store_gfp(&mas_writer, NULL, GFP_KERNEL); >> + mas_set_range(&mas_writer, 0, 0); >> + for (i = 0; i <= 5; i++) { >> + mas_writer.index = i * 10; >> + mas_writer.last = i * 10 + 5; >> + mas_store_gfp(&mas_writer, xa_mk_value(i), GFP_KERNEL); >> + } >> + mas_unlock(&mas_writer); >> + target = 10; >> + mas_set_range(&mas_reader, target, target); >> + rcu_read_lock(); >> + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(target/10)); >> + >> + /* Overwrite the whole range */ >> + mas_lock(&mas_writer); >> + mas_set_range(&mas_writer, 0, ULONG_MAX); >> + mas_store_gfp(&mas_writer, xa_mk_value(val), GFP_KERNEL); >> + mas_unlock(&mas_writer); >> + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(val)); >> + rcu_read_unlock(); >> + >> rcu_unregister_thread(); >> } >> >> -- >> 2.34.1 >>
* Wei Yang <richard.weiyang@gmail.com> [250211 03:02]: > On Mon, Feb 10, 2025 at 09:18:53AM -0500, Liam R. Howlett wrote: > >* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: > >> Original code may get a stall value when overwriting the whole range on a > > ^^^^^- stale value. I thought you were saying > > it was stalling which did not make sense. > > > > I want to say we don't get the new value as we expect. > > How about: > > Original code may not get the new value after overwriting the whole range on a > maple tree with only root node. That's more clear than what you had before. Usually you try to concentrate on what you did, not what was there. Ensure the new value is returned when overwriting a tree containing just a leaf node. > > >> maple tree with only root node. The reason is we didn't set the only > >> root node dead during destroy. > >> > >> Add a test case to verify this is not recreated. > >> > >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > >> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> > >> --- > >> tools/testing/radix-tree/maple.c | 24 ++++++++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > >> > >> diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c > >> index bc30050227fd..1e293e4d856d 100644 > >> --- a/tools/testing/radix-tree/maple.c > >> +++ b/tools/testing/radix-tree/maple.c > >> @@ -35256,6 +35256,30 @@ static noinline void __init check_rcu_simulated(struct maple_tree *mt) > >> MT_BUG_ON(mt, mas_prev(&mas_reader, 0) != xa_mk_value(val)); > >> rcu_read_unlock(); > >> > >> + /* Clear out tree & create one with only root node */ > >> + mas_lock(&mas_writer); > >> + mas_set_range(&mas_writer, 0, ULONG_MAX); > >> + mas_store_gfp(&mas_writer, NULL, GFP_KERNEL); > >> + mas_set_range(&mas_writer, 0, 0); > >> + for (i = 0; i <= 5; i++) { > >> + mas_writer.index = i * 10; > >> + mas_writer.last = i * 10 + 5; > >> + mas_store_gfp(&mas_writer, xa_mk_value(i), GFP_KERNEL); > >> + } > >> + mas_unlock(&mas_writer); > >> + target = 10; > >> + mas_set_range(&mas_reader, target, target); > >> + rcu_read_lock(); > >> + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(target/10)); > >> + > >> + /* Overwrite the whole range */ > >> + mas_lock(&mas_writer); > >> + mas_set_range(&mas_writer, 0, ULONG_MAX); > >> + mas_store_gfp(&mas_writer, xa_mk_value(val), GFP_KERNEL); > >> + mas_unlock(&mas_writer); > >> + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(val)); > >> + rcu_read_unlock(); > >> + > >> rcu_unregister_thread(); > >> } > >> > >> -- > >> 2.34.1 > >> > > -- > Wei Yang > Help you, Help me
On Tue, Feb 11, 2025 at 10:25:27AM -0500, Liam R. Howlett wrote: >* Wei Yang <richard.weiyang@gmail.com> [250211 03:02]: >> On Mon, Feb 10, 2025 at 09:18:53AM -0500, Liam R. Howlett wrote: >> >* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: >> >> Original code may get a stall value when overwriting the whole range on a >> > ^^^^^- stale value. I thought you were saying >> > it was stalling which did not make sense. >> > >> >> I want to say we don't get the new value as we expect. >> >> How about: >> >> Original code may not get the new value after overwriting the whole range on a >> maple tree with only root node. > >That's more clear than what you had before. Usually you try to >concentrate on what you did, not what was there. Thanks, I didn't notice it. Will pay attention to it. > >Ensure the new value is returned when overwriting a tree containing just >a leaf node. > >> >> >> maple tree with only root node. The reason is we didn't set the only >> >> root node dead during destroy. >> >> >> >> Add a test case to verify this is not recreated. >> >> >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> >> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> >> >> --- >> >> tools/testing/radix-tree/maple.c | 24 ++++++++++++++++++++++++ >> >> 1 file changed, 24 insertions(+) >> >> >> >> diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c >> >> index bc30050227fd..1e293e4d856d 100644 >> >> --- a/tools/testing/radix-tree/maple.c >> >> +++ b/tools/testing/radix-tree/maple.c >> >> @@ -35256,6 +35256,30 @@ static noinline void __init check_rcu_simulated(struct maple_tree *mt) >> >> MT_BUG_ON(mt, mas_prev(&mas_reader, 0) != xa_mk_value(val)); >> >> rcu_read_unlock(); >> >> >> >> + /* Clear out tree & create one with only root node */ >> >> + mas_lock(&mas_writer); >> >> + mas_set_range(&mas_writer, 0, ULONG_MAX); >> >> + mas_store_gfp(&mas_writer, NULL, GFP_KERNEL); >> >> + mas_set_range(&mas_writer, 0, 0); >> >> + for (i = 0; i <= 5; i++) { >> >> + mas_writer.index = i * 10; >> >> + mas_writer.last = i * 10 + 5; >> >> + mas_store_gfp(&mas_writer, xa_mk_value(i), GFP_KERNEL); >> >> + } >> >> + mas_unlock(&mas_writer); >> >> + target = 10; >> >> + mas_set_range(&mas_reader, target, target); >> >> + rcu_read_lock(); >> >> + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(target/10)); >> >> + >> >> + /* Overwrite the whole range */ >> >> + mas_lock(&mas_writer); >> >> + mas_set_range(&mas_writer, 0, ULONG_MAX); >> >> + mas_store_gfp(&mas_writer, xa_mk_value(val), GFP_KERNEL); >> >> + mas_unlock(&mas_writer); >> >> + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(val)); >> >> + rcu_read_unlock(); >> >> + >> >> rcu_unregister_thread(); >> >> } >> >> >> >> -- >> >> 2.34.1 >> >> >> >> -- >> Wei Yang >> Help you, Help me
diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c index bc30050227fd..1e293e4d856d 100644 --- a/tools/testing/radix-tree/maple.c +++ b/tools/testing/radix-tree/maple.c @@ -35256,6 +35256,30 @@ static noinline void __init check_rcu_simulated(struct maple_tree *mt) MT_BUG_ON(mt, mas_prev(&mas_reader, 0) != xa_mk_value(val)); rcu_read_unlock(); + /* Clear out tree & create one with only root node */ + mas_lock(&mas_writer); + mas_set_range(&mas_writer, 0, ULONG_MAX); + mas_store_gfp(&mas_writer, NULL, GFP_KERNEL); + mas_set_range(&mas_writer, 0, 0); + for (i = 0; i <= 5; i++) { + mas_writer.index = i * 10; + mas_writer.last = i * 10 + 5; + mas_store_gfp(&mas_writer, xa_mk_value(i), GFP_KERNEL); + } + mas_unlock(&mas_writer); + target = 10; + mas_set_range(&mas_reader, target, target); + rcu_read_lock(); + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(target/10)); + + /* Overwrite the whole range */ + mas_lock(&mas_writer); + mas_set_range(&mas_writer, 0, ULONG_MAX); + mas_store_gfp(&mas_writer, xa_mk_value(val), GFP_KERNEL); + mas_unlock(&mas_writer); + MT_BUG_ON(mt, mas_walk(&mas_reader) != xa_mk_value(val)); + rcu_read_unlock(); + rcu_unregister_thread(); }
Original code may get a stall value when overwriting the whole range on a maple tree with only root node. The reason is we didn't set the only root node dead during destroy. Add a test case to verify this is not recreated. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> --- tools/testing/radix-tree/maple.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)