From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757972AbdKORTh (ORCPT ); Wed, 15 Nov 2017 12:19:37 -0500 Received: from mail-io0-f196.google.com ([209.85.223.196]:37124 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757558AbdKORT2 (ORCPT ); Wed, 15 Nov 2017 12:19:28 -0500 X-Google-Smtp-Source: AGs4zMagXMtdxVR65CwUoDZNXHk5c58pGNSg+dPyadu4igLO3PVa4hkOZUYETLuSVgVqFTHYXA6zPg== Subject: Re: [PATCH 6/7] blkcg: account requests instead of bios for request based request_queues To: Shaohua Li , Tejun Heo Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com, lizefan@huawei.com, hannes@cmpxchg.org, cgroups@vger.kernel.org, guro@fb.com References: <20171112222613.3613362-1-tj@kernel.org> <20171112222613.3613362-7-tj@kernel.org> <20171114232355.vjxlzfbqbqj5ihq4@kernel.org> From: Jens Axboe Message-ID: <9a2ddc6a-d618-a896-290c-254ffeb5e9d6@kernel.dk> Date: Wed, 15 Nov 2017 10:19:26 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171114232355.vjxlzfbqbqj5ihq4@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/14/2017 04:23 PM, Shaohua Li wrote: > On Sun, Nov 12, 2017 at 02:26:12PM -0800, Tejun Heo wrote: >> blkcg accounting is currently bio based, which is silly for request >> based request_queues. This is silly as the number of bios doesn't >> have much to do with the actual number of IOs issued to the underlying >> device (can be significantly higher or lower) and may change depending >> on the implementation details on how the bios are issued (e.g. from >> the recent split-bios-while-issuing change). >> >> request based request_queues have QUEUE_FLAG_IO_STAT set by default >> which controls the gendisk accounting. Do cgroup accounting for those >> request_queues together with gendisk accounting on request completion. >> >> This makes cgroup accounting consistent with gendisk accounting and >> what's happening on the system. >> >> Signed-off-by: Tejun Heo >> --- >> block/blk-core.c | 3 +++ >> include/linux/blk-cgroup.h | 18 +++++++++++++++++- >> 2 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/block/blk-core.c b/block/blk-core.c >> index 048be4a..ad23b96 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -2429,6 +2429,7 @@ void blk_account_io_completion(struct request *req, unsigned int bytes) >> cpu = part_stat_lock(); >> part = req->part; >> part_stat_add(cpu, part, sectors[rw], bytes >> 9); >> + blkcg_account_io_completion(req, bytes); >> part_stat_unlock(); >> } >> } >> @@ -2454,6 +2455,8 @@ void blk_account_io_done(struct request *req) >> part_round_stats(req->q, cpu, part); >> part_dec_in_flight(req->q, part, rw); >> >> + blkcg_account_io_done(req); >> + >> hd_struct_put(part); >> part_stat_unlock(); >> } >> diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h >> index 96eed0f..f2f9691 100644 >> --- a/include/linux/blk-cgroup.h >> +++ b/include/linux/blk-cgroup.h >> @@ -715,7 +715,8 @@ static inline bool blkcg_bio_issue_check(struct request_queue *q, >> >> throtl = blk_throtl_bio(q, blkg, bio); >> >> - if (!throtl) { >> + /* if @q does io stat, blkcg stats are updated together with them */ >> + if (!blk_queue_io_stat(q) && !throtl) { > > Reviewed-by: Shaohua Li > > One nitpick, can we use q->request_fn to determine request based queue? I think > that is more reliable and usual way for this. That won't work for mq - but there is a helper for this, queue_is_rq_based(). -- Jens Axboe