Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait

[Jeff Layton]

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@xxxxxxxxxx>