* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-15 3:18 [PATCH] block: make CONFIG_BLK_CGROUP visible Li Zefan
@ 2010-03-15 4:31 ` Ben Blum
2010-03-15 11:57 ` Jens Axboe
2010-03-15 13:21 ` Vivek Goyal
2 siblings, 0 replies; 8+ messages in thread
From: Ben Blum @ 2010-03-15 4:31 UTC (permalink / raw)
To: Li Zefan
Cc: Jens Axboe, Vivek Goyal, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-15 3:18 [PATCH] block: make CONFIG_BLK_CGROUP visible Li Zefan
2010-03-15 4:31 ` Ben Blum
@ 2010-03-15 11:57 ` Jens Axboe
2010-03-16 1:42 ` Li Zefan
2010-03-15 13:21 ` Vivek Goyal
2 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2010-03-15 11:57 UTC (permalink / raw)
To: Li Zefan
Cc: Vivek Goyal, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-15 11:57 ` Jens Axboe
@ 2010-03-16 1:42 ` Li Zefan
2010-03-16 7:56 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Li Zefan @ 2010-03-16 1:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Vivek Goyal, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-16 1:42 ` Li Zefan
@ 2010-03-16 7:56 ` Jens Axboe
0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2010-03-16 7:56 UTC (permalink / raw)
To: Li Zefan
Cc: Vivek Goyal, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-15 3:18 [PATCH] block: make CONFIG_BLK_CGROUP visible Li Zefan
2010-03-15 4:31 ` Ben Blum
2010-03-15 11:57 ` Jens Axboe
@ 2010-03-15 13:21 ` Vivek Goyal
2010-03-16 1:36 ` Li Zefan
2 siblings, 1 reply; 8+ messages in thread
From: Vivek Goyal @ 2010-03-15 13:21 UTC (permalink / raw)
To: Li Zefan
Cc: Jens Axboe, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-15 13:21 ` Vivek Goyal
@ 2010-03-16 1:36 ` Li Zefan
2010-03-16 13:53 ` Vivek Goyal
0 siblings, 1 reply; 8+ messages in thread
From: Li Zefan @ 2010-03-16 1:36 UTC (permalink / raw)
To: Vivek Goyal
Cc: Jens Axboe, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
>> ---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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: make CONFIG_BLK_CGROUP visible
2010-03-16 1:36 ` Li Zefan
@ 2010-03-16 13:53 ` Vivek Goyal
0 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2010-03-16 13:53 UTC (permalink / raw)
To: Li Zefan
Cc: Jens Axboe, Gui Jianfeng, Ben Blum, LKML,
containers@lists.osdl.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread