diff mbox series

[25/34] maple_tree: Clear up index and last setting in single entry tree

Message ID 20230425140955.3834476-26-Liam.Howlett@oracle.com
State New
Headers
Series Maple tree mas_{next,prev}_range() and cleanup |

Commit Message

Liam R. Howlett April 25, 2023, 2:09 p.m. UTC 
  When there is a single entry tree (range of 0-0 pointing to an entry),
then ensure the limit is either 0-0 or 1-oo, depending on where the user
walks.  Ensure the correct node setting as well; either MAS_ROOT or
MAS_NONE.

Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 lib/maple_tree.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Peng Zhang April 27, 2023, 11:19 a.m. UTC | #1 
在 2023/4/25 22:09, Liam R. Howlett 写道:
> When there is a single entry tree (range of 0-0 pointing to an entry),
> then ensure the limit is either 0-0 or 1-oo, depending on where the user
> walks.  Ensure the correct node setting as well; either MAS_ROOT or
> MAS_NONE.
> 
> Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> ---
>   lib/maple_tree.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 20f0a10dc5608..31cbfd7b44728 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -5099,24 +5099,25 @@ void *mas_walk(struct ma_state *mas)
>   {
>   	void *entry;
>   
> +	if (mas_is_none(mas) || mas_is_paused(mas))
> +		mas->node = MAS_START;
>   retry:
>   	entry = mas_state_walk(mas);
> -	if (mas_is_start(mas))
> +	if (mas_is_start(mas)) {
>   		goto retry;
> -
> -	if (mas_is_ptr(mas)) {
> +	} else if (mas_is_none(mas)) {
> +		mas->index = 0;
> +		mas->last = ULONG_MAX;
> +	} else if (mas_is_ptr(mas)) {
>   		if (!mas->index) {
>   			mas->last = 0;
> -		} else {
> -			mas->index = 1;
> -			mas->last = ULONG_MAX;
> +			return mas_root(mas);
Why we call mas_root() to get the single entry stored in root again?
I think it's not safe. In RCU mode, if someone modify the tree to a 
normal tree(not a single entry tree), mas_root() will return a address.
So, this may cause a race bug. We can return entry directly.
>   		}
> -		return entry;
> -	}
>   
> -	if (mas_is_none(mas)) {
> -		mas->index = 0;
> +		mas->index = 1;
>   		mas->last = ULONG_MAX;
> +		mas->node = MAS_NONE;
> +		return NULL;
>   	}
>   
>   	return entry;
Liam R. Howlett April 27, 2023, 5:25 p.m. UTC | #2 
* Peng Zhang <perlyzhang@gmail.com> [230427 07:20]:
> 
> 
> 在 2023/4/25 22:09, Liam R. Howlett 写道:
> > When there is a single entry tree (range of 0-0 pointing to an entry),
> > then ensure the limit is either 0-0 or 1-oo, depending on where the user
> > walks.  Ensure the correct node setting as well; either MAS_ROOT or
> > MAS_NONE.
> > 
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> > ---
> >   lib/maple_tree.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 20f0a10dc5608..31cbfd7b44728 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -5099,24 +5099,25 @@ void *mas_walk(struct ma_state *mas)
> >   {
> >   	void *entry;
> > +	if (mas_is_none(mas) || mas_is_paused(mas))
> > +		mas->node = MAS_START;
> >   retry:
> >   	entry = mas_state_walk(mas);
> > -	if (mas_is_start(mas))
> > +	if (mas_is_start(mas)) {
> >   		goto retry;
> > -
> > -	if (mas_is_ptr(mas)) {
> > +	} else if (mas_is_none(mas)) {
> > +		mas->index = 0;
> > +		mas->last = ULONG_MAX;
> > +	} else if (mas_is_ptr(mas)) {
> >   		if (!mas->index) {
> >   			mas->last = 0;
> > -		} else {
> > -			mas->index = 1;
> > -			mas->last = ULONG_MAX;
> > +			return mas_root(mas);
> Why we call mas_root() to get the single entry stored in root again?
> I think it's not safe. In RCU mode, if someone modify the tree to a normal
> tree(not a single entry tree), mas_root() will return a address.
> So, this may cause a race bug. We can return entry directly.

Good catch.  I will address this in v2.

> >   		}
> > -		return entry;
> > -	}
> > -	if (mas_is_none(mas)) {
> > -		mas->index = 0;
> > +		mas->index = 1;
> >   		mas->last = ULONG_MAX;
> > +		mas->node = MAS_NONE;
> > +		return NULL;
> >   	}
> >   	return entry;
diff mbox series

Patch

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 20f0a10dc5608..31cbfd7b44728 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -5099,24 +5099,25 @@  void *mas_walk(struct ma_state *mas)
 {
 	void *entry;
 
+	if (mas_is_none(mas) || mas_is_paused(mas))
+		mas->node = MAS_START;
 retry:
 	entry = mas_state_walk(mas);
-	if (mas_is_start(mas))
+	if (mas_is_start(mas)) {
 		goto retry;
-
-	if (mas_is_ptr(mas)) {
+	} else if (mas_is_none(mas)) {
+		mas->index = 0;
+		mas->last = ULONG_MAX;
+	} else if (mas_is_ptr(mas)) {
 		if (!mas->index) {
 			mas->last = 0;
-		} else {
-			mas->index = 1;
-			mas->last = ULONG_MAX;
+			return mas_root(mas);
 		}
-		return entry;
-	}
 
-	if (mas_is_none(mas)) {
-		mas->index = 0;
+		mas->index = 1;
 		mas->last = ULONG_MAX;
+		mas->node = MAS_NONE;
+		return NULL;
 	}
 
 	return entry;