[PATCH] block: make CONFIG_BLK_CGROUP visible

[PATCH] block: make CONFIG_BLK_CGROUP visible

[Li Zefan]

Make the config visible, so we can choose from CONFIG_BLK_CGROUP=y
and CONFIG_BLK_CGROUP=m when CONFIG_IOSCHED_CFQ=m.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 block/Kconfig |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/Kconfig b/block/Kconfig
index 62a5921..906950c 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -78,8 +78,9 @@ config BLK_DEV_INTEGRITY
 	Protection.  If in doubt, say N.
 
 config BLK_CGROUP
-	tristate
+	tristate "Block cgroup support"
 	depends on CGROUPS
+	depends on CFQ_GROUP_IOSCHED
 	default n
 	---help---
 	Generic block IO controller cgroup interface. This is the common
@@ -91,7 +92,7 @@ config BLK_CGROUP
 	to such task groups.
 
 config DEBUG_BLK_CGROUP
-	bool
+	bool "Block cgroup debugging help"
 	depends on BLK_CGROUP
 	default n
 	---help---
-- 
1.6.3

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Ben Blum]

On Mon, Mar 15, 2010 at 11:18:13AM +0800, Li Zefan wrote:
> Make the config visible, so we can choose from CONFIG_BLK_CGROUP=y
> and CONFIG_BLK_CGROUP=m when CONFIG_IOSCHED_CFQ=m.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  block/Kconfig |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index 62a5921..906950c 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -78,8 +78,9 @@ config BLK_DEV_INTEGRITY
>  	Protection.  If in doubt, say N.
>  
>  config BLK_CGROUP
> -	tristate
> +	tristate "Block cgroup support"
>  	depends on CGROUPS
> +	depends on CFQ_GROUP_IOSCHED
>  	default n
>  	---help---
>  	Generic block IO controller cgroup interface. This is the common
> @@ -91,7 +92,7 @@ config BLK_CGROUP
>  	to such task groups.
>  
>  config DEBUG_BLK_CGROUP
> -	bool
> +	bool "Block cgroup debugging help"
>  	depends on BLK_CGROUP
>  	default n
>  	---help---
> -- 
> 1.6.3
> 
> 

LGTM.

Acked-by: Ben Blum <bblum@andrew.cmu.edu>

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Jens Axboe]

On Mon, Mar 15 2010, Li Zefan wrote:
> Make the config visible, so we can choose from CONFIG_BLK_CGROUP=y
> and CONFIG_BLK_CGROUP=m when CONFIG_IOSCHED_CFQ=m.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

What kernel is this against?

> ---
>  block/Kconfig |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index 62a5921..906950c 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -78,8 +78,9 @@ config BLK_DEV_INTEGRITY
>  	Protection.  If in doubt, say N.
>  
>  config BLK_CGROUP
> -	tristate
> +	tristate "Block cgroup support"
>  	depends on CGROUPS
> +	depends on CFQ_GROUP_IOSCHED
>  	default n
>  	---help---
>  	Generic block IO controller cgroup interface. This is the common
> @@ -91,7 +92,7 @@ config BLK_CGROUP
>  	to such task groups.
>  
>  config DEBUG_BLK_CGROUP
> -	bool
> +	bool "Block cgroup debugging help"
>  	depends on BLK_CGROUP
>  	default n
>  	---help---
> -- 
> 1.6.3
> 

-- 
Jens Axboe

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Vivek Goyal]

On Mon, Mar 15, 2010 at 11:18:13AM +0800, Li Zefan wrote:
> Make the config visible, so we can choose from CONFIG_BLK_CGROUP=y
> and CONFIG_BLK_CGROUP=m when CONFIG_IOSCHED_CFQ=m.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  block/Kconfig |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index 62a5921..906950c 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -78,8 +78,9 @@ config BLK_DEV_INTEGRITY
>  	Protection.  If in doubt, say N.
>  
>  config BLK_CGROUP
> -	tristate
> +	tristate "Block cgroup support"
>  	depends on CGROUPS
> +	depends on CFQ_GROUP_IOSCHED
>  	default n

Hi Gui,

This part makes sense. If need to give user an option to keep BLK_CGROUP=y
even if CFQ=m.

>  	---help---
>  	Generic block IO controller cgroup interface. This is the common
> @@ -91,7 +92,7 @@ config BLK_CGROUP
>  	to such task groups.
>  
>  config DEBUG_BLK_CGROUP
> -	bool
> +	bool "Block cgroup debugging help"


Why are you making DEBUG_BLK_CGROUP this as a user visible/configurable
option? This is already controlled by DEBUG_CFQ_IOSCHED. If you don't want
the DEBUG overhead, just set DEBUG_CFQ_IOSCHED=n and DEBUG_BLK_CGROUP will
not be selected? Making it user visible does not seem to be buying us
anything?

Thanks
Vivek

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Li Zefan]

>>  	---help---
>>  	Generic block IO controller cgroup interface. This is the common
>> @@ -91,7 +92,7 @@ config BLK_CGROUP
>>  	to such task groups.
>>  
>>  config DEBUG_BLK_CGROUP
>> -	bool
>> +	bool "Block cgroup debugging help"
> 
> 
> Why are you making DEBUG_BLK_CGROUP this as a user visible/configurable
> option? This is already controlled by DEBUG_CFQ_IOSCHED. If you don't want
> the DEBUG overhead, just set DEBUG_CFQ_IOSCHED=n and DEBUG_BLK_CGROUP will
> not be selected? Making it user visible does not seem to be buying us
> anything?
> 

Sounds reasonable. A minor question, since DEBUG_BLK_CGROUP is not
visible, the help message for this config is not visible too, so we
still keep it?

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Li Zefan]

Jens Axboe wrote:
> On Mon, Mar 15 2010, Li Zefan wrote:
>> Make the config visible, so we can choose from CONFIG_BLK_CGROUP=y
>> and CONFIG_BLK_CGROUP=m when CONFIG_IOSCHED_CFQ=m.
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> 
> What kernel is this against?
> 

It's against linus' tree. I've just checked the block tree, and found
it needs to be synced, otherwise this patch can't be applied on it.

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Jens Axboe]

On Tue, Mar 16 2010, Li Zefan wrote:
> Jens Axboe wrote:
> > On Mon, Mar 15 2010, Li Zefan wrote:
> >> Make the config visible, so we can choose from CONFIG_BLK_CGROUP=y
> >> and CONFIG_BLK_CGROUP=m when CONFIG_IOSCHED_CFQ=m.
> >>
> >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > 
> > What kernel is this against?
> > 
> 
> It's against linus' tree. I've just checked the block tree, and found
> it needs to be synced, otherwise this patch can't be applied on it.

OK, I'll hand apply it.

-- 
Jens Axboe

Re: [PATCH] block: make CONFIG_BLK_CGROUP visible

[Vivek Goyal]

On Tue, Mar 16, 2010 at 09:36:41AM +0800, Li Zefan wrote:
> >>  	---help---
> >>  	Generic block IO controller cgroup interface. This is the common
> >> @@ -91,7 +92,7 @@ config BLK_CGROUP
> >>  	to such task groups.
> >>  
> >>  config DEBUG_BLK_CGROUP
> >> -	bool
> >> +	bool "Block cgroup debugging help"
> > 
> > 
> > Why are you making DEBUG_BLK_CGROUP this as a user visible/configurable
> > option? This is already controlled by DEBUG_CFQ_IOSCHED. If you don't want
> > the DEBUG overhead, just set DEBUG_CFQ_IOSCHED=n and DEBUG_BLK_CGROUP will
> > not be selected? Making it user visible does not seem to be buying us
> > anything?
> > 
> 
> Sounds reasonable. A minor question, since DEBUG_BLK_CGROUP is not
> visible, the help message for this config is not visible too, so we
> still keep it?

Right now the message is only for developer if somebody opens the Kconfig
file. I think it does not harm if somebody wants to understand what this
config option is doing. But if you think that it should not be there, I have
no strong opinion about it.

Thanks
Vivek