From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752301AbcEKQyw (ORCPT ); Wed, 11 May 2016 12:54:52 -0400 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:36834 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751899AbcEKQyv (ORCPT ); Wed, 11 May 2016 12:54:51 -0400 Date: Wed, 11 May 2016 09:54:40 -0700 From: Shaohua Li To: Simon Guo CC: , , Steven Rostedt , , Subject: Re: [PATCH 3/3] blk-throttle: don't parse cgroup path if trace isn't enabled Message-ID: <20160511165439.GA96558@devbig084.prn1.facebook.com> References: <2c3ee532132f8925dd0be1178e8dee0bba6b448c.1462838969.git.shli@fb.com> <20160510054613.GA12366@simonLocalRHEL7.x64> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160510054613.GA12366@simonLocalRHEL7.x64> User-Agent: Mutt/1.5.20 (2009-12-10) X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-05-11_05:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 10, 2016 at 01:46:13PM +0800, Simon Guo wrote: > Hi, > On Mon, May 09, 2016 at 05:22:15PM -0700, Shaohua Li wrote: > > if trace isn't enabled, parsing cgroup path just wastes cpu > > > > Signed-off-by: Shaohua Li > > --- > > block/blk-throttle.c | 5 ++--- > > include/linux/blktrace_api.h | 9 +++++++++ > > 2 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > > index 2149a1d..47a3e54 100644 > > --- a/block/blk-throttle.c > > +++ b/block/blk-throttle.c > > @@ -211,15 +211,14 @@ 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 (likely(!blk_trace_note_message_enabled(__td->queue))) \ > > + break; \ > > if ((__tg)) { \ > > char __pbuf[128]; \ > > \ > > diff --git a/include/linux/blktrace_api.h b/include/linux/blktrace_api.h > > index afc1343..0f3172b 100644 > > --- a/include/linux/blktrace_api.h > > +++ b/include/linux/blktrace_api.h > > @@ -57,6 +57,14 @@ void __trace_note_message(struct blk_trace *, const char *fmt, ...); > > } while (0) > > #define BLK_TN_MAX_MSG 128 > > > > +static inline bool blk_trace_note_message_enabled(struct request_queue *q) > > +{ > > + struct blk_trace *bt = q->blk_trace; > > + if (likely(!bt)) > > + return false; > > + return bt->act_mask & BLK_TC_NOTIFY; > > +} > Is there any reason to skip following condition? > if (unlikely(bt->trace_state != Blktrace_running && > !blk_tracer_enabled)) It's marked unlikely and to add the check we must export blk_tracer_enabled, so I ignored it. Though I don't know how unlikely it is. Thanks, Shaohua