public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* 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