Re: [PATCH v3 2/4] ceph: add __ceph_sync_read helper support

[Xiubo Li <xiubli@xxxxxxxxxx>]

On 10/29/21 2:21 AM, Jeff Layton wrote:
> On Thu, 2021-10-28 at 17:14 +0800, xiubli@xxxxxxxxxx wrote:
> > From: Xiubo Li <xiubli@xxxxxxxxxx>
> >
> > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> > ---
> >   fs/ceph/file.c  | 31 +++++++++++++++++++++----------
> >   fs/ceph/super.h |  2 ++
> >   2 files changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index 6e677b40410e..74db403a4c35 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -901,20 +901,17 @@ static inline void fscrypt_adjust_off_and_len(struct inode *inode, u64 *off, u64
> >    * If we get a short result from the OSD, check against i_size; we need to
> >    * only return a short read to the caller if we hit EOF.
> >    */
> > -static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> > -			      int *retry_op)
> > +ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
> > +			 struct iov_iter *to, int *retry_op)
> >   {
> > -	struct file *file = iocb->ki_filp;
> > -	struct inode *inode = file_inode(file);
> >   	struct ceph_inode_info *ci = ceph_inode(inode);
> >   	struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
> >   	struct ceph_osd_client *osdc = &fsc->client->osdc;
> >   	ssize_t ret;
> > -	u64 off = iocb->ki_pos;
> > +	u64 off = *ki_pos;
> >   	u64 len = iov_iter_count(to);
> >   
> > -	dout("sync_read on file %p %llu~%u %s\n", file, off, (unsigned)len,
> > -	     (file->f_flags & O_DIRECT) ? "O_DIRECT" : "");
> > +	dout("sync_read on inode %p %llu~%u\n", inode, *ki_pos, (unsigned)len);
> >   
> >   	if (!len)
> >   		return 0;
> > @@ -1058,18 +1055,32 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> >   			break;
> >   	}
> >   
> > -	if (off > iocb->ki_pos) {
> > +	if (off > *ki_pos) {
> >   		if (ret >= 0 &&
> >   		    iov_iter_count(to) > 0 && off >= i_size_read(inode))
> >   			*retry_op = CHECK_EOF;
> > -		ret = off - iocb->ki_pos;
> > -		iocb->ki_pos = off;
> > +		ret = off - *ki_pos;
> > +		*ki_pos = off;
> >   	}
> >   out:
> >   	dout("sync_read result %zd retry_op %d\n", ret, *retry_op);
> >   	return ret;
> >   }
> >   
> > +static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> > +			      int *retry_op)
> > +{
> > +	struct file *file = iocb->ki_filp;
> > +	struct inode *inode = file_inode(file);
> > +
> > +	dout("sync_read on file %p %llu~%u %s\n", file, iocb->ki_pos,
> > +	     (unsigned)iov_iter_count(to),
> > +	     (file->f_flags & O_DIRECT) ? "O_DIRECT" : "");
> > +
> > +	return __ceph_sync_read(inode, &iocb->ki_pos, to, retry_op);
> > +
> > +}
> > +
> >   struct ceph_aio_request {
> >   	struct kiocb *iocb;
> >   	size_t total_len;
> > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > index 027d5f579ba0..57bc952c54e1 100644
> > --- a/fs/ceph/super.h
> > +++ b/fs/ceph/super.h
> > @@ -1235,6 +1235,8 @@ extern int ceph_renew_caps(struct inode *inode, int fmode);
> >   extern int ceph_open(struct inode *inode, struct file *file);
> >   extern int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> >   			    struct file *file, unsigned flags, umode_t mode);
> > +extern ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
> > +				struct iov_iter *to, int *retry_op);
> >   extern int ceph_release(struct inode *inode, struct file *filp);
> >   extern void ceph_fill_inline_data(struct inode *inode, struct page *locked_page,
> >   				  char *data, size_t len);
> I went ahead and picked this patch too since #3 had a dependency on it.
> If we decide we want #3 for stable though, then we probably ought to
> respin these to avoid it.

I saw you have merged these two into the testing branch, should I respin 
for the #3 ?