Messages in this thread | | | From | Dmitry Monakhov <> | Subject | Re: [PATCH] blk: optimize blk_trace macros for cgroup case PING.. | Date | Fri, 17 Apr 2015 14:23:00 +0300 |
| |
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]
| |