Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753548AbbDQLZK (ORCPT ); Fri, 17 Apr 2015 07:25:10 -0400 Received: from mail-la0-f54.google.com ([209.85.215.54]:33104 "EHLO mail-la0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752488AbbDQLZD (ORCPT ); Fri, 17 Apr 2015 07:25:03 -0400 From: Dmitry Monakhov To: axboe@kernel.dk Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] blk: optimize blk_trace macros for cgroup case PING.. In-Reply-To: <1428787023-30967-1-git-send-email-dmonakhov@openvz.org> References: <1428787023-30967-1-git-send-email-dmonakhov@openvz.org> User-Agent: Notmuch/0.18.1 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Fri, 17 Apr 2015 14:23:00 +0300 Message-ID: <877ftbhucr.fsf@openvz.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5403 Lines: 143 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable 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 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=3D1 > #echo cfq > /sys/block/nullb0/queue/scheduler > #fio --ioengine=3Dlibaio --direct=3D1 --group_reporting --filename=3D/dev= /nullb0 \ > --rw=3Dwrite --iodepth=3D64 --bs=3D4k --numjobs=3D8 --size=3D4G > | | baseline | w/ patch | gain | > | WR iops | 161483 | 189989 | +17% | > | stddev | 14.36 | 13.19 | | > > Signed-off-by: Dmitry Monakhov > --- > 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_s= ervice_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 =3D sq_to_tg((sq)); \ > struct throtl_data *__td =3D 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) >=20=20 > 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) > } >=20=20 > #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)) =3D=3D 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)) =3D=3D SYNC_NOIDLE_WORKLOAD ? 'N' : ' ', \ > + __pbuf, ##args); \ > + } \ > } while (0) >=20=20 > #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) >=20=20 > 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; > }; >=20=20 > +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, > --=20 > 1.7.1 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBCgAGBQJVMO0UAAoJELhyPTmIL6kB1fUIAKQ1t+0DzQfDEQu2FrDtUCF3 FLvqI5xXS8/ZIC6VFXI8f9pxcngNY1AbN1DTPuEEd/CzdKl9K8aUJEINO6X2GE3N WVhKYG7CZKDTOd0orqd3ovtx7wXCFOUbSiC66Ffkpz+h/heWPknCGZ8l3q0qAj8a Nuykxv4MgXvjoh4Br9FMEizQtVuI36Zd6AEYeRWW77aKE5C+r2lwxvmNrIvWWlME XflHv2l3258boVDKzPYsAmBMWstxZWnkB1c6xMlTFt0P0RwCqeGwgaj/3ozvS4ja VxTPuvylG7g8blIoAPkvWPzmHMIAGrtQsiCo7kA5ZPTfVApUz7YrrCpH/64rnzw= =mV1q -----END PGP SIGNATURE----- --=-=-=-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/