From patchwork Fri Sep 13 04:46:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junichi Nomura X-Patchwork-Id: 2881411 Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5DE0E9F1C0 for ; Fri, 13 Sep 2013 04:52:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6ACD420413 for ; Fri, 13 Sep 2013 04:52:53 +0000 (UTC) Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com [209.132.183.25]) by mail.kernel.org (Postfix) with ESMTP id 1E9E82040F for ; Fri, 13 Sep 2013 04:52:52 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8D4mHJb028243; Fri, 13 Sep 2013 00:48:19 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8D4lrcq027214 for ; Fri, 13 Sep 2013 00:47:53 -0400 Received: from mx1.redhat.com (ext-mx11.extmail.prod.ext.phx2.redhat.com [10.5.110.16]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8D4lqTr017157; Fri, 13 Sep 2013 00:47:53 -0400 Received: from tyo201.gate.nec.co.jp (TYO201.gate.nec.co.jp [202.32.8.193]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8D4loom023084; Fri, 13 Sep 2013 00:47:50 -0400 Received: from mailgate3.nec.co.jp ([10.7.69.197]) by tyo201.gate.nec.co.jp (8.13.8/8.13.4) with ESMTP id r8D4lmvG007815; Fri, 13 Sep 2013 13:47:48 +0900 (JST) Received: from mailsv.nec.co.jp (imss62.nec.co.jp [10.7.69.157]) by mailgate3.nec.co.jp (8.11.7/3.7W-MAILGATE-NEC) with ESMTP id r8D4ll505477; Fri, 13 Sep 2013 13:47:47 +0900 (JST) Received: from mail01b.kamome.nec.co.jp (mail01b.kamome.nec.co.jp [10.25.43.2]) by mailsv.nec.co.jp (8.13.8/8.13.4) with ESMTP id r8D4llMQ011797; Fri, 13 Sep 2013 13:47:47 +0900 (JST) Received: from shoin.jp.nec.com ([10.26.220.3] [10.26.220.3]) by mail03.kamome.nec.co.jp with ESMTP id BT-MMP-3787; Fri, 13 Sep 2013 13:46:44 +0900 Received: from xzibit.linux.bs1.fc.nec.co.jp ([10.34.125.128] [10.34.125.128]) by mail.jp.nec.com with ESMTP; Fri, 13 Sep 2013 13:46:43 +0900 Message-ID: <523298B3.9050400@ce.jp.nec.com> Date: Fri, 13 Sep 2013 13:46:43 +0900 From: "Jun'ichi Nomura" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8 MIME-Version: 1.0 To: device-mapper development , Mike Snitzer , Mikulas Patocka , Frank Mayhar References: <1379024698-10487-1-git-send-email-snitzer@redhat.com> <1379024698-10487-8-git-send-email-snitzer@redhat.com> <20130912230646.GA31577@redhat.com> <20130912235346.GF31577@redhat.com> In-Reply-To: <20130912235346.GF31577@redhat.com> X-RedHat-Spam-Score: -4.202 (BAYES_00, RCVD_IN_DNSWL_MED, SPF_HELO_PASS, SPF_PASS) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-Scanned-By: MIMEDefang 2.68 on 10.5.110.16 X-loop: dm-devel@redhat.com Subject: Re: [dm-devel] [PATCH 7/7] dm: optimize clone_rq() when track_peak_rq_based_ios is disabled X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 09/13/13 08:53, Mike Snitzer wrote: > On Thu, Sep 12 2013 at 7:30pm -0400, > Mikulas Patocka wrote: >>>>> Make use of static keys to eliminate the relevant branch in clone_rq() >>>>> when dm_mod's 'track_peak_rq_based_ios' paramter is set to 0 (disabled). >>>>> >>>>> Even if 'track_peak_rq_based_ios' is set to 0 it will not take effect >>>>> until the next request-based table reload. Once it is disabled the >>>>> 'peak_rq_based_ios' parameter will be reset to 0. >>>> >>>> This patch changes it so that the value track_peak_rq_based_ios is sampled >>>> only when reloading a table. I think it will be confusing to the user if >>>> he changes the value, but the change doesn't take effect until something >>>> reloads some table. >>> >>> Well we need to be able to notice the change but I do not wish to do so >>> in a performance critical path (which clone_rq is). >> >> I think one condition isn't that bad so there is no need to use static >> keys. > > Considering we're talking about the IO path I'd rather avoid it given > 'track_peak_rq_based_ios' will almost always be disabled. > >> If you want to use static keys, you need to refresh them in some place >> that it called regularly. > > No you don't. I'm assuming if someone enables 'track_peak_rq_based_ios' > they know what they are doing. > > But I'm open to suggestions on a more appropriate hook to be catching > the update to track_peak_rq_based_ios. Other approach might be putting the info on tracepoints. Then the tracepoint framework will take care of the static_key thing. Since 'the number of bios in a request' is a block-layer generic info, you could extend the block layer events instead of having your own. Attached patch adds it to block_rq_remap event. You could do something like this to see requests with many bios: echo 'nr_bios > 32' > /sys/kernel/debug/tracing/events/block/block_rq_remap/filter echo 1 > /sys/kernel/debug/tracing/events/block/block_rq_remap/enable For example, kworker/1:1H-308 [001] d..1 276.242448: block_rq_remap: 8,64 W 1967760 + 512 <- (253,1) 1967760 64 kworker/1:1H-308 [001] d..1 276.242482: block_rq_remap: 8,160 W 1968272 + 512 <- (253,1) 1968272 64 kworker/1:1H-308 [001] d..1 276.242515: block_rq_remap: 65,0 W 1968784 + 512 <- (253,1) 1968784 64 # the last item in line is the number of bios. ("64" in this case) Acked-by: Mike Snitzer --- Jun'ichi Nomura, NEC Corporation -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 2fdb4a4..0e6f765 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -862,6 +862,17 @@ static inline unsigned int blk_rq_get_max_sectors(struct request *rq) return blk_queue_get_max_sectors(q, rq->cmd_flags); } +static inline unsigned int blk_rq_count_bios(struct request *rq) +{ + unsigned int nr_bios = 0; + struct bio *bio; + + __rq_for_each_bio(bio, rq) + nr_bios++; + + return nr_bios; +} + /* * Request issue related functions. */ diff --git a/include/trace/events/block.h b/include/trace/events/block.h index 60ae7c3..4c2301d 100644 --- a/include/trace/events/block.h +++ b/include/trace/events/block.h @@ -618,6 +618,7 @@ TRACE_EVENT(block_rq_remap, __field( unsigned int, nr_sector ) __field( dev_t, old_dev ) __field( sector_t, old_sector ) + __field( unsigned int, nr_bios ) __array( char, rwbs, RWBS_LEN) ), @@ -627,15 +628,16 @@ TRACE_EVENT(block_rq_remap, __entry->nr_sector = blk_rq_sectors(rq); __entry->old_dev = dev; __entry->old_sector = from; + __entry->nr_bios = blk_rq_count_bios(rq); blk_fill_rwbs(__entry->rwbs, rq->cmd_flags, blk_rq_bytes(rq)); ), - TP_printk("%d,%d %s %llu + %u <- (%d,%d) %llu", + TP_printk("%d,%d %s %llu + %u <- (%d,%d) %llu %u", MAJOR(__entry->dev), MINOR(__entry->dev), __entry->rwbs, (unsigned long long)__entry->sector, __entry->nr_sector, MAJOR(__entry->old_dev), MINOR(__entry->old_dev), - (unsigned long long)__entry->old_sector) + (unsigned long long)__entry->old_sector, __entry->nr_bios) ); #endif /* _TRACE_BLOCK_H */