On 12/9/21 2:22 AM, Jeff Layton wrote:
On Wed, 2021-12-08 at 10:33 -0500, Jeff Layton wrote:
On Wed, 2021-12-08 at 20:45 +0800, xiubli@xxxxxxxxxx wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>
Always return the last object's version.
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
fs/ceph/file.c | 8 ++++++--
fs/ceph/super.h | 3 ++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index b42158c9aa16..9279b8642add 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -883,7 +883,8 @@ enum {
* only return a short read to the caller if we hit EOF.
*/
ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
- struct iov_iter *to, int *retry_op)
+ struct iov_iter *to, int *retry_op,
+ u64 *last_objver)
{
struct ceph_inode_info *ci = ceph_inode(inode);
struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
@@ -950,6 +951,9 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
req->r_end_latency,
len, ret);
+ if (last_objver)
+ *last_objver = req->r_version;
+
Much better! That said, this might be unreliable if (say) the first OSD
read was successful and then the second one failed on a long read that
spans objects. We'd want to return a short read in that case, but the
last_objver would end up being set to 0.
I think you shouldn't set last_objver unless the call is going to return
0, and then you want to set it to the object version of the last
successful read in the series.
Make sense to me.
Since this was a simple change, I went ahead and folded the patch below
into this patch and updated wip-fscrypt-size. Let me know if you have
any objections:
[PATCH] SQUASH: only set last_objver iff we're returning success
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/ceph/file.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 9279b8642add..ee6fb642cf05 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -893,6 +893,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
u64 off = *ki_pos;
u64 len = iov_iter_count(to);
u64 i_size = i_size_read(inode);
+ u64 objver = 0;
dout("sync_read on inode %p %llu~%u\n", inode, *ki_pos, (unsigned)len);
@@ -951,9 +952,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
req->r_end_latency,
len, ret);
- if (last_objver)
- *last_objver = req->r_version;
-
+ objver = req->r_version;
ceph_osdc_put_request(req);
i_size = i_size_read(inode);
@@ -1010,6 +1009,9 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
}
}
+ if (ret > 0)
+ *last_objver = objver;
+
dout("sync_read result %zd retry_op %d\n", ret, *retry_op);
return ret;
}
This also looks good to me :-)
Thanks.
BRs
