On Wed 09-10-19 21:18:50, Matthew Bobrowski wrote:
> > Just small nits below:
> >
> > > +static int ext4_handle_inode_extension(struct inode *inode, loff_t offset,
> > > + ssize_t written, size_t count)
> > > +{
> > > + int ret = 0;
> >
> > I think both the function and callsites may be slightly simpler if you let
> > the function return 'written' or error (not 0 or error). But I'll leave
> > that decision upto you.
>
> Hm, don't we actually need to return 0 for success cases so that
> iomap_dio_complete() behaves correctly i.e. increments iocb->ki_pos,
> etc?
Correct, iomap_dio_complete() expects 0 on success. So if we keep calling
ext4_handle_inode_extension() from ->end_io handler, we'd need some
specialcasing there and I agree that changing ext4_handle_inode_extension()
return convention isn't then very beneficial. If we stop calling
ext4_handle_inode_extension() from ->end_io handler (patch 8/8 discussion
pending), then the change would be a clear win.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
