On Thu, Aug 03, 2017 at 02:10:47PM -0400, Jeff Moyer wrote:
> Al, would you mind taking this in through your tree? It's been reviewed
> by myself and Jan in this mail thread.
Still needs more fixing, I think?
Sorry, this is the first time I've seen this patch....
> > diff --git a/fs/iomap.c b/fs/iomap.c
> > index 1732228..144512e 100644
> > --- a/fs/iomap.c
> > +++ b/fs/iomap.c
> > @@ -713,8 +713,15 @@ struct iomap_dio {
> > static ssize_t iomap_dio_complete(struct iomap_dio *dio)
> > {
> > struct kiocb *iocb = dio->iocb;
> > + struct inode *inode = file_inode(iocb->ki_filp);
> > ssize_t ret;
> >
> > + if (!dio->error &&
> > + (dio->flags & IOMAP_DIO_WRITE) && inode->i_mapping->nrpages)
> > + invalidate_inode_pages2_range(inode->i_mapping,
> > + iocb->ki_pos >> PAGE_SHIFT,
> > + (iocb->ki_pos + dio->size - 1) >> PAGE_SHIFT);
> > +
> > if (dio->end_io) {
> > ret = dio->end_io(iocb,
> > dio->error ? dio->error : dio->size,
>
This invalidation is already run in iomap_dio_rw() for the sync IO
case directly after the call to iomap_dio_complete(). It also has a
comment to explain exactly why the the invalidation is needed, and
it issues a warning to dmesg if the invalidation fails to indicate
the reason why the user is reporting data corruption to us. i.e.:
.....
ret = iomap_dio_complete(dio);
/*
* Try again to invalidate clean pages which might have been cached by
* non-direct readahead, or faulted in by get_user_pages() if the source
* of the write was an mmap'ed region of the file we're writing. Either
* one is a pretty crazy thing to do, so we don't support it 100%. If
* this invalidation fails, tough, the write still worked...
*/
if (iov_iter_rw(iter) == WRITE) {
int err = invalidate_inode_pages2_range(mapping,
start >> PAGE_SHIFT, end >> PAGE_SHIFT);
WARN_ON_ONCE(err);
}
return ret;
If we're going to replace this with an invalidation in
iomap_dio_complete() so it also handles the AIO path, then the
comment and warning on invalidation failure also need to be moved to
iomap_dio_complete() and the duplicate code removed from
iomap_dio_rw()...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
