On Wed, Aug 31 2016, Jeff Layton wrote:
>
> Good catch. Even better might be to just declare a int ret2 and not
> clobber "ret" at all.
Like the following? Must better, yes.
>
> Clearly, mixing buffered and direct I/O is gross, but I suppose you
> could hit the occasional problem here with a real workload
> occasionally.
>
> Should this go to stable? The patch seems safe enough.
Hardly seems worth it, but certainly safe enough.
>
> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
Thanks,
NeilBrown
From: NeilBrown <neilb@xxxxxxxx>
Subject: [PATCH] cephfs: ignore error from invalidate_inode_pages2_range() in
direct write.
This call can fail if there are dirty pages. The preceding call to
filemap_write_and_wait_range() will normally remove dirty pages, but
as inode_lock() is not held over calls to ceph_direct_read_write(), it
could race with non-direct writes and pages could be dirtied
immediately after filemap_write_and_wait_range() returns
If there are dirty pages, they will be removed by the subsequent call
to truncate_inode_pages_range(), so having them here is not a problem.
If the 'ret' value is left holding an error, then in the async IO case
(aio_req is not NULL) the loop that would normally call
ceph_osdc_start_request() will see the error in 'ret' and abort all
requests. This doesn't seem like correct behaviour.
So use separate 'ret2' instead of overloading 'ret'
Signed-off-by: NeilBrown <neilb@xxxxxxxx>
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/ceph/file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 0f5375d8e030..395c7fcb1cea 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -902,10 +902,10 @@ ceph_direct_read_write(struct kiocb *iocb, struct iov_iter *iter,
return ret;
if (write) {
- ret = invalidate_inode_pages2_range(inode->i_mapping,
+ int ret2 = invalidate_inode_pages2_range(inode->i_mapping,
pos >> PAGE_SHIFT,
(pos + count) >> PAGE_SHIFT);
- if (ret < 0)
+ if (ret2 < 0)
dout("invalidate_inode_pages2_range returned %d\n", ret);
flags = CEPH_OSD_FLAG_ORDERSNAP |
--
2.9.3
Attachment:
signature.asc
Description: PGP signature
