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.
>
>
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;
}
--
2.33.1
