| Message ID | 20230425140955.3834476-26-Liam.Howlett@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Maple tree mas_{next,prev}_range() and cleanup | expand |
在 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;
* 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 --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;
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(-)