* cfq-iosched.c:RB_CLEAR_COLOR @ 2006-06-21 0:34 David Miller 2006-06-21 1:03 ` cfq-iosched.c:RB_CLEAR_COLOR Linus Torvalds 0 siblings, 1 reply; 4+ messages in thread From: David Miller @ 2006-06-21 0:34 UTC (permalink / raw) To: linux-kernel; +Cc: torvalds, akpm, dwmw2 That got removed in Linus's tree today, yet cfq-iosched.c still contains a reference to it. The culprit changeset seems to be 3db3a44. There were two explicit calls in the cfq-iosched.c file to RB_CLEAR_COLOR, only the one in cfq_del_crq_rb() got removed so the build fails. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cfq-iosched.c:RB_CLEAR_COLOR 2006-06-21 0:34 cfq-iosched.c:RB_CLEAR_COLOR David Miller @ 2006-06-21 1:03 ` Linus Torvalds 2006-06-21 7:31 ` cfq-iosched.c:RB_CLEAR_COLOR David Woodhouse 0 siblings, 1 reply; 4+ messages in thread From: Linus Torvalds @ 2006-06-21 1:03 UTC (permalink / raw) To: David Miller; +Cc: linux-kernel, akpm, dwmw2 On Tue, 20 Jun 2006, David Miller wrote: > > That got removed in Linus's tree today, yet cfq-iosched.c > still contains a reference to it. > > The culprit changeset seems to be 3db3a44. > > There were two explicit calls in the cfq-iosched.c file > to RB_CLEAR_COLOR, only the one in cfq_del_crq_rb() got > removed so the build fails. I think the right fix is to just remove the RB_CLEAR_COLOR() call, since the memset will set everything to 0/NULL, which should be the correct initialization these days anyway. David (the other one - dwmw2), pls confirm? Linus ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cfq-iosched.c:RB_CLEAR_COLOR 2006-06-21 1:03 ` cfq-iosched.c:RB_CLEAR_COLOR Linus Torvalds @ 2006-06-21 7:31 ` David Woodhouse 2006-06-21 7:38 ` cfq-iosched.c:RB_CLEAR_COLOR Jens Axboe 0 siblings, 1 reply; 4+ messages in thread From: David Woodhouse @ 2006-06-21 7:31 UTC (permalink / raw) To: Linus Torvalds; +Cc: David Miller, linux-kernel, akpm, Jens Axboe On Tue, 2006-06-20 at 18:03 -0700, Linus Torvalds wrote: > > There were two explicit calls in the cfq-iosched.c file > > to RB_CLEAR_COLOR, only the one in cfq_del_crq_rb() got > > removed so the build fails. Apologies for that; the new one got added only last week, and escaped my attention. > I think the right fix is to just remove the RB_CLEAR_COLOR() call, since > the memset will set everything to 0/NULL, which should be the correct > initialization these days anyway. > > David (the other one - dwmw2), pls confirm? Yes, that looks correct. Other code, including the AS scheduler, was (ab)using the colour field by storing a 'RB_NONE' value which was neither red nor black to mark an 'off-tree' node, then checking for it with ON_RB(). I changed that scheme -- we now set the node's parent pointer to point to itself to mark an off-tree node. However, the cfq scheduler looks like it only inherited the macros from AS, and was never actually _checking_ if a given node was on-tree or not. So just dropping the magic initialisation stuff there is fine. -- dwmw2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cfq-iosched.c:RB_CLEAR_COLOR 2006-06-21 7:31 ` cfq-iosched.c:RB_CLEAR_COLOR David Woodhouse @ 2006-06-21 7:38 ` Jens Axboe 0 siblings, 0 replies; 4+ messages in thread From: Jens Axboe @ 2006-06-21 7:38 UTC (permalink / raw) To: David Woodhouse; +Cc: Linus Torvalds, David Miller, linux-kernel, akpm On Wed, Jun 21 2006, David Woodhouse wrote: > On Tue, 2006-06-20 at 18:03 -0700, Linus Torvalds wrote: > > > There were two explicit calls in the cfq-iosched.c file > > > to RB_CLEAR_COLOR, only the one in cfq_del_crq_rb() got > > > removed so the build fails. > > Apologies for that; the new one got added only last week, and escaped my > attention. > > > I think the right fix is to just remove the RB_CLEAR_COLOR() call, since > > the memset will set everything to 0/NULL, which should be the correct > > initialization these days anyway. > > > > David (the other one - dwmw2), pls confirm? > > Yes, that looks correct. Other code, including the AS scheduler, was > (ab)using the colour field by storing a 'RB_NONE' value which was > neither red nor black to mark an 'off-tree' node, then checking for it > with ON_RB(). I changed that scheme -- we now set the node's parent > pointer to point to itself to mark an off-tree node. > > However, the cfq scheduler looks like it only inherited the macros from > AS, and was never actually _checking_ if a given node was on-tree or > not. So just dropping the magic initialisation stuff there is fine. I cleaned up the ioscheduler rb private defines now, instead of having each roll their own. Result is here: http://brick.kernel.dk/git/?p=linux-2.6-block.git;a=commitdiff;h=52e7beda68fe0b08d74b6665d47e6024efe46101 -- Jens Axboe ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-06-21 7:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-06-21 0:34 cfq-iosched.c:RB_CLEAR_COLOR David Miller 2006-06-21 1:03 ` cfq-iosched.c:RB_CLEAR_COLOR Linus Torvalds 2006-06-21 7:31 ` cfq-iosched.c:RB_CLEAR_COLOR David Woodhouse 2006-06-21 7:38 ` cfq-iosched.c:RB_CLEAR_COLOR Jens Axboe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox