| Message ID | 20250208011852.31434-2-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]: > On destroy, we should set each node dead. But current code miss this > when the maple tree has only the root node. > > The reason is mt_destroy_walk() leverage mte_destroy_descend() to set > node dead, but this is skipped since the only root node is a leaf. > > This patch fixes this by setting the root dead before mt_destroy_walk(). > > Fixes: 54a611b60590 ("Maple Tree: add new data structure") > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > CC: Liam R. Howlett <Liam.Howlett@Oracle.com> > Cc: <stable@vger.kernel.org> > --- > lib/maple_tree.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 198c14dd3377..d31f0a2858f7 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -5347,6 +5347,8 @@ static inline void mte_destroy_walk(struct maple_enode *enode, > { > struct maple_node *node = mte_to_node(enode); > > + mte_set_node_dead(enode); > + This belongs in mt_destroy_walk(). > if (mt_in_rcu(mt)) { > mt_destroy_walk(enode, mt, false); > call_rcu(&node->rcu, mt_free_walk); > -- > 2.34.1 >
On Mon, Feb 10, 2025 at 09:19:46AM -0500, Liam R. Howlett wrote: >* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: >> On destroy, we should set each node dead. But current code miss this >> when the maple tree has only the root node. >> >> The reason is mt_destroy_walk() leverage mte_destroy_descend() to set >> node dead, but this is skipped since the only root node is a leaf. >> >> This patch fixes this by setting the root dead before mt_destroy_walk(). >> >> Fixes: 54a611b60590 ("Maple Tree: add new data structure") >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> >> Cc: <stable@vger.kernel.org> >> --- >> lib/maple_tree.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >> index 198c14dd3377..d31f0a2858f7 100644 >> --- a/lib/maple_tree.c >> +++ b/lib/maple_tree.c >> @@ -5347,6 +5347,8 @@ static inline void mte_destroy_walk(struct maple_enode *enode, >> { >> struct maple_node *node = mte_to_node(enode); >> >> + mte_set_node_dead(enode); >> + > >This belongs in mt_destroy_walk(). You prefer a change like this? diff --git a/lib/maple_tree.c b/lib/maple_tree.c index e64ffa5b9970..79f8632c61a3 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -5288,6 +5288,7 @@ static void mt_destroy_walk(struct maple_enode *enode, struct maple_tree *mt, struct maple_enode *start; if (mte_is_leaf(enode)) { + mte_set_node_dead(enode); node->type = mte_node_type(enode); goto free_leaf; } > >> if (mt_in_rcu(mt)) { >> mt_destroy_walk(enode, mt, false); >> call_rcu(&node->rcu, mt_free_walk); >> -- >> 2.34.1 >>
* Wei Yang <richard.weiyang@gmail.com> [250211 02:49]: > On Mon, Feb 10, 2025 at 09:19:46AM -0500, Liam R. Howlett wrote: > >* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: > >> On destroy, we should set each node dead. But current code miss this > >> when the maple tree has only the root node. > >> > >> The reason is mt_destroy_walk() leverage mte_destroy_descend() to set > >> node dead, but this is skipped since the only root node is a leaf. > >> > >> This patch fixes this by setting the root dead before mt_destroy_walk(). > >> > >> Fixes: 54a611b60590 ("Maple Tree: add new data structure") > >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > >> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> > >> Cc: <stable@vger.kernel.org> > >> --- > >> lib/maple_tree.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c > >> index 198c14dd3377..d31f0a2858f7 100644 > >> --- a/lib/maple_tree.c > >> +++ b/lib/maple_tree.c > >> @@ -5347,6 +5347,8 @@ static inline void mte_destroy_walk(struct maple_enode *enode, > >> { > >> struct maple_node *node = mte_to_node(enode); > >> > >> + mte_set_node_dead(enode); > >> + > > > >This belongs in mt_destroy_walk(). > > You prefer a change like this? Yes. > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index e64ffa5b9970..79f8632c61a3 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -5288,6 +5288,7 @@ static void mt_destroy_walk(struct maple_enode *enode, struct maple_tree *mt, > struct maple_enode *start; > > if (mte_is_leaf(enode)) { > + mte_set_node_dead(enode); > node->type = mte_node_type(enode); > goto free_leaf; > } > > > >> if (mt_in_rcu(mt)) { > >> mt_destroy_walk(enode, mt, false); > >> call_rcu(&node->rcu, mt_free_walk); > >> -- > >> 2.34.1 > >> > > -- > Wei Yang > Help you, Help me
On Tue, Feb 11, 2025 at 10:23:26AM -0500, Liam R. Howlett wrote: >* Wei Yang <richard.weiyang@gmail.com> [250211 02:49]: >> On Mon, Feb 10, 2025 at 09:19:46AM -0500, Liam R. Howlett wrote: >> >* Wei Yang <richard.weiyang@gmail.com> [250207 20:26]: >> >> On destroy, we should set each node dead. But current code miss this >> >> when the maple tree has only the root node. >> >> >> >> The reason is mt_destroy_walk() leverage mte_destroy_descend() to set >> >> node dead, but this is skipped since the only root node is a leaf. >> >> >> >> This patch fixes this by setting the root dead before mt_destroy_walk(). >> >> >> >> Fixes: 54a611b60590 ("Maple Tree: add new data structure") >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> >> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> >> >> Cc: <stable@vger.kernel.org> >> >> --- >> >> lib/maple_tree.c | 2 ++ >> >> 1 file changed, 2 insertions(+) >> >> >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >> >> index 198c14dd3377..d31f0a2858f7 100644 >> >> --- a/lib/maple_tree.c >> >> +++ b/lib/maple_tree.c >> >> @@ -5347,6 +5347,8 @@ static inline void mte_destroy_walk(struct maple_enode *enode, >> >> { >> >> struct maple_node *node = mte_to_node(enode); >> >> >> >> + mte_set_node_dead(enode); >> >> + >> > >> >This belongs in mt_destroy_walk(). >> >> You prefer a change like this? > >Yes. > Thanks, will adjust in v2. >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >> index e64ffa5b9970..79f8632c61a3 100644 >> --- a/lib/maple_tree.c >> +++ b/lib/maple_tree.c >> @@ -5288,6 +5288,7 @@ static void mt_destroy_walk(struct maple_enode *enode, struct maple_tree *mt, >> struct maple_enode *start; >> >> if (mte_is_leaf(enode)) { >> + mte_set_node_dead(enode); >> node->type = mte_node_type(enode); >> goto free_leaf; >> } >> > >> >> if (mt_in_rcu(mt)) { >> >> mt_destroy_walk(enode, mt, false); >> >> call_rcu(&node->rcu, mt_free_walk); >> >> -- >> >> 2.34.1 >> >> >> >> -- >> Wei Yang >> Help you, Help me
diff --git a/lib/maple_tree.c b/lib/maple_tree.c index 198c14dd3377..d31f0a2858f7 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -5347,6 +5347,8 @@ static inline void mte_destroy_walk(struct maple_enode *enode, { struct maple_node *node = mte_to_node(enode); + mte_set_node_dead(enode); + if (mt_in_rcu(mt)) { mt_destroy_walk(enode, mt, false); call_rcu(&node->rcu, mt_free_walk);
On destroy, we should set each node dead. But current code miss this when the maple tree has only the root node. The reason is mt_destroy_walk() leverage mte_destroy_descend() to set node dead, but this is skipped since the only root node is a leaf. This patch fixes this by setting the root dead before mt_destroy_walk(). Fixes: 54a611b60590 ("Maple Tree: add new data structure") Signed-off-by: Wei Yang <richard.weiyang@gmail.com> CC: Liam R. Howlett <Liam.Howlett@Oracle.com> Cc: <stable@vger.kernel.org> --- lib/maple_tree.c | 2 ++ 1 file changed, 2 insertions(+)