* [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally
@ 2019-10-28 2:14 Wei Yang
2019-10-28 2:14 ` [Patch v2 2/2] lib/rbtree: get successor's color directly Wei Yang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Wei Yang @ 2019-10-28 2:14 UTC (permalink / raw)
To: akpm, peterz, walken, richardw.yang, dbueso, tglx; +Cc: linux-kernel
Both in Case 2 and 3, we exchange n and s. This mean no matter whether
child2 is NULL or not, successor's parent should be assigned to node's.
This patch takes this step out to make it explicit and reduce the
ambiguity.
Besides, this step reduces some symbol size like rb_erase().
KERN_CONFIG upstream patched
OPT_FOR_PERF 877 870
OPT_FOR_SIZE 635 621
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
include/linux/rbtree_augmented.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/rbtree_augmented.h b/include/linux/rbtree_augmented.h
index fdd421b8d9ae..99c42e1a74b8 100644
--- a/include/linux/rbtree_augmented.h
+++ b/include/linux/rbtree_augmented.h
@@ -283,14 +283,13 @@ __rb_erase_augmented(struct rb_node *node, struct rb_root *root,
__rb_change_child(node, successor, tmp, root);
if (child2) {
- successor->__rb_parent_color = pc;
rb_set_parent_color(child2, parent, RB_BLACK);
rebalance = NULL;
} else {
unsigned long pc2 = successor->__rb_parent_color;
- successor->__rb_parent_color = pc;
rebalance = __rb_is_black(pc2) ? parent : NULL;
}
+ successor->__rb_parent_color = pc;
tmp = successor;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Patch v2 2/2] lib/rbtree: get successor's color directly
2019-10-28 2:14 [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Wei Yang
@ 2019-10-28 2:14 ` Wei Yang
2019-10-28 9:21 ` Peter Zijlstra
2019-10-28 4:29 ` [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Michel Lespinasse
2019-10-28 9:20 ` Peter Zijlstra
2 siblings, 1 reply; 6+ messages in thread
From: Wei Yang @ 2019-10-28 2:14 UTC (permalink / raw)
To: akpm, peterz, walken, richardw.yang, dbueso, tglx; +Cc: linux-kernel
After move parent assignment out, we can check the color directly.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
include/linux/rbtree_augmented.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/rbtree_augmented.h b/include/linux/rbtree_augmented.h
index 99c42e1a74b8..724b0d036b57 100644
--- a/include/linux/rbtree_augmented.h
+++ b/include/linux/rbtree_augmented.h
@@ -286,8 +286,7 @@ __rb_erase_augmented(struct rb_node *node, struct rb_root *root,
rb_set_parent_color(child2, parent, RB_BLACK);
rebalance = NULL;
} else {
- unsigned long pc2 = successor->__rb_parent_color;
- rebalance = __rb_is_black(pc2) ? parent : NULL;
+ rebalance = rb_is_black(successor) ? parent : NULL;
}
successor->__rb_parent_color = pc;
tmp = successor;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally
2019-10-28 2:14 [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Wei Yang
2019-10-28 2:14 ` [Patch v2 2/2] lib/rbtree: get successor's color directly Wei Yang
@ 2019-10-28 4:29 ` Michel Lespinasse
2019-10-28 6:23 ` Wei Yang
2019-10-28 9:20 ` Peter Zijlstra
2 siblings, 1 reply; 6+ messages in thread
From: Michel Lespinasse @ 2019-10-28 4:29 UTC (permalink / raw)
To: Wei Yang
Cc: Andrew Morton, Peter Zijlstra, Davidlohr Bueso, Thomas Gleixner,
LKML
Code looks fine, for both commits in this series. Please make sure to
double check that lib/rbtree_test does not show any performance
regressions, but assuming they don't, looks great !
Reviewed-By: Michel Lespinasse <walken@google.com>
On Sun, Oct 27, 2019 at 7:15 PM Wei Yang <richardw.yang@linux.intel.com> wrote:
>
> Both in Case 2 and 3, we exchange n and s. This mean no matter whether
> child2 is NULL or not, successor's parent should be assigned to node's.
>
> This patch takes this step out to make it explicit and reduce the
> ambiguity.
>
> Besides, this step reduces some symbol size like rb_erase().
>
> KERN_CONFIG upstream patched
> OPT_FOR_PERF 877 870
> OPT_FOR_SIZE 635 621
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> include/linux/rbtree_augmented.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/linux/rbtree_augmented.h b/include/linux/rbtree_augmented.h
> index fdd421b8d9ae..99c42e1a74b8 100644
> --- a/include/linux/rbtree_augmented.h
> +++ b/include/linux/rbtree_augmented.h
> @@ -283,14 +283,13 @@ __rb_erase_augmented(struct rb_node *node, struct rb_root *root,
> __rb_change_child(node, successor, tmp, root);
>
> if (child2) {
> - successor->__rb_parent_color = pc;
> rb_set_parent_color(child2, parent, RB_BLACK);
> rebalance = NULL;
> } else {
> unsigned long pc2 = successor->__rb_parent_color;
> - successor->__rb_parent_color = pc;
> rebalance = __rb_is_black(pc2) ? parent : NULL;
> }
> + successor->__rb_parent_color = pc;
> tmp = successor;
> }
>
> --
> 2.17.1
>
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally
2019-10-28 4:29 ` [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Michel Lespinasse
@ 2019-10-28 6:23 ` Wei Yang
0 siblings, 0 replies; 6+ messages in thread
From: Wei Yang @ 2019-10-28 6:23 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Wei Yang, Andrew Morton, Peter Zijlstra, Davidlohr Bueso,
Thomas Gleixner, LKML
On Sun, Oct 27, 2019 at 09:29:43PM -0700, Michel Lespinasse wrote:
>Code looks fine, for both commits in this series. Please make sure to
>double check that lib/rbtree_test does not show any performance
>regressions, but assuming they don't, looks great !
>
Thanks :-)
>Reviewed-By: Michel Lespinasse <walken@google.com>
>
>On Sun, Oct 27, 2019 at 7:15 PM Wei Yang <richardw.yang@linux.intel.com> wrote:
>>
>> Both in Case 2 and 3, we exchange n and s. This mean no matter whether
>> child2 is NULL or not, successor's parent should be assigned to node's.
>>
>> This patch takes this step out to make it explicit and reduce the
>> ambiguity.
>>
>> Besides, this step reduces some symbol size like rb_erase().
>>
>> KERN_CONFIG upstream patched
>> OPT_FOR_PERF 877 870
>> OPT_FOR_SIZE 635 621
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> include/linux/rbtree_augmented.h | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/linux/rbtree_augmented.h b/include/linux/rbtree_augmented.h
>> index fdd421b8d9ae..99c42e1a74b8 100644
>> --- a/include/linux/rbtree_augmented.h
>> +++ b/include/linux/rbtree_augmented.h
>> @@ -283,14 +283,13 @@ __rb_erase_augmented(struct rb_node *node, struct rb_root *root,
>> __rb_change_child(node, successor, tmp, root);
>>
>> if (child2) {
>> - successor->__rb_parent_color = pc;
>> rb_set_parent_color(child2, parent, RB_BLACK);
>> rebalance = NULL;
>> } else {
>> unsigned long pc2 = successor->__rb_parent_color;
>> - successor->__rb_parent_color = pc;
>> rebalance = __rb_is_black(pc2) ? parent : NULL;
>> }
>> + successor->__rb_parent_color = pc;
>> tmp = successor;
>> }
>>
>> --
>> 2.17.1
>>
>
>
>--
>Michel "Walken" Lespinasse
>A program is never fully debugged until the last user dies.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally
2019-10-28 2:14 [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Wei Yang
2019-10-28 2:14 ` [Patch v2 2/2] lib/rbtree: get successor's color directly Wei Yang
2019-10-28 4:29 ` [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Michel Lespinasse
@ 2019-10-28 9:20 ` Peter Zijlstra
2 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2019-10-28 9:20 UTC (permalink / raw)
To: Wei Yang; +Cc: akpm, walken, dbueso, tglx, linux-kernel
On Mon, Oct 28, 2019 at 10:14:41AM +0800, Wei Yang wrote:
> Both in Case 2 and 3, we exchange n and s. This mean no matter whether
> child2 is NULL or not, successor's parent should be assigned to node's.
>
> This patch takes this step out to make it explicit and reduce the
> ambiguity.
>
> Besides, this step reduces some symbol size like rb_erase().
>
> KERN_CONFIG upstream patched
> OPT_FOR_PERF 877 870
> OPT_FOR_SIZE 635 621
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-28 9:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-28 2:14 [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Wei Yang
2019-10-28 2:14 ` [Patch v2 2/2] lib/rbtree: get successor's color directly Wei Yang
2019-10-28 9:21 ` Peter Zijlstra
2019-10-28 4:29 ` [Patch v2 1/2] lib/rbtree: set successor's parent unconditionally Michel Lespinasse
2019-10-28 6:23 ` Wei Yang
2019-10-28 9:20 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox