From patchwork Tue May 10 17:38:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Moyer X-Patchwork-Id: 9060531 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A4168BF29F for ; Tue, 10 May 2016 17:38:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DD6CD20123 for ; Tue, 10 May 2016 17:38:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F4239200F3 for ; Tue, 10 May 2016 17:38:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751525AbcEJRiI (ORCPT ); Tue, 10 May 2016 13:38:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60776 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751041AbcEJRiH (ORCPT ); Tue, 10 May 2016 13:38:07 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7807C3B725; Tue, 10 May 2016 17:38:06 +0000 (UTC) Received: from segfault.boston.devel.redhat.com (segfault.boston.devel.redhat.com [10.19.60.26]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4AHc5d6022134 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 10 May 2016 13:38:06 -0400 From: Jeff Moyer To: Eryu Guan Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org Subject: Re: [PATCH v2 1/2] direct-io: cleanup get_more_blocks() References: <1462645910-23290-1-git-send-email-guaneryu@gmail.com> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 X-PCLoadLetter: What the f**k does that mean? Date: Tue, 10 May 2016 13:38:05 -0400 In-Reply-To: <1462645910-23290-1-git-send-email-guaneryu@gmail.com> (Eryu Guan's message of "Sun, 8 May 2016 02:31:49 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 10 May 2016 17:38:06 +0000 (UTC) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-9.0 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 Eryu Guan writes: > Save one level of indention by returning error early. > > Introduce some local variables to make the code easier to read a bit, > and do preparation for next patch. > > Signed-off-by: Eryu Guan Hi, Eryu, I don't think you have a full appreciation of the amount of optimization that goes into this code. I don't see anything wrong with what you've done, but I also don't want to introduce all these local variables and change a branch in order to find out several months down the line that we introduced some TPC-C regression of .5%. Look, I think this is all you need for the full fix: Can you just test that? Thanks, Jeff --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/direct-io.c b/fs/direct-io.c index 4720377..f66754e 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -639,8 +639,7 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio, */ create = dio->rw & WRITE; if (dio->flags & DIO_SKIP_HOLES) { - if (sdio->block_in_file < (i_size_read(dio->inode) >> - sdio->blkbits)) + if (fs_startblk < fs_count) create = 0; }