On Fri, 2021-10-29 at 16:14 +0800, Xiubo Li wrote:
> 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 ?
>
>
Yeah, it's probably worthwhile to mark this for stable. It _is_ a data
integrity issue after all.
Could you respin it so that it doesn't rely on patch #2?
--
Jeff Layton <jlayton@xxxxxxxxxx>
