On Thu 29-08-19 07:54:21, Matthew Bobrowski wrote:
> On Wed, Aug 28, 2019 at 09:59:14PM +0200, Jan Kara wrote:
> > > @@ -257,6 +308,13 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > > goto out;
> > >
> > > ret = dax_iomap_rw(iocb, from, &ext4_iomap_ops);
> > > +
> > > + if (ret > 0 && iocb->ki_pos > i_size_read(inode)) {
> > > + err = ext4_handle_inode_extension(inode, iocb->ki_pos,
> > > + iov_iter_count(from));
> > > + if (err)
> > > + ret = err;
> > > + }
>
> I noticed that within ext4_dax_write_iter() we're not accommodating
> for error cases. Subsequently, there's no clean up code that goes with
> that. So, for example, in the case where we've added the inode onto
> the orphan list as a result of an extension and we bump into any error
> i.e. -ENOSPC, we'll be left with inconsistencies. Perhaps it might be
> worthwhile to introduce a helper here
> i.e. ext4_dax_handle_failed_write(), which would be the path taken to
> perform any necessary clean up routines in the case of a failed write?
> I think it'd be better to have this rather than polluting
> ext4_dax_write_iter(). What do you think?
Esentially you'll need the same error-handling code as you have in
ext4_dio_write_end_io(). So probably just factor that out into a common
helper like ext4_handle_failed_inode_extension() and call it from
ext4_dio_write_end_io() and ext4_dax_write_iter().
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
