Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Mon, Oct 02, 2006 at 04:49:38PM +0300, Ismail Donmez wrote:
02 Eki 2006 Pts 13:24 tarihinde, Jarek Poplawski şunlar?? yazm??şt??:
On 30-09-2006 21:23, Ismail Donmez wrote:
Hi,
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
...
- if (!RB_EMPTY_NODE(rb)) {
+ if (RB_EMPTY_NODE(rb)) {
Maybe you have some kind of agreement with Jens Axboe but I
can't understand current way of kernel cooperation:
he changes some global behavior to the opposite and fixes
his code in three places but can't fix it in the fourth place
where it's used? Isn't it both trivial and automatic kind
of patch?
I don't know if Jens will going to fix other occurrences but the sch_htb.c
fix
was recent and Jens' commit got my attention hence the patch.
As a matter of fact this kind of cooperation has also many pluses:
developers have to be more vigilant, read about other's work,
more people are engaged and last but not least there is this
infallible akpm reminding subsystem...
Best regards,
Jarek P.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Mon, Oct 02 2006, Jarek Poplawski wrote:
On Mon, Oct 02, 2006 at 12:24:47PM +0200, Jarek Poplawski wrote:
On 30-09-2006 21:23, Ismail Donmez wrote:
Hi,
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
...
- if (!RB_EMPTY_NODE(rb)) {
+ if (RB_EMPTY_NODE(rb)) {
Maybe you have some kind of agreement with Jens Axboe but I
can't understand current way of kernel cooperation:
he changes some global behavior to the opposite and fixes
his code in three places but can't fix it in the fourth place
...
But I see it's not so bad and net_sched isn't the last!
It looks deadline-iosched.c and one place in as-iosched.c
(~ 466 line) where probably also forgotten.
I don't see any missed changes.
Another question - is there any change planned?
If not why in rbtree.c is:
+ if (rb_parent(node) == node)
instead of:
+ if (RB_EMPTY_NODE(rb))
RB_EMPTY_NODE was (re)-introduced after that change, and it never
propagated. Changing it now would be fine and easier to read.
--
Jens Axboe
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Mon, Oct 02 2006, Ismail Donmez wrote:
02 Eki 2006 Pts 13:24 tarihinde, Jarek Poplawski ??unlar?? yazmt??:
On 30-09-2006 21:23, Ismail Donmez wrote:
Hi,
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
...
- if (!RB_EMPTY_NODE(rb)) {
+ if (RB_EMPTY_NODE(rb)) {
Maybe you have some kind of agreement with Jens Axboe but I
can't understand current way of kernel cooperation:
he changes some global behavior to the opposite and fixes
his code in three places but can't fix it in the fourth place
where it's used? Isn't it both trivial and automatic kind
of patch?
I don't know if Jens will going to fix other occurrences but the
sch_htb.c fix was recent and Jens' commit got my attention hence the
patch.
Send me a patch and I'll review / integrate it. Most of the rbtree
mixups stem from the fact that the first color change was incomplete.
--
Jens Axboe
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Tue, Oct 03, 2006 at 12:28:32PM +0200, Jens Axboe wrote: ... I don't see any missed changes. And you are right! I've checked the link from the first message of this thread and now I see it's not current enough. I'm sorry for this false alarm. Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
From: Patrick McHardy [EMAIL PROTECTED] Date: Tue, 03 Oct 2006 20:05:33 +0200 Ismail Donmez wrote: With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE changed behaviour so it returns true when the node is empty as expected. Hence Patrick McHardy's fix for sched_htb.c should be reverted. Signed-off-by: Ismail Donmez [EMAIL PROTECTED] ACKed-by: Patrick McHardy [EMAIL PROTECTED] Applied, thanks everyone. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On 30-09-2006 21:23, Ismail Donmez wrote:
Hi,
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
...
- if (!RB_EMPTY_NODE(rb)) {
+ if (RB_EMPTY_NODE(rb)) {
Maybe you have some kind of agreement with Jens Axboe but I
can't understand current way of kernel cooperation:
he changes some global behavior to the opposite and fixes
his code in three places but can't fix it in the fourth place
where it's used? Isn't it both trivial and automatic kind
of patch?
Second question is this title alarming enough?:
[PATCH] rbtree: fixed reversed RB_EMPTY_NODE and rb_next/prev
Maybe:
[PATCH] rbtree: reversed RB_EMPTY_NODE and fixed rb_next/prev
but shouldn't there be some special [XYZ!] keyword used for
such severe situations?
Jarek P.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Mon, Oct 02, 2006 at 01:15:55PM +0200, Jarek Poplawski wrote: ... instead of: + if (RB_EMPTY_NODE(rb)) should be: + if (RB_EMPTY_NODE(node)) Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
02 Eki 2006 Pts 13:24 tarihinde, Jarek Poplawski şunları yazmıştı:
On 30-09-2006 21:23, Ismail Donmez wrote:
Hi,
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
...
- if (!RB_EMPTY_NODE(rb)) {
+ if (RB_EMPTY_NODE(rb)) {
Maybe you have some kind of agreement with Jens Axboe but I
can't understand current way of kernel cooperation:
he changes some global behavior to the opposite and fixes
his code in three places but can't fix it in the fourth place
where it's used? Isn't it both trivial and automatic kind
of patch?
I don't know if Jens will going to fix other occurrences but the sch_htb.c fix
was recent and Jens' commit got my attention hence the patch.
Regards,
ismail
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Sunday 01 October 2006 04:52, Herbert Xu wrote:
On Sat, Sep 30, 2006 at 10:23:46PM +0300, Ismail Donmez wrote:
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
Hence Herbert's fix for sched_htb.c should be reverted.
I've fixed sched_htb.c? That's news to me :)
I fully agree with your patch though.
Oh it was Patrick McHardy [EMAIL PROTECTED], sorry. Patch again with a
Signed-off line too this time.
Hi,
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
Hence Patrick McHardy's fix for sched_htb.c should be reverted.
[1]
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=10fd48f2376db52f08bf0420d2c4f580e39269e1;hp=9817064b68fef7e4580c6df1ea597e106b9ff88b
Signed-off-by: Ismail Donmez [EMAIL PROTECTED]
Regards,
ismail
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 6c058e3..1f1360e 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -391,7 +391,7 @@ static inline void htb_add_class_to_row(
/* If this triggers, it is a bug in this code, but it need not be fatal */
static void htb_safe_rb_erase(struct rb_node *rb, struct rb_root *root)
{
- if (!RB_EMPTY_NODE(rb)) {
+ if (RB_EMPTY_NODE(rb)) {
WARN_ON(1);
} else {
rb_erase(rb, root);
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Saturday 30 September 2006 22:23, you wrote: Hi, With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE changed behaviour so it returns false when the node is empty as expected. ^ make it : so it returns true when the node is empty as expected /ismail -- They that can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety. -- Benjamin Franklin - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert [NET_SCHED]: HTB: fix incorrect use of RB_EMPTY_NODE
On Sat, Sep 30, 2006 at 10:23:46PM +0300, Ismail Donmez wrote:
With commit 10fd48f2376db52f08bf0420d2c4f580e39269e1 [1] , RB_EMPTY_NODE
changed behaviour so it returns false when the node is empty as expected.
Hence Herbert's fix for sched_htb.c should be reverted.
I've fixed sched_htb.c? That's news to me :)
I fully agree with your patch though.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
