From patchwork Mon Jul 31 11:27:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 9871615 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0032E602F0 for ; Mon, 31 Jul 2017 11:27:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E89EF2858F for ; Mon, 31 Jul 2017 11:27:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DD59E28593; Mon, 31 Jul 2017 11:27:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.4 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 75E1F2858F for ; Mon, 31 Jul 2017 11:27:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752091AbdGaL1G (ORCPT ); Mon, 31 Jul 2017 07:27:06 -0400 Received: from mail-qk0-f173.google.com ([209.85.220.173]:35477 "EHLO mail-qk0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751199AbdGaL1E (ORCPT ); Mon, 31 Jul 2017 07:27:04 -0400 Received: by mail-qk0-f173.google.com with SMTP id d145so139673392qkc.2 for ; Mon, 31 Jul 2017 04:27:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=+aucBCmtigNOkgbG7fchY9/sh78xis11dk19OagU3BE=; b=f3gKL84eDCHSNNAsi04Yln7YF58LpXBAFdCRU3oRz8feAXBEeuR8E+tp3SXRcLwmdu bb0vQotBpsOnAUwgebI4BBp6XsQi7dVmMGTA6jFyzun2iFF0AtPgggiLJ5UfZ6TWNQpc vaE9EikNPFaNzJbemf4uFLhuVXRnEpyo1DUIJLdCfuDpNZttgFniEk5QQPzNOH4JlVEo e8kOxVxm5hAby7U2hsnIXTzpZ3sWmD5fnYWxPnQLF8j5gKO7fvbIzHt3V0u4do5v6Tyh bFnKgH9lSQ8JiRLU+jD7x72UjS9jBu5Mhfg8CVILkBJLSQQHFOEYhswBJU0IyBZQUAR2 NTow== X-Gm-Message-State: AIVw113zAxG966lOIWsdeX7WwpsjilbCy9eZ4je+hGH5XEEppTbcxnV4 p+t0XBgZbIRSXmx3 X-Received: by 10.55.207.85 with SMTP id e82mr19111420qkj.235.1501500423377; Mon, 31 Jul 2017 04:27:03 -0700 (PDT) Received: from tleilax.poochiereds.net (cpe-45-37-196-243.nc.res.rr.com. [45.37.196.243]) by smtp.gmail.com with ESMTPSA id y36sm5636240qtc.38.2017.07.31.04.27.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 31 Jul 2017 04:27:02 -0700 (PDT) Message-ID: <1501500421.4663.4.camel@redhat.com> Subject: Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait From: Jeff Layton To: Jan Kara , Marcelo Tosatti Cc: Alexander Viro , "J . Bruce Fields" , Andrew Morton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Matthew Wilcox , Bob Peterson , Steven Whitehouse , cluster-devel@redhat.com Date: Mon, 31 Jul 2017 07:27:01 -0400 In-Reply-To: <1501159710.6279.1.camel@redhat.com> References: <20170726175538.13885-1-jlayton@kernel.org> <20170726175538.13885-3-jlayton@kernel.org> <20170727084914.GC21100@quack2.suse.cz> <1501159710.6279.1.camel@redhat.com> X-Mailer: Evolution 3.24.4 (3.24.4-1.fc26) Mime-Version: 1.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 2017-07-27 at 08:48 -0400, Jeff Layton wrote: > On Thu, 2017-07-27 at 10:49 +0200, Jan Kara wrote: > > On Wed 26-07-17 13:55:36, Jeff Layton wrote: > > > +int file_write_and_wait(struct file *file) > > > +{ > > > + int err = 0, err2; > > > + struct address_space *mapping = file->f_mapping; > > > + > > > + if ((!dax_mapping(mapping) && mapping->nrpages) || > > > + (dax_mapping(mapping) && mapping->nrexceptional)) { > > > + err = filemap_fdatawrite(mapping); > > > + /* See comment of filemap_write_and_wait() */ > > > + if (err != -EIO) { > > > + loff_t i_size = i_size_read(mapping->host); > > > + > > > + if (i_size != 0) > > > + __filemap_fdatawait_range(mapping, 0, > > > + i_size - 1); > > > + } > > > + } > > > > Err, what's the i_size check doing here? I'd just pass ~0 as the end of the > > range and ignore i_size. It is much easier than trying to wrap your head > > around possible races with file operations modifying i_size. > > > > Honza > > I'm basically emulating _exactly_ what filemap_write_and_wait does here, > as I'm leery of making subtle behavior changes in the actual writeback > behavior. For example: > > -----------------8<---------------- > static inline int __filemap_fdatawrite(struct address_space *mapping, > int sync_mode) > { > return __filemap_fdatawrite_range(mapping, 0, LLONG_MAX, sync_mode); > } > > int filemap_fdatawrite(struct address_space *mapping) > { > return __filemap_fdatawrite(mapping, WB_SYNC_ALL); > } > EXPORT_SYMBOL(filemap_fdatawrite); > -----------------8<---------------- > > ...which then sets up the wbc with the right ranges and sync mode and > kicks off writepages. But then, it does the i_size_read to figure out > what range it should wait on (with the shortcut for the size == 0 case). > > My assumption was that it was intentionally designed that way, but I'm > guessing from your comments that it wasn't? If so, then we can turn > file_write_and_wait a static inline wrapper around > file_write_and_wait_range. FWIW, I did a bit of archaeology in the linux-history tree and found this patch from Marcelo in 2004. Is this optimization still helpful? If not, then that does simplify the code a bit. -------------------8<-------------------- [PATCH] small wait_on_page_writeback_range() optimization filemap_fdatawait() calls wait_on_page_writeback_range() with -1 as "end" parameter. This is not needed since we know the EOF from the inode. Use that instead. Signed-off-by: Marcelo Tosatti Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/filemap.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mm/filemap.c b/mm/filemap.c index 78e18b7639b6..55fb7b4141e4 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -287,7 +287,13 @@ EXPORT_SYMBOL(sync_page_range); */ int filemap_fdatawait(struct address_space *mapping) { - return wait_on_page_writeback_range(mapping, 0, -1); + loff_t i_size = i_size_read(mapping->host); + + if (i_size == 0) + return 0; + + return wait_on_page_writeback_range(mapping, 0, + (i_size - 1) >> PAGE_CACHE_SHIFT); } EXPORT_SYMBOL(filemap_fdatawait);