lkml.org 
[lkml]   [2015]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] blk: optimize blk_trace macros for cgroup case PING..
Date

Hi Jens, do you have any objections against this patch?
Please let me know if you have any and I'll fix it.

Dmitry Monakhov <dmonakhov@openvz.org> writes:
> Usually blk_trace is not active, but cfq_log_xxx macros unconditionally
> prepare cgroup path via blkg_path() which is suboptimal. This provoke
> significant performance overhead
>
> ## Test
> #modprobe null_blk queue_mode=1
> #echo cfq > /sys/block/nullb0/queue/scheduler
> #fio --ioengine=libaio --direct=1 --group_reporting --filename=/dev/nullb0 \
> --rw=write --iodepth=64 --bs=4k --numjobs=8 --size=4G
> | | baseline | w/ patch | gain |
> | WR iops | 161483 | 189989 | +17% |
> | stddev | 14.36 | 13.19 | |
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> block/blk-throttle.c | 17 ++++++++---------
> block/cfq-iosched.c | 22 +++++++++++++---------
> include/linux/blktrace_api.h | 5 +++++
> 3 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 5b9c6d5..6ce9750 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -238,21 +238,20 @@ static struct throtl_data *sq_to_td(struct throtl_service_queue *sq)
> * The messages are prefixed with "throtl BLKG_NAME" if @sq belongs to a
> * throtl_grp; otherwise, just "throtl".
> *
> - * TODO: this should be made a function and name formatting should happen
> - * after testing whether blktrace is enabled.
> */
> #define throtl_log(sq, fmt, args...) do { \
> struct throtl_grp *__tg = sq_to_tg((sq)); \
> struct throtl_data *__td = sq_to_td((sq)); \
> \
> - (void)__td; \
> - if ((__tg)) { \
> - char __pbuf[128]; \
> + if (is_blk_trace_active(__td->queue)) { \
> + if ((__tg)) { \
> + char __pbuf[128]; \
> \
> - blkg_path(tg_to_blkg(__tg), __pbuf, sizeof(__pbuf)); \
> - blk_add_trace_msg(__td->queue, "throtl %s " fmt, __pbuf, ##args); \
> - } else { \
> - blk_add_trace_msg(__td->queue, "throtl " fmt, ##args); \
> + blkg_path(tg_to_blkg(__tg), __pbuf, sizeof(__pbuf)); \
> + blk_add_trace_msg(__td->queue, "throtl %s " fmt, __pbuf, ##args); \
> + } else { \
> + blk_add_trace_msg(__td->queue, "throtl " fmt, ##args); \
> + } \
> } \
> } while (0)
>
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 5da8e6e..7b90897 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -625,20 +625,24 @@ static inline void cfqg_put(struct cfq_group *cfqg)
> }
>
> #define cfq_log_cfqq(cfqd, cfqq, fmt, args...) do { \
> - char __pbuf[128]; \
> + if (is_blk_trace_active((cfqd)->queue)) { \
> + char __pbuf[128]; \
> \
> - blkg_path(cfqg_to_blkg((cfqq)->cfqg), __pbuf, sizeof(__pbuf)); \
> - blk_add_trace_msg((cfqd)->queue, "cfq%d%c%c %s " fmt, (cfqq)->pid, \
> - cfq_cfqq_sync((cfqq)) ? 'S' : 'A', \
> - cfqq_type((cfqq)) == SYNC_NOIDLE_WORKLOAD ? 'N' : ' ',\
> - __pbuf, ##args); \
> + blkg_path(cfqg_to_blkg((cfqq)->cfqg), __pbuf, sizeof(__pbuf)); \
> + blk_add_trace_msg((cfqd)->queue, "cfq%d%c%c %s " fmt, (cfqq)->pid, \
> + cfq_cfqq_sync((cfqq)) ? 'S' : 'A', \
> + cfqq_type((cfqq)) == SYNC_NOIDLE_WORKLOAD ? 'N' : ' ', \
> + __pbuf, ##args); \
> + } \
> } while (0)
>
> #define cfq_log_cfqg(cfqd, cfqg, fmt, args...) do { \
> - char __pbuf[128]; \
> + if (is_blk_trace_active((cfqd)->queue)) { \
> + char __pbuf[128]; \
> \
> - blkg_path(cfqg_to_blkg(cfqg), __pbuf, sizeof(__pbuf)); \
> - blk_add_trace_msg((cfqd)->queue, "%s " fmt, __pbuf, ##args); \
> + blkg_path(cfqg_to_blkg(cfqg), __pbuf, sizeof(__pbuf)); \
> + blk_add_trace_msg((cfqd)->queue, "%s " fmt, __pbuf, ##args); \
> + } \
> } while (0)
>
> static inline void cfqg_stats_update_io_add(struct cfq_group *cfqg,
> diff --git a/include/linux/blktrace_api.h b/include/linux/blktrace_api.h
> index afc1343..2981c9e 100644
> --- a/include/linux/blktrace_api.h
> +++ b/include/linux/blktrace_api.h
> @@ -28,6 +28,11 @@ struct blk_trace {
> atomic_t dropped;
> };
>
> +static inline bool is_blk_trace_active(struct request_queue *q)
> +{
> + return q->blk_trace;
> +}
> +
> extern int blk_trace_ioctl(struct block_device *, unsigned, char __user *);
> extern void blk_trace_shutdown(struct request_queue *);
> extern int do_blk_trace_setup(struct request_queue *q, char *name,
> --
> 1.7.1
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2015-04-17 13:41    [W:0.073 / U:1.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site