| Message ID | 20230517085809.86522-8-zhangpeng.00@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Clean ups for maple tree | expand |
* Peng Zhang <zhangpeng.00@bytedance.com> [230517 04:59]: > Add comment for mas_wr_append(), move mas_update_gap() into > mas_wr_append(), and other cleanups to make mas_wr_modify() cleaner. > > Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> > --- > lib/maple_tree.c | 33 ++++++++++++++++++++------------- > 1 file changed, 20 insertions(+), 13 deletions(-) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 3b9d227f3d7d..bbe4c6f2858c 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -4308,6 +4308,12 @@ static inline unsigned char mas_wr_new_end(struct ma_wr_state *wr_mas) > return new_end; > } > > +/* > + * mas_wr_append: Attempt to append > + * @wr_mas: the maple write state > + * > + * Return: True if appended, false otherwise > + */ > static inline bool mas_wr_append(struct ma_wr_state *wr_mas) > { > unsigned char end = wr_mas->node_end; > @@ -4315,7 +4321,11 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) > struct ma_state *mas = wr_mas->mas; > unsigned char node_pivots = mt_pivots[wr_mas->type]; > > + if (!(mas->offset == wr_mas->node_end)) > + return false; > + > if ((mas->index != wr_mas->r_min) && (mas->last == wr_mas->r_max)) { > + /* Append to end of range */ > if (new_end < node_pivots) > wr_mas->pivots[new_end] = wr_mas->pivots[end]; > > @@ -4323,13 +4333,10 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) > ma_set_meta(wr_mas->node, maple_leaf_64, 0, new_end); > > rcu_assign_pointer(wr_mas->slots[new_end], wr_mas->entry); > - mas->offset = new_end; > wr_mas->pivots[end] = mas->index - 1; > - > - return true; > - } > - > - if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) { > + mas->offset = new_end; > + } else if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) { > + /* Append to start of range */ > if (new_end < node_pivots) > wr_mas->pivots[new_end] = wr_mas->pivots[end]; > > @@ -4339,10 +4346,13 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) > > wr_mas->pivots[end] = mas->last; > rcu_assign_pointer(wr_mas->slots[end], wr_mas->entry); > - return true; > + } else { > + return false; I don't think we can get here with your changes, what do you think? If so, we can move the metadata setting to the outside of the if/else. I checked by adding a BUG_ON() and the test code never gets here. My thinking is, we know offset == node_end and new_end == node_end + 1.. so we must be inserting into the last slot and only adding one entry. > } > > - return false; > + if (!wr_mas->content || !wr_mas->entry) > + mas_update_gap(mas); > + return true; > } > > /* > @@ -4382,12 +4392,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) > if (new_end >= mt_slots[wr_mas->type]) > goto slow_path; > > - if (wr_mas->entry && (wr_mas->node_end < mt_slots[wr_mas->type] - 1) && > - (mas->offset == wr_mas->node_end) && mas_wr_append(wr_mas)) { > - if (!wr_mas->content || !wr_mas->entry) > - mas_update_gap(mas); > + /* Attempt to append */ > + if (new_end == wr_mas->node_end + 1 && mas_wr_append(wr_mas)) > return; > - } > > if ((wr_mas->offset_end - mas->offset <= 1) && mas_wr_slot_store(wr_mas)) > return; > -- > 2.20.1 >
在 2023/5/18 03:33, Liam R. Howlett 写道: > * Peng Zhang <zhangpeng.00@bytedance.com> [230517 04:59]: >> Add comment for mas_wr_append(), move mas_update_gap() into >> mas_wr_append(), and other cleanups to make mas_wr_modify() cleaner. >> >> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> >> --- >> lib/maple_tree.c | 33 ++++++++++++++++++++------------- >> 1 file changed, 20 insertions(+), 13 deletions(-) >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >> index 3b9d227f3d7d..bbe4c6f2858c 100644 >> --- a/lib/maple_tree.c >> +++ b/lib/maple_tree.c >> @@ -4308,6 +4308,12 @@ static inline unsigned char mas_wr_new_end(struct ma_wr_state *wr_mas) >> return new_end; >> } >> >> +/* >> + * mas_wr_append: Attempt to append >> + * @wr_mas: the maple write state >> + * >> + * Return: True if appended, false otherwise >> + */ >> static inline bool mas_wr_append(struct ma_wr_state *wr_mas) >> { >> unsigned char end = wr_mas->node_end; >> @@ -4315,7 +4321,11 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) >> struct ma_state *mas = wr_mas->mas; >> unsigned char node_pivots = mt_pivots[wr_mas->type]; >> >> + if (!(mas->offset == wr_mas->node_end)) >> + return false; >> + >> if ((mas->index != wr_mas->r_min) && (mas->last == wr_mas->r_max)) { >> + /* Append to end of range */ >> if (new_end < node_pivots) >> wr_mas->pivots[new_end] = wr_mas->pivots[end]; >> >> @@ -4323,13 +4333,10 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) >> ma_set_meta(wr_mas->node, maple_leaf_64, 0, new_end); >> >> rcu_assign_pointer(wr_mas->slots[new_end], wr_mas->entry); >> - mas->offset = new_end; >> wr_mas->pivots[end] = mas->index - 1; >> - >> - return true; >> - } >> - >> - if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) { >> + mas->offset = new_end; >> + } else if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) { >> + /* Append to start of range */ >> if (new_end < node_pivots) >> wr_mas->pivots[new_end] = wr_mas->pivots[end]; >> >> @@ -4339,10 +4346,13 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) >> >> wr_mas->pivots[end] = mas->last; >> rcu_assign_pointer(wr_mas->slots[end], wr_mas->entry); >> - return true; >> + } else { >> + return false; > > I don't think we can get here with your changes, what do you think? If > so, we can move the metadata setting to the outside of the if/else. I > checked by adding a BUG_ON() and the test code never gets here. > > My thinking is, we know offset == node_end and new_end == node_end + 1.. > so we must be inserting into the last slot and only adding one entry. Yes, I'll address this in v3. Thanks. > >> } >> >> - return false; >> + if (!wr_mas->content || !wr_mas->entry) >> + mas_update_gap(mas); >> + return true; >> } >> >> /* >> @@ -4382,12 +4392,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) >> if (new_end >= mt_slots[wr_mas->type]) >> goto slow_path; >> >> - if (wr_mas->entry && (wr_mas->node_end < mt_slots[wr_mas->type] - 1) && >> - (mas->offset == wr_mas->node_end) && mas_wr_append(wr_mas)) { >> - if (!wr_mas->content || !wr_mas->entry) >> - mas_update_gap(mas); >> + /* Attempt to append */ >> + if (new_end == wr_mas->node_end + 1 && mas_wr_append(wr_mas)) >> return; >> - } >> >> if ((wr_mas->offset_end - mas->offset <= 1) && mas_wr_slot_store(wr_mas)) >> return; >> -- >> 2.20.1 >>
diff --git a/lib/maple_tree.c b/lib/maple_tree.c index 3b9d227f3d7d..bbe4c6f2858c 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -4308,6 +4308,12 @@ static inline unsigned char mas_wr_new_end(struct ma_wr_state *wr_mas) return new_end; } +/* + * mas_wr_append: Attempt to append + * @wr_mas: the maple write state + * + * Return: True if appended, false otherwise + */ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) { unsigned char end = wr_mas->node_end; @@ -4315,7 +4321,11 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) struct ma_state *mas = wr_mas->mas; unsigned char node_pivots = mt_pivots[wr_mas->type]; + if (!(mas->offset == wr_mas->node_end)) + return false; + if ((mas->index != wr_mas->r_min) && (mas->last == wr_mas->r_max)) { + /* Append to end of range */ if (new_end < node_pivots) wr_mas->pivots[new_end] = wr_mas->pivots[end]; @@ -4323,13 +4333,10 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) ma_set_meta(wr_mas->node, maple_leaf_64, 0, new_end); rcu_assign_pointer(wr_mas->slots[new_end], wr_mas->entry); - mas->offset = new_end; wr_mas->pivots[end] = mas->index - 1; - - return true; - } - - if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) { + mas->offset = new_end; + } else if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) { + /* Append to start of range */ if (new_end < node_pivots) wr_mas->pivots[new_end] = wr_mas->pivots[end]; @@ -4339,10 +4346,13 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas) wr_mas->pivots[end] = mas->last; rcu_assign_pointer(wr_mas->slots[end], wr_mas->entry); - return true; + } else { + return false; } - return false; + if (!wr_mas->content || !wr_mas->entry) + mas_update_gap(mas); + return true; } /* @@ -4382,12 +4392,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) if (new_end >= mt_slots[wr_mas->type]) goto slow_path; - if (wr_mas->entry && (wr_mas->node_end < mt_slots[wr_mas->type] - 1) && - (mas->offset == wr_mas->node_end) && mas_wr_append(wr_mas)) { - if (!wr_mas->content || !wr_mas->entry) - mas_update_gap(mas); + /* Attempt to append */ + if (new_end == wr_mas->node_end + 1 && mas_wr_append(wr_mas)) return; - } if ((wr_mas->offset_end - mas->offset <= 1) && mas_wr_slot_store(wr_mas)) return;
Add comment for mas_wr_append(), move mas_update_gap() into mas_wr_append(), and other cleanups to make mas_wr_modify() cleaner. Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> --- lib/maple_tree.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)