| Message ID | 20230515131757.60035-7-zhangpeng.00@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Clean ups for maple tree | expand |
* Peng Zhang <zhangpeng.00@bytedance.com> [230515 09:18]: > To make mas_wr_modify() cleaner, wrap the replace operation with an > inline function. mas_wr_modify() is already pretty small. Is there any reason you want this in its own function besides it looking cleaner? > > Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> > --- > lib/maple_tree.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 4c649d75a4923..ce695adc670ec 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -4288,6 +4288,19 @@ static inline void mas_wr_extend_null(struct ma_wr_state *wr_mas) > } > } > > +static inline bool mas_wr_replace(struct ma_wr_state *wr_mas) > +{ > + struct ma_state *mas = wr_mas->mas; > + > + if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { > + rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); > + if (!!wr_mas->entry ^ !!wr_mas->content) > + mas_update_gap(mas); > + return true; > + } > + return false; > +} > + > static inline bool mas_wr_append(struct ma_wr_state *wr_mas) > { > unsigned char end = wr_mas->node_end; > @@ -4347,13 +4360,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) > unsigned char node_size; > struct ma_state *mas = wr_mas->mas; > > - /* Direct replacement */ > - if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { > - rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); > - if (!!wr_mas->entry ^ !!wr_mas->content) > - mas_update_gap(mas); > + /* Attempt to direct replace */ > + if (mas_wr_replace(wr_mas)) > return; > - } > > /* Attempt to append */ > node_slots = mt_slots[wr_mas->type]; > -- > 2.20.1 >
在 2023/5/16 01:07, Liam R. Howlett 写道: > * Peng Zhang <zhangpeng.00@bytedance.com> [230515 09:18]: >> To make mas_wr_modify() cleaner, wrap the replace operation with an >> inline function. > > mas_wr_modify() is already pretty small. Is there any reason you want > this in its own function besides it looking cleaner? I just want to make the four fast paths in mas_wr_modify() look uniform without any functional effect. > >> >> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> >> --- >> lib/maple_tree.c | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >> index 4c649d75a4923..ce695adc670ec 100644 >> --- a/lib/maple_tree.c >> +++ b/lib/maple_tree.c >> @@ -4288,6 +4288,19 @@ static inline void mas_wr_extend_null(struct ma_wr_state *wr_mas) >> } >> } >> >> +static inline bool mas_wr_replace(struct ma_wr_state *wr_mas) >> +{ >> + struct ma_state *mas = wr_mas->mas; >> + >> + if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { >> + rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); >> + if (!!wr_mas->entry ^ !!wr_mas->content) >> + mas_update_gap(mas); >> + return true; >> + } >> + return false; >> +} >> + >> static inline bool mas_wr_append(struct ma_wr_state *wr_mas) >> { >> unsigned char end = wr_mas->node_end; >> @@ -4347,13 +4360,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) >> unsigned char node_size; >> struct ma_state *mas = wr_mas->mas; >> >> - /* Direct replacement */ >> - if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { >> - rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); >> - if (!!wr_mas->entry ^ !!wr_mas->content) >> - mas_update_gap(mas); >> + /* Attempt to direct replace */ >> + if (mas_wr_replace(wr_mas)) >> return; >> - } >> >> /* Attempt to append */ >> node_slots = mt_slots[wr_mas->type]; >> -- >> 2.20.1 >>
* Peng Zhang <zhangpeng.00@bytedance.com> [230515 20:46]: > > > 在 2023/5/16 01:07, Liam R. Howlett 写道: > > * Peng Zhang <zhangpeng.00@bytedance.com> [230515 09:18]: > > > To make mas_wr_modify() cleaner, wrap the replace operation with an > > > inline function. > > > > mas_wr_modify() is already pretty small. Is there any reason you want > > this in its own function besides it looking cleaner? > I just want to make the four fast paths in mas_wr_modify() > look uniform without any functional effect. I'd like to keep it the way it is. I think the comment stating what is going on is clear enough and mas_wr_modify() isn't too big. > > > > > > > > Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> > > > --- > > > lib/maple_tree.c | 21 +++++++++++++++------ > > > 1 file changed, 15 insertions(+), 6 deletions(-) > > > > > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > > > index 4c649d75a4923..ce695adc670ec 100644 > > > --- a/lib/maple_tree.c > > > +++ b/lib/maple_tree.c > > > @@ -4288,6 +4288,19 @@ static inline void mas_wr_extend_null(struct ma_wr_state *wr_mas) > > > } > > > } > > > +static inline bool mas_wr_replace(struct ma_wr_state *wr_mas) > > > +{ > > > + struct ma_state *mas = wr_mas->mas; > > > + > > > + if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { > > > + rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); > > > + if (!!wr_mas->entry ^ !!wr_mas->content) > > > + mas_update_gap(mas); > > > + return true; > > > + } > > > + return false; > > > +} > > > + > > > static inline bool mas_wr_append(struct ma_wr_state *wr_mas) > > > { > > > unsigned char end = wr_mas->node_end; > > > @@ -4347,13 +4360,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) > > > unsigned char node_size; > > > struct ma_state *mas = wr_mas->mas; > > > - /* Direct replacement */ > > > - if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { > > > - rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); > > > - if (!!wr_mas->entry ^ !!wr_mas->content) > > > - mas_update_gap(mas); > > > + /* Attempt to direct replace */ > > > + if (mas_wr_replace(wr_mas)) > > > return; > > > - } > > > /* Attempt to append */ > > > node_slots = mt_slots[wr_mas->type]; > > > -- > > > 2.20.1 > > >
在 2023/5/16 22:16, Liam R. Howlett 写道: > * Peng Zhang <zhangpeng.00@bytedance.com> [230515 20:46]: >> >> >> 在 2023/5/16 01:07, Liam R. Howlett 写道: >>> * Peng Zhang <zhangpeng.00@bytedance.com> [230515 09:18]: >>>> To make mas_wr_modify() cleaner, wrap the replace operation with an >>>> inline function. >>> >>> mas_wr_modify() is already pretty small. Is there any reason you want >>> this in its own function besides it looking cleaner? >> I just want to make the four fast paths in mas_wr_modify() >> look uniform without any functional effect. > > I'd like to keep it the way it is. I think the comment stating what is > going on is clear enough and mas_wr_modify() isn't too big. Ok, I'll drop this patch in v3. > >>> >>>> >>>> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> >>>> --- >>>> lib/maple_tree.c | 21 +++++++++++++++------ >>>> 1 file changed, 15 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c >>>> index 4c649d75a4923..ce695adc670ec 100644 >>>> --- a/lib/maple_tree.c >>>> +++ b/lib/maple_tree.c >>>> @@ -4288,6 +4288,19 @@ static inline void mas_wr_extend_null(struct ma_wr_state *wr_mas) >>>> } >>>> } >>>> +static inline bool mas_wr_replace(struct ma_wr_state *wr_mas) >>>> +{ >>>> + struct ma_state *mas = wr_mas->mas; >>>> + >>>> + if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { >>>> + rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); >>>> + if (!!wr_mas->entry ^ !!wr_mas->content) >>>> + mas_update_gap(mas); >>>> + return true; >>>> + } >>>> + return false; >>>> +} >>>> + >>>> static inline bool mas_wr_append(struct ma_wr_state *wr_mas) >>>> { >>>> unsigned char end = wr_mas->node_end; >>>> @@ -4347,13 +4360,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) >>>> unsigned char node_size; >>>> struct ma_state *mas = wr_mas->mas; >>>> - /* Direct replacement */ >>>> - if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { >>>> - rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); >>>> - if (!!wr_mas->entry ^ !!wr_mas->content) >>>> - mas_update_gap(mas); >>>> + /* Attempt to direct replace */ >>>> + if (mas_wr_replace(wr_mas)) >>>> return; >>>> - } >>>> /* Attempt to append */ >>>> node_slots = mt_slots[wr_mas->type]; >>>> -- >>>> 2.20.1 >>>>
diff --git a/lib/maple_tree.c b/lib/maple_tree.c index 4c649d75a4923..ce695adc670ec 100644 --- a/lib/maple_tree.c +++ b/lib/maple_tree.c @@ -4288,6 +4288,19 @@ static inline void mas_wr_extend_null(struct ma_wr_state *wr_mas) } } +static inline bool mas_wr_replace(struct ma_wr_state *wr_mas) +{ + struct ma_state *mas = wr_mas->mas; + + if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { + rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); + if (!!wr_mas->entry ^ !!wr_mas->content) + mas_update_gap(mas); + return true; + } + return false; +} + static inline bool mas_wr_append(struct ma_wr_state *wr_mas) { unsigned char end = wr_mas->node_end; @@ -4347,13 +4360,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas) unsigned char node_size; struct ma_state *mas = wr_mas->mas; - /* Direct replacement */ - if (wr_mas->r_min == mas->index && wr_mas->r_max == mas->last) { - rcu_assign_pointer(wr_mas->slots[mas->offset], wr_mas->entry); - if (!!wr_mas->entry ^ !!wr_mas->content) - mas_update_gap(mas); + /* Attempt to direct replace */ + if (mas_wr_replace(wr_mas)) return; - } /* Attempt to append */ node_slots = mt_slots[wr_mas->type];
To make mas_wr_modify() cleaner, wrap the replace operation with an inline function. Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com> --- lib/maple_tree.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)