[2.6 patch] remove CONFIG_NET_SCH_RR

[2.6 patch] remove CONFIG_NET_SCH_RR

[Adrian Bunk]

Commit d62733c8e437fdb58325617c4b3331769ba82d70
([SCHED]: Qdisc changes and sch_rr added for multiqueue)
added a NET_SCH_RR option that was unused since the code
went unconditionally into sch_prio.

Reported-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Adrian Bunk <bunk@kernel.org>

---
572f329b3e8fc3648f7585e36291fcd30dd7daec diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 82adfe6..9437b27 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -106,17 +106,6 @@ config NET_SCH_PRIO
 	  To compile this code as a module, choose M here: the
 	  module will be called sch_prio.
 
-config NET_SCH_RR
-	tristate "Multi Band Round Robin Queuing (RR)"
-	select NET_SCH_PRIO
-	---help---
-	  Say Y here if you want to use an n-band round robin packet
-	  scheduler.
-
-	  The module uses sch_prio for its framework and is aliased as
-	  sch_rr, so it will load sch_prio, although it is referred
-	  to using sch_rr.
-
 config NET_SCH_RED
 	tristate "Random Early Detection (RED)"
 	---help---

RE: [2.6 patch] remove CONFIG_NET_SCH_RR

[Waskiewicz Jr, Peter P]

>Commit d62733c8e437fdb58325617c4b3331769ba82d70
>([SCHED]: Qdisc changes and sch_rr added for multiqueue)
>added a NET_SCH_RR option that was unused since the code
>went unconditionally into sch_prio.

I don't agree we should remove this option, purely for the fact that
there isn't any way a user would know the qdisc existed without reading
the code.  I'd rather see a change to sch_prio.c to correctly wrap the
RR code, which I should have done in the first place.  My hurdle is when
the module is selected, NET_SCH_RR isn't defined to be used as an #ifdef
check in the code.  Only when it's selected to be built-in is the symbol
defined.  If I can get around that, I'll generate a patch to properly
wrap sch_prio.c code.  I know I'm missing something silly, so any
guidance is appreciated.

-PJ Waskiewicz

Re: [2.6 patch] remove CONFIG_NET_SCH_RR

[Patrick McHardy]

Waskiewicz Jr, Peter P wrote:
>> Commit d62733c8e437fdb58325617c4b3331769ba82d70
>> ([SCHED]: Qdisc changes and sch_rr added for multiqueue)
>> added a NET_SCH_RR option that was unused since the code
>> went unconditionally into sch_prio.
> 
> I don't agree we should remove this option, purely for the fact that
> there isn't any way a user would know the qdisc existed without reading
> the code.  I'd rather see a change to sch_prio.c to correctly wrap the
> RR code, which I should have done in the first place.  My hurdle is when
> the module is selected, NET_SCH_RR isn't defined to be used as an #ifdef
> check in the code.  Only when it's selected to be built-in is the symbol
> defined.  If I can get around that, I'll generate a patch to properly
> wrap sch_prio.c code.  I know I'm missing something silly, so any
> guidance is appreciated.

For modular selected options, the name is <name>_MODULE (so
CONFIG_NET_SCH_RR_MODULE in this case).

I tend to agree with Adrian though, this option doesn't
provide any value.

Re: [2.6 patch] remove CONFIG_NET_SCH_RR

[Adrian Bunk]

On Mon, Jun 23, 2008 at 11:31:30AM -0700, Waskiewicz Jr, Peter P wrote:
> >Commit d62733c8e437fdb58325617c4b3331769ba82d70
> >([SCHED]: Qdisc changes and sch_rr added for multiqueue)
> >added a NET_SCH_RR option that was unused since the code
> >went unconditionally into sch_prio.
> 
> I don't agree we should remove this option, purely for the fact that
> there isn't any way a user would know the qdisc existed without reading
> the code.

Most users of the kernel will anyway use a distribution kernel, and 
therefore never see it.

If the problem is a lack of any user documentation then kconfig is wrong 
place to attack that.

> I'd rather see a change to sch_prio.c to correctly wrap the
> RR code, which I should have done in the first place.  My hurdle is when
> the module is selected, NET_SCH_RR isn't defined to be used as an #ifdef
> check in the code. Only when it's selected to be built-in is the symbol
> defined.  If I can get around that, I'll generate a patch to properly
> wrap sch_prio.c code.  I know I'm missing something silly, so any
> guidance is appreciated.

In this case NET_SCH_RR is not a module but a feature inside 
NET_SCH_PRIO, and that can be expressed as follows:

config NET_SCH_RR
        bool "Multi Band Round Robin Queuing (RR)"
        depends on NET_SCH_PRIO

But I doubt making these #ifdef's makes much sense.

IMHO the best you can do is to add a note about RR to the NET_SCH_PRIO 
help text.

> -PJ Waskiewicz

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

RE: [2.6 patch] remove CONFIG_NET_SCH_RR

[Waskiewicz Jr, Peter P]

>But I doubt making these #ifdef's makes much sense.
>
>IMHO the best you can do is to add a note about RR to the NET_SCH_PRIO 
>help text.

Agreed.  I can generate a change to the help text to prio for it.

Thanks Adrian,

-PJ Waskiewicz

Re: [2.6 patch] remove CONFIG_NET_SCH_RR

[David Miller]

From: "Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@intel.com>
Date: Mon, 23 Jun 2008 11:31:30 -0700

> >Commit d62733c8e437fdb58325617c4b3331769ba82d70
> >([SCHED]: Qdisc changes and sch_rr added for multiqueue)
> >added a NET_SCH_RR option that was unused since the code
> >went unconditionally into sch_prio.
> 
> I don't agree we should remove this option, purely for the fact that
> there isn't any way a user would know the qdisc existed without reading
> the code.

That's not how config options work.

We add features and facilities all the time that lack CONFIG
option entries.

And frankly, I plan on removing the RR scheduler in the TX
multiqueue bits I'm doing.

But even if I didn't plan this, we should remove these unused
CONFIG variables, because that is our tree wide policy.  We
don't use them as dinky knobs for documentation.

Re: [2.6 patch] remove CONFIG_NET_SCH_RR

[David Miller]

From: Adrian Bunk <bunk@kernel.org>
Date: Mon, 23 Jun 2008 20:47:02 +0300

> Commit d62733c8e437fdb58325617c4b3331769ba82d70
> ([SCHED]: Qdisc changes and sch_rr added for multiqueue)
> added a NET_SCH_RR option that was unused since the code
> went unconditionally into sch_prio.
> 
> Reported-by: Robert P. J. Day <rpjday@crashcourse.ca>
> Signed-off-by: Adrian Bunk <bunk@kernel.org>

Applied, thanks Adrian.