Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751544AbdG0Mse (ORCPT ); Thu, 27 Jul 2017 08:48:34 -0400 Received: from mail-qt0-f181.google.com ([209.85.216.181]:33616 "EHLO mail-qt0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751122AbdG0Msc (ORCPT ); Thu, 27 Jul 2017 08:48:32 -0400 Message-ID: <1501159710.6279.1.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 , Jeff Layton 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: Thu, 27 Jul 2017 08:48:30 -0400 In-Reply-To: <20170727084914.GC21100@quack2.suse.cz> References: <20170726175538.13885-1-jlayton@kernel.org> <20170726175538.13885-3-jlayton@kernel.org> <20170727084914.GC21100@quack2.suse.cz> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.24.4 (3.24.4-1.fc26) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1928 Lines: 54 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. -- Jeff Layton