| Message ID | 20200903165632.1338996-1-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iomap: Fix direct I/O write consistency check | expand |
On 9/3/20 11:56 AM, Andreas Gruenbacher wrote: > When a direct I/O write falls back to buffered I/O entirely, dio->size > will be 0 in iomap_dio_complete. Function invalidate_inode_pages2_range > will try to invalidate the rest of the address space. (Because if ->size == 0 and offset == 0, then invalidating up to (0+0-1) will invalidate the entire range.) err = invalidate_inode_pages2_range(inode->i_mapping, offset >> PAGE_SHIFT, (offset + dio->size - 1) >> PAGE_SHIFT); so I guess this behavior is unique to writing to a hole at offset 0? FWIW, this same test yields the same results on ext3 when it falls back to buffered. -Eric > If there are any > dirty pages in that range, the write will fail and a "Page cache > invalidation failure on direct I/O" error will be logged. > > On gfs2, this can be reproduced as follows: > > xfs_io \ > -c "open -ft foo" -c "pwrite 4k 4k" -c "close" \ > -c "open -d foo" -c "pwrite 0 4k" > > Fix this by recognizing 0-length writes. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > --- > fs/iomap/direct-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index c1aafb2ab990..c9d6b4eecdb7 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -108,7 +108,7 @@ static ssize_t iomap_dio_complete(struct iomap_dio *dio) > * ->end_io() when necessary, otherwise a racing buffer read would cache > * zeros from unwritten extents. > */ > - if (!dio->error && > + if (!dio->error && dio->size && > (dio->flags & IOMAP_DIO_WRITE) && inode->i_mapping->nrpages) { > int err; > err = invalidate_inode_pages2_range(inode->i_mapping, >
On Thu, Sep 3, 2020 at 11:12 PM Eric Sandeen <sandeen@sandeen.net> wrote: > On 9/3/20 11:56 AM, Andreas Gruenbacher wrote: > > When a direct I/O write falls back to buffered I/O entirely, dio->size > > will be 0 in iomap_dio_complete. Function invalidate_inode_pages2_range > > will try to invalidate the rest of the address space. > > (Because if ->size == 0 and offset == 0, then invalidating up to (0+0-1) will > invalidate the entire range.) > > > err = invalidate_inode_pages2_range(inode->i_mapping, > offset >> PAGE_SHIFT, > (offset + dio->size - 1) >> PAGE_SHIFT); > > so I guess this behavior is unique to writing to a hole at offset 0? > > FWIW, this same test yields the same results on ext3 when it falls back to > buffered. That's interesting. An ext3 formatted filesystem will invoke dio_warn_stale_pagecache and thus log the error message, but the error isn't immediately reported by the "pwrite 0 4k". It takes adding '-c "fsync"' to the xfs_io command or similar to make it fail. An ext4 formatted filesystem doesn't show any of these problems. Thanks, Andreas > -Eric > > > If there are any > > dirty pages in that range, the write will fail and a "Page cache > > invalidation failure on direct I/O" error will be logged. > > > > On gfs2, this can be reproduced as follows: > > > > xfs_io \ > > -c "open -ft foo" -c "pwrite 4k 4k" -c "close" \ > > -c "open -d foo" -c "pwrite 0 4k" > > > > Fix this by recognizing 0-length writes. > > > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > > --- > > fs/iomap/direct-io.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > > index c1aafb2ab990..c9d6b4eecdb7 100644 > > --- a/fs/iomap/direct-io.c > > +++ b/fs/iomap/direct-io.c > > @@ -108,7 +108,7 @@ static ssize_t iomap_dio_complete(struct iomap_dio *dio) > > * ->end_io() when necessary, otherwise a racing buffer read would cache > > * zeros from unwritten extents. > > */ > > - if (!dio->error && > > + if (!dio->error && dio->size && > > (dio->flags & IOMAP_DIO_WRITE) && inode->i_mapping->nrpages) { > > int err; > > err = invalidate_inode_pages2_range(inode->i_mapping, > > >
On Thu, Sep 03, 2020 at 06:56:32PM +0200, Andreas Gruenbacher wrote: > When a direct I/O write falls back to buffered I/O entirely, dio->size > will be 0 in iomap_dio_complete. Function invalidate_inode_pages2_range > will try to invalidate the rest of the address space. If there are any > dirty pages in that range, the write will fail and a "Page cache > invalidation failure on direct I/O" error will be logged. Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index c1aafb2ab990..c9d6b4eecdb7 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -108,7 +108,7 @@ static ssize_t iomap_dio_complete(struct iomap_dio *dio) * ->end_io() when necessary, otherwise a racing buffer read would cache * zeros from unwritten extents. */ - if (!dio->error && + if (!dio->error && dio->size && (dio->flags & IOMAP_DIO_WRITE) && inode->i_mapping->nrpages) { int err; err = invalidate_inode_pages2_range(inode->i_mapping,
When a direct I/O write falls back to buffered I/O entirely, dio->size will be 0 in iomap_dio_complete. Function invalidate_inode_pages2_range will try to invalidate the rest of the address space. If there are any dirty pages in that range, the write will fail and a "Page cache invalidation failure on direct I/O" error will be logged. On gfs2, this can be reproduced as follows: xfs_io \ -c "open -ft foo" -c "pwrite 4k 4k" -c "close" \ -c "open -d foo" -c "pwrite 0 4k" Fix this by recognizing 0-length writes. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/iomap/direct-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)