Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934696AbbEMPWx (ORCPT ); Wed, 13 May 2015 11:22:53 -0400 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:26199 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965146AbbEMPWu (ORCPT ); Wed, 13 May 2015 11:22:50 -0400 Date: Wed, 13 May 2015 08:22:06 -0700 From: Shaohua Li To: Jens Axboe CC: Jeff Moyer , Subject: Re: [PATCH] blk: don't account discard request size Message-ID: <20150513152159.GA3046840@devbig257.prn2.facebook.com> References: <10cba6675b3e93f28b9c1c7a21e4b93b923e9531.1431467011.git.shli@fb.com> <55535D9C.3050706@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <55535D9C.3050706@fb.com> User-Agent: Mutt/1.5.20 (2009-12-10) X-Originating-IP: [192.168.52.13] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-05-13_05:2015-05-13,2015-05-13,1970-01-01 signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2138 Lines: 54 On Wed, May 13, 2015 at 10:20:12AM -0400, Jens Axboe wrote: > On 05/13/2015 09:10 AM, Jeff Moyer wrote: > >Shaohua Li writes: > > > >>In a workload with discard request, the IO throughput is generally much > >>higher than expected. This is quite confusing checking iostat. Discard > >>request doesn't really write data to drive, so don't account it. > >> > >>Signed-off-by: Shaohua Li > >>--- > >> block/blk-core.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >>diff --git a/block/blk-core.c b/block/blk-core.c > >>index fd154b9..0128d18 100644 > >>--- a/block/blk-core.c > >>+++ b/block/blk-core.c > >>@@ -2138,7 +2138,11 @@ EXPORT_SYMBOL_GPL(blk_rq_err_bytes); > >> > >> void blk_account_io_completion(struct request *req, unsigned int bytes) > >> { > >>- if (blk_do_io_stat(req)) { > >>+ /* > >>+ * discard request doesn't really write @bytes to drive, > >>+ * doesn't account it > >>+ **/ > >>+ if (blk_do_io_stat(req) && !(req->cmd_flags & REQ_DISCARD)) { > >> const int rw = rq_data_dir(req); > >> struct hd_struct *part; > >> int cpu; > > > >I think you want to modify __get_request to not set REQ_IO_STAT for > >discard requests. This patch will still account the start of I/O, which > >means in_flight will be off. > > That would be better. But I'm still not sure we want to turn off > accounting for discards. For the mixed write/discard cases it's > definitely confusing. The better option would be to account it as a > discard and not a write. Preferably in a way that would not break > existing tools, but so that they could get updated to support it. It's intentional discard IO start gets accounted, so tools will show there is IO. I'm not sure if this is better though. Adding separate columns for discard (maybe flush too) is definitely preferred. Is breaking existing tools really ok? Thanks, Shaohua -- 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/