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