lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <n2iaf41c7c41004221642if5d7ea8cz47ae1e6458420259@mail.gmail.com>
Date:	Fri, 23 Apr 2010 05:12:32 +0530
From:	Divyesh Shah <dpshah@...gle.com>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	linux kernel mailing list <linux-kernel@...r.kernel.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	Nauman Rafique <nauman@...gle.com>,
	Gui Jianfeng <guijianfeng@...fujitsu.com>
Subject: Re: [PATCH] blk-cgroup: config options re-arrangement

Thanks for the config options cleanup. Looks better now. LGTM

Acked-by: Divyesh Shah <dpshah@...gle.com>

On Fri, Apr 23, 2010 at 4:07 AM, Vivek Goyal <vgoyal@...hat.com> wrote:
> This patch fixes few usability and configurability issues.
>
> o All the cgroup based controller options are configurable from
>  "Genral Setup/Control Group Support/" menu. blkio is the only exception.
>  Hence make this option visible in above menu and make it configurable from
>  there to bring it inline with rest of the cgroup based controllers.
>
> o Get rid of CONFIG_DEBUG_CFQ_IOSCHED.
>
>  This option currently does two things.
>
>  - Enable printing of cgroup paths in blktrace
>  - Enables CONFIG_DEBUG_BLK_CGROUP, which in turn displays additional stat
>    files in cgroup.
>
>  If we are using group scheduling, blktrace data is of not really much use
>  if cgroup information is not present. To get this data, currently one has to
>  also enable CONFIG_DEBUG_CFQ_IOSCHED, which in turn brings the overhead of
>  all the additional debug stat files which is not desired.
>
>  Hence, this patch moves printing of cgroup paths under
>  CONFIG_CFQ_GROUP_IOSCHED.
>
>  This allows us to get rid of CONFIG_DEBUG_CFQ_IOSCHED completely. Now all
>  the debug stat files are controlled only by CONFIG_DEBUG_BLK_CGROUP which
>  can be enabled through config menu.
>
> Signed-off-by: Vivek Goyal <vgoyal@...hat.com>
> ---
>  Documentation/cgroups/blkio-controller.txt |   35 ++++++++++++----------------
>  block/Kconfig                              |   23 ------------------
>  block/Kconfig.iosched                      |   16 ++++--------
>  block/blk-cgroup.c                         |    2 -
>  block/blk-cgroup.h                         |   14 +++++-----
>  block/cfq-iosched.c                        |    2 +-
>  init/Kconfig                               |   27 +++++++++++++++++++++
>  7 files changed, 55 insertions(+), 64 deletions(-)
>
> diff --git a/Documentation/cgroups/blkio-controller.txt b/Documentation/cgroups/blkio-controller.txt
> index d422b41..48e0b21 100644
> --- a/Documentation/cgroups/blkio-controller.txt
> +++ b/Documentation/cgroups/blkio-controller.txt
> @@ -17,6 +17,9 @@ HOWTO
>  You can do a very simple testing of running two dd threads in two different
>  cgroups. Here is what you can do.
>
> +- Enable Block IO controller
> +       CONFIG_BLK_CGROUP=y
> +
>  - Enable group scheduling in CFQ
>        CONFIG_CFQ_GROUP_IOSCHED=y
>
> @@ -54,24 +57,16 @@ cgroups. Here is what you can do.
>
>  Various user visible config options
>  ===================================
> -CONFIG_CFQ_GROUP_IOSCHED
> -       - Enables group scheduling in CFQ. Currently only 1 level of group
> -         creation is allowed.
> -
> -CONFIG_DEBUG_CFQ_IOSCHED
> -       - Enables some debugging messages in blktrace. Also creates extra
> -         cgroup file blkio.dequeue.
> -
> -Config options selected automatically
> -=====================================
> -These config options are not user visible and are selected/deselected
> -automatically based on IO scheduler configuration.
> -
>  CONFIG_BLK_CGROUP
> -       - Block IO controller. Selected by CONFIG_CFQ_GROUP_IOSCHED.
> +       - Block IO controller.
>
>  CONFIG_DEBUG_BLK_CGROUP
> -       - Debug help. Selected by CONFIG_DEBUG_CFQ_IOSCHED.
> +       - Debug help. Right now some additional stats file show up in cgroup
> +         if this option is enabled.
> +
> +CONFIG_CFQ_GROUP_IOSCHED
> +       - Enables group scheduling in CFQ. Currently only 1 level of group
> +         creation is allowed.
>
>  Details of cgroup files
>  =======================
> @@ -174,13 +169,13 @@ Details of cgroup files
>          write, sync or async.
>
>  - blkio.avg_queue_size
> -       - Debugging aid only enabled if CONFIG_DEBUG_CFQ_IOSCHED=y.
> +       - Debugging aid only enabled if CONFIG_DEBUG_BLK_CGROUP=y.
>          The average queue size for this cgroup over the entire time of this
>          cgroup's existence. Queue size samples are taken each time one of the
>          queues of this cgroup gets a timeslice.
>
>  - blkio.group_wait_time
> -       - Debugging aid only enabled if CONFIG_DEBUG_CFQ_IOSCHED=y.
> +       - Debugging aid only enabled if CONFIG_DEBUG_BLK_CGROUP=y.
>          This is the amount of time the cgroup had to wait since it became busy
>          (i.e., went from 0 to 1 request queued) to get a timeslice for one of
>          its queues. This is different from the io_wait_time which is the
> @@ -191,7 +186,7 @@ Details of cgroup files
>          got a timeslice and will not include the current delta.
>
>  - blkio.empty_time
> -       - Debugging aid only enabled if CONFIG_DEBUG_CFQ_IOSCHED=y.
> +       - Debugging aid only enabled if CONFIG_DEBUG_BLK_CGROUP=y.
>          This is the amount of time a cgroup spends without any pending
>          requests when not being served, i.e., it does not include any time
>          spent idling for one of the queues of the cgroup. This is in
> @@ -200,7 +195,7 @@ Details of cgroup files
>          time it had a pending request and will not include the current delta.
>
>  - blkio.idle_time
> -       - Debugging aid only enabled if CONFIG_DEBUG_CFQ_IOSCHED=y.
> +       - Debugging aid only enabled if CONFIG_DEBUG_BLK_CGROUP=y.
>          This is the amount of time spent by the IO scheduler idling for a
>          given cgroup in anticipation of a better request than the exising ones
>          from other queues/cgroups. This is in nanoseconds. If this is read
> @@ -209,7 +204,7 @@ Details of cgroup files
>          the current delta.
>
>  - blkio.dequeue
> -       - Debugging aid only enabled if CONFIG_DEBUG_CFQ_IOSCHED=y. This
> +       - Debugging aid only enabled if CONFIG_DEBUG_BLK_CGROUP=y. This
>          gives the statistics about how many a times a group was dequeued
>          from service tree of the device. First two fields specify the major
>          and minor number of the device and third field specifies the number
> diff --git a/block/Kconfig b/block/Kconfig
> index f9e89f4..9be0b56 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -77,29 +77,6 @@ config BLK_DEV_INTEGRITY
>        T10/SCSI Data Integrity Field or the T13/ATA External Path
>        Protection.  If in doubt, say N.
>
> -config BLK_CGROUP
> -       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
> -       cgroup interface which should be used by various IO controlling
> -       policies.
> -
> -       Currently, CFQ IO scheduler uses it to recognize task groups and
> -       control disk bandwidth allocation (proportional time slice allocation)
> -       to such task groups.
> -
> -config DEBUG_BLK_CGROUP
> -       bool
> -       depends on BLK_CGROUP
> -       default n
> -       ---help---
> -       Enable some debugging help. Currently it stores the cgroup path
> -       in the blk group which can be used by cfq for tracing various
> -       group related activity.
> -
>  endif # BLOCK
>
>  config BLOCK_COMPAT
> diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
> index fc71cf0..3199b76 100644
> --- a/block/Kconfig.iosched
> +++ b/block/Kconfig.iosched
> @@ -23,7 +23,8 @@ config IOSCHED_DEADLINE
>
>  config IOSCHED_CFQ
>        tristate "CFQ I/O scheduler"
> -       select BLK_CGROUP if CFQ_GROUP_IOSCHED
> +       # If BLK_CGROUP is a module, CFQ has to be built as module.
> +       depends on (BLK_CGROUP=m && m) || !BLK_CGROUP || BLK_CGROUP=y
>        default y
>        ---help---
>          The CFQ I/O scheduler tries to distribute bandwidth equally
> @@ -33,22 +34,15 @@ config IOSCHED_CFQ
>
>          This is the default I/O scheduler.
>
> +         Note: If BLK_CGROUP=m, then CFQ can be built only as module.
> +
>  config CFQ_GROUP_IOSCHED
>        bool "CFQ Group Scheduling support"
> -       depends on IOSCHED_CFQ && CGROUPS
> +       depends on IOSCHED_CFQ && BLK_CGROUP
>        default n
>        ---help---
>          Enable group IO scheduling in CFQ.
>
> -config DEBUG_CFQ_IOSCHED
> -       bool "Debug CFQ Scheduling"
> -       depends on CFQ_GROUP_IOSCHED
> -       select DEBUG_BLK_CGROUP
> -       default n
> -       ---help---
> -         Enable CFQ IO scheduling debugging in CFQ. Currently it makes
> -         blktrace output more verbose.
> -
>  choice
>        prompt "Default I/O scheduler"
>        default DEFAULT_CFQ
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index 83930f6..92d3b74 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -348,10 +348,8 @@ void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg,
>        blkg->blkcg_id = css_id(&blkcg->css);
>        hlist_add_head_rcu(&blkg->blkcg_node, &blkcg->blkg_list);
>        spin_unlock_irqrestore(&blkcg->lock, flags);
> -#ifdef CONFIG_DEBUG_BLK_CGROUP
>        /* Need to take css reference ? */
>        cgroup_path(blkcg->css.cgroup, blkg->path, sizeof(blkg->path));
> -#endif
>        blkg->dev = dev;
>  }
>  EXPORT_SYMBOL_GPL(blkiocg_add_blkio_group);
> diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
> index 2c956a0..8a357f1 100644
> --- a/block/blk-cgroup.h
> +++ b/block/blk-cgroup.h
> @@ -108,10 +108,8 @@ struct blkio_group {
>        void *key;
>        struct hlist_node blkcg_node;
>        unsigned short blkcg_id;
> -#ifdef CONFIG_DEBUG_BLK_CGROUP
>        /* Store cgroup path */
>        char path[128];
> -#endif
>        /* The device MKDEV(major, minor), this group has been created for */
>        dev_t dev;
>
> @@ -147,6 +145,11 @@ struct blkio_policy_type {
>  extern void blkio_policy_register(struct blkio_policy_type *);
>  extern void blkio_policy_unregister(struct blkio_policy_type *);
>
> +static inline char *blkg_path(struct blkio_group *blkg)
> +{
> +       return blkg->path;
> +}
> +
>  #else
>
>  struct blkio_group {
> @@ -158,6 +161,8 @@ struct blkio_policy_type {
>  static inline void blkio_policy_register(struct blkio_policy_type *blkiop) { }
>  static inline void blkio_policy_unregister(struct blkio_policy_type *blkiop) { }
>
> +static inline char *blkg_path(struct blkio_group *blkg) { return NULL; }
> +
>  #endif
>
>  #define BLKIO_WEIGHT_MIN       100
> @@ -165,10 +170,6 @@ static inline void blkio_policy_unregister(struct blkio_policy_type *blkiop) { }
>  #define BLKIO_WEIGHT_DEFAULT   500
>
>  #ifdef CONFIG_DEBUG_BLK_CGROUP
> -static inline char *blkg_path(struct blkio_group *blkg)
> -{
> -       return blkg->path;
> -}
>  void blkiocg_update_avg_queue_size_stats(struct blkio_group *blkg);
>  void blkiocg_update_dequeue_stats(struct blkio_group *blkg,
>                                unsigned long dequeue);
> @@ -197,7 +198,6 @@ BLKG_FLAG_FNS(idling)
>  BLKG_FLAG_FNS(empty)
>  #undef BLKG_FLAG_FNS
>  #else
> -static inline char *blkg_path(struct blkio_group *blkg) { return NULL; }
>  static inline void blkiocg_update_avg_queue_size_stats(
>                                                struct blkio_group *blkg) {}
>  static inline void blkiocg_update_dequeue_stats(struct blkio_group *blkg,
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index d5927b5..5fa0e98 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -345,7 +345,7 @@ CFQ_CFQQ_FNS(deep);
>  CFQ_CFQQ_FNS(wait_busy);
>  #undef CFQ_CFQQ_FNS
>
> -#ifdef CONFIG_DEBUG_CFQ_IOSCHED
> +#ifdef CONFIG_CFQ_GROUP_IOSCHED
>  #define cfq_log_cfqq(cfqd, cfqq, fmt, args...) \
>        blk_add_trace_msg((cfqd)->queue, "cfq%d%c %s " fmt, (cfqq)->pid, \
>                        cfq_cfqq_sync((cfqq)) ? 'S' : 'A', \
> diff --git a/init/Kconfig b/init/Kconfig
> index eb77e8c..087c14f 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -612,6 +612,33 @@ config RT_GROUP_SCHED
>
>  endif #CGROUP_SCHED
>
> +config BLK_CGROUP
> +       tristate "Block IO controller"
> +       depends on CGROUPS && BLOCK
> +       default n
> +       ---help---
> +       Generic block IO controller cgroup interface. This is the common
> +       cgroup interface which should be used by various IO controlling
> +       policies.
> +
> +       Currently, CFQ IO scheduler uses it to recognize task groups and
> +       control disk bandwidth allocation (proportional time slice allocation)
> +       to such task groups.
> +
> +       This option only enables generic Block IO controller infrastructure.
> +       One needs to also enable actual IO controlling logic in CFQ for it
> +       to take effect. (CONFIG_CFQ_GROUP_IOSCHED=y).
> +
> +       See Documentation/cgroups/blkio-controller.txt for more information.
> +
> +config DEBUG_BLK_CGROUP
> +       bool "Enable Block IO controller debugging"
> +       depends on BLK_CGROUP
> +       default n
> +       ---help---
> +       Enable some debugging help. Currently it exports additional stat
> +       files in a cgroup which can be useful for debugging.
> +
>  endif # CGROUPS
>
>  config MM_OWNER
> --
> 1.6.2.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ