Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751458AbdGZWSe (ORCPT <rfc822;w@1wt.eu>);
        Wed, 26 Jul 2017 18:18:34 -0400
Received: from mail-qt0-f169.google.com ([209.85.216.169]:36600 "EHLO
        mail-qt0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751103AbdGZWSd (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 26 Jul 2017 18:18:33 -0400
Message-ID: <1501107510.15159.4.camel@redhat.com>
Subject: Re: [PATCH v2 2/4] mm: add file_fdatawait_range and
 file_write_and_wait
From: Jeff Layton <jlayton@redhat.com>
To: Matthew Wilcox <willy@infradead.org>, Jeff Layton <jlayton@kernel.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
        "J . Bruce Fields" <bfields@fieldses.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, Bob Peterson <rpeterso@redhat.com>,
        Steven Whitehouse <swhiteho@redhat.com>, cluster-devel@redhat.com
Date: Wed, 26 Jul 2017 18:18:30 -0400
In-Reply-To: <20170726191305.GC15980@bombadil.infradead.org>
References: <20170726175538.13885-1-jlayton@kernel.org>
         <20170726175538.13885-3-jlayton@kernel.org>
         <20170726191305.GC15980@bombadil.infradead.org>
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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1285
Lines: 53

On Wed, 2017-07-26 at 12:13 -0700, Matthew Wilcox wrote:
> On Wed, Jul 26, 2017 at 01:55:36PM -0400, 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)) {
> 
> Since patch 1 exists, shouldn't this use the new helper?
> 

<facepalm>

yes, will fix


> > +		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);
> > +		}
> > +	}
> > +	err2 = file_check_and_advance_wb_err(file);
> > +	if (!err)
> > +		err = err2;
> > +	return err;
> 
> Would this be clearer written as:
> 
> 	if (err)
> 		return err;
> 	return err2;
> 
> or even ...
> 
> 	return err ? err : err2;
> 

Meh -- I like it the way I have it. If we don't have an error already,
then just take the one from the check and advance.

That said, I don't have a terribly strong preference here, so if anyone
does, then I can be easily persuaded.

-- 
-- 
Jeff Layton <jlayton@redhat.com>