From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BAC3FC433E1 for ; Thu, 27 Aug 2020 08:35:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9092520707 for ; Thu, 27 Aug 2020 08:35:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="QW3Qe/Nl" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727986AbgH0Ifz (ORCPT ); Thu, 27 Aug 2020 04:35:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58998 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726826AbgH0Ify (ORCPT ); Thu, 27 Aug 2020 04:35:54 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F180BC061264; Thu, 27 Aug 2020 01:35:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=1MBOvjB+9m0M7w4PQdZ2NOCfksOiohzbEbMFMspbqoc=; b=QW3Qe/NlFGPNasZPafhuJ7jcdn 8OHUxL+KZ3L/ZK90HAdySeBpBxqH+oQMwUYY2ybWhjl6ySP4LXESV9Ky+fFPP+kBPufC5QuRsX5Op 5WS0EzX3egRn8JFShjXtd9q1PF9BfDdMq/HKI5ZtUVspNwVs6O38Dw2b/zGCBgxyOE9rLkxAGk0Wk gUt+9/HB0x5wLyg0ZnvHLDuG58nqnVFFeVCqcdILhDiVP/ndQEh4jmuyz/h+vUhmO+F4X6mjFKca/ JuBA8ecVSCcH+nbv/AtSxmFZgBKgl/bK58IBvsF1yDv77smtLkq3766whaoQMmr9pLSG/3+LBnYfW JSQ2cCyQ==; Received: from hch by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1kBDNw-0003ns-Gs; Thu, 27 Aug 2020 08:35:52 +0000 Date: Thu, 27 Aug 2020 09:35:52 +0100 From: Christoph Hellwig To: "Matthew Wilcox (Oracle)" Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, "Darrick J . Wong" , linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/9] iomap: Convert read_count to byte count Message-ID: <20200827083552.GC11067@infradead.org> References: <20200824145511.10500-1-willy@infradead.org> <20200824145511.10500-7-willy@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200824145511.10500-7-willy@infradead.org> X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org > @@ -269,20 +263,17 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > if (ctx->bio && bio_end_sector(ctx->bio) == sector) > is_contig = true; > > - > /* > - * If we start a new segment we need to increase the read count, and we > - * need to do so before submitting any previous full bio to make sure > - * that we don't prematurely unlock the page. > + * We need to increase the read count before submitting any > + * previous bio to make sure that we don't prematurely unlock > + * the page. > */ > if (iop) > - atomic_inc(&iop->read_count); > + atomic_add(plen, &iop->read_count); > + > + if (is_contig && > + __bio_try_merge_page(ctx->bio, page, plen, poff, &same_page)) > + goto done; > > if (!ctx->bio || !is_contig || bio_full(ctx->bio, plen)) { I think we can simplify this a bit by reordering the checks: if (iop) atomic_add(plen, &iop->read_count); if (ctx->bio && bio_end_sector(ctx->bio) == sector) if (__bio_try_merge_page(ctx->bio, page, plen, poff, &same_page)) goto done; is_contig = true; } if (!is_contig || bio_full(ctx->bio, plen)) { // the existing case to allocate a new bio } Also I think read_count should be renamed to be more descriptive, something like read_bytes_pending? Same for the write side.