On Mon, Oct 21, 2019 at 03:28:18PM +0200, Jan Kara wrote: > On Mon 21-10-19 20:17:46, Matthew Bobrowski wrote: > > This patch is effectively addressed what Dave Chinner had found and > > fixed within this commit: 8a23414ee345. Justification for needing this > > modification has been provided below: > > > > When doing a direct IO that spans the current EOF, and there are > > written blocks beyond EOF that extend beyond the current write, the > > only metadata update that needs to be done is a file size extension. > > > > However, we don't mark such iomaps as IOMAP_F_DIRTY to indicate that > > there is IO completion metadata updates required, and hence we may > > fail to correctly sync file size extensions made in IO completion when > > O_DSYNC writes are being used and the hardware supports FUA. > > > > Hence when setting IOMAP_F_DIRTY, we need to also take into account > > whether the iomap spans the current EOF. If it does, then we need to > > mark it dirty so that IO completion will call generic_write_sync() to > > flush the inode size update to stable storage correctly. > > > > Signed-off-by: Matthew Bobrowski <mbobrowski@xxxxxxxxxxxxxx> > > Looks good to me. You could possibly also comment in the changelog that > currently, this change doesn't have user visible impact for ext4 as none of > current users of ext4_iomap_begin() that extend files depend of > IOMAP_F_DIRTY. Sure, I will add this. > Also this patch would make slightly more sense to be before 1/12 so that > you don't have there those two strange unused arguments. But these are just > small nits. You're right. I will rearrange it in v6 so that this patch comes first. > Feel free to add: > > Reviewed-by: Jan Kara <jack@xxxxxxx> Thanks Jan! --<M>--
