| Message ID | 20241011214451.7286-1-sidhartha.kumar@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] maple_tree: check for MA_STATE_BULK on setting wr_rebalance | expand |
On Fri, Oct 11, 2024 at 05:44:50PM -0400, Sidhartha Kumar wrote: >It is possible for a bulk operation (MA_STATE_BULK is set) to enter the >new_end < mt_min_slots[type] case and set wr_rebalance as a store type. >This is incorrect as bulk stores do not rebalance per write, but rather >after the all of the writes are done through the mas_bulk_rebalance() >path. Therefore, add a check to make sure MA_STATE_BULK is not set >before we return wr_rebalance as the store type. > >Also add a test to make sure wr_rebalance is never the store type when >doing bulk operations via mas_expected_entries() > >Fixes: 5d659bbb52a2 ("maple_tree: introduce mas_wr_store_type()") >Suggested-by: Liam Howlett <liam.howlett@oracle.com> >Signed-off-by: Sidhartha <sidhartha.kumar@oracle.com> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
* Sidhartha Kumar <sidhartha.kumar@oracle.com> [241011 17:44]: > It is possible for a bulk operation (MA_STATE_BULK is set) to enter the > new_end < mt_min_slots[type] case and set wr_rebalance as a store type. > This is incorrect as bulk stores do not rebalance per write, but rather > after the all of the writes are done through the mas_bulk_rebalance() > path. Therefore, add a check to make sure MA_STATE_BULK is not set > before we return wr_rebalance as the store type. > > Also add a test to make sure wr_rebalance is never the store type when > doing bulk operations via mas_expected_entries() > > Fixes: 5d659bbb52a2 ("maple_tree: introduce mas_wr_store_type()") > Suggested-by: Liam Howlett <liam.howlett@oracle.com> > Signed-off-by: Sidhartha <sidhartha.kumar@oracle.com> Reviewed-by: Liam Howlett <liam.howlett@oracle.com> > --- > > Hi Andrew, this commit is a hotfix for this rc however it has no userspace > effects as there are no users of the bulk instertion mode. > > lib/maple_tree.c | 2 +- > tools/testing/radix-tree/maple.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 4b423330d83c..b3b1d4b8126b 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -4228,7 +4228,7 @@ static inline void mas_wr_store_type(struct ma_wr_state *wr_mas) > > /* Potential spanning rebalance collapsing a node */ > if (new_end < mt_min_slots[wr_mas->type]) { > - if (!mte_is_root(mas->node)) { > + if (!mte_is_root(mas->node) && !(mas->mas_flags & MA_STATE_BULK)) { > mas->store_type = wr_rebalance; > return; > } > diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c > index 5fde09999be4..551ae6898c1d 100644 > --- a/tools/testing/radix-tree/maple.c > +++ b/tools/testing/radix-tree/maple.c > @@ -36317,6 +36317,28 @@ static inline int check_vma_modification(struct maple_tree *mt) > return 0; > } > > +/* > + * test to check that bulk stores do not use wr_rebalance as the store > + * type. > + */ > +static inline void check_bulk_rebalance(struct maple_tree *mt) > +{ > + MA_STATE(mas, mt, ULONG_MAX, ULONG_MAX); > + int max = 10; > + > + build_full_tree(mt, 0, 2); > + > + /* erase every entry in the tree */ > + do { > + /* set up bulk store mode */ > + mas_expected_entries(&mas, max); > + mas_erase(&mas); > + MT_BUG_ON(mt, mas.store_type == wr_rebalance); > + } while (mas_prev(&mas, 0) != NULL); > + > + mas_destroy(&mas); > +} > + > void farmer_tests(void) > { > struct maple_node *node; > @@ -36328,6 +36350,10 @@ void farmer_tests(void) > check_vma_modification(&tree); > mtree_destroy(&tree); > > + mt_init(&tree); > + check_bulk_rebalance(&tree); > + mtree_destroy(&tree); > + > tree.ma_root = xa_mk_value(0); > mt_dump(&tree, mt_dump_dec); > > -- > 2.43.0 >
diff --git a/lib/maple_tree.c b/lib/maple_tree.c index 4b423330d83c..b3b1d4b8126b 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -4228,7 +4228,7 @@ static inline void mas_wr_store_type(struct ma_wr_state *wr_mas) /* Potential spanning rebalance collapsing a node */ if (new_end < mt_min_slots[wr_mas->type]) { - if (!mte_is_root(mas->node)) { + if (!mte_is_root(mas->node) && !(mas->mas_flags & MA_STATE_BULK)) { mas->store_type = wr_rebalance; return; } diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c index 5fde09999be4..551ae6898c1d 100644 --- a/tools/testing/radix-tree/maple.c +++ b/tools/testing/radix-tree/maple.c @@ -36317,6 +36317,28 @@ static inline int check_vma_modification(struct maple_tree *mt) return 0; } +/* + * test to check that bulk stores do not use wr_rebalance as the store + * type. + */ +static inline void check_bulk_rebalance(struct maple_tree *mt) +{ + MA_STATE(mas, mt, ULONG_MAX, ULONG_MAX); + int max = 10; + + build_full_tree(mt, 0, 2); + + /* erase every entry in the tree */ + do { + /* set up bulk store mode */ + mas_expected_entries(&mas, max); + mas_erase(&mas); + MT_BUG_ON(mt, mas.store_type == wr_rebalance); + } while (mas_prev(&mas, 0) != NULL); + + mas_destroy(&mas); +} + void farmer_tests(void) { struct maple_node *node; @@ -36328,6 +36350,10 @@ void farmer_tests(void) check_vma_modification(&tree); mtree_destroy(&tree); + mt_init(&tree); + check_bulk_rebalance(&tree); + mtree_destroy(&tree); + tree.ma_root = xa_mk_value(0); mt_dump(&tree, mt_dump_dec);
It is possible for a bulk operation (MA_STATE_BULK is set) to enter the new_end < mt_min_slots[type] case and set wr_rebalance as a store type. This is incorrect as bulk stores do not rebalance per write, but rather after the all of the writes are done through the mas_bulk_rebalance() path. Therefore, add a check to make sure MA_STATE_BULK is not set before we return wr_rebalance as the store type. Also add a test to make sure wr_rebalance is never the store type when doing bulk operations via mas_expected_entries() Fixes: 5d659bbb52a2 ("maple_tree: introduce mas_wr_store_type()") Suggested-by: Liam Howlett <liam.howlett@oracle.com> Signed-off-by: Sidhartha <sidhartha.kumar@oracle.com> --- Hi Andrew, this commit is a hotfix for this rc however it has no userspace effects as there are no users of the bulk instertion mode. lib/maple_tree.c | 2 +- tools/testing/radix-tree/maple.c | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-)