| Message ID | 20220421083619.161391-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: skip reading from Rados if pos exceeds i_size | expand |
On Thu, Apr 21, 2022 at 1:37 AM Xiubo Li <xiubli@redhat.com> wrote: > > Since we have held the Fr capibility it's safe to skip reading from > Rados if the ki_pos is larger than or euqals to the file size. I'm not sure this is correct, based on the patch description. If you're in a mixed mode where there are writers and readers, they can all have Frw and extend the file size by issuing rados writes up to the max_size without updating each other. The client needs to have Fs before it can rely on knowing the file size, so we should check that before skipping a read, right? -Greg > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/file.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 6c9e837aa1d3..330e42b3afec 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -1614,7 +1614,9 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) > return ret; > } > > - if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 || > + if (unlikely(iocb->ki_pos >= i_size_read(inode))) { > + ret = 0; > + } else if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 || > (iocb->ki_flags & IOCB_DIRECT) || > (fi->flags & CEPH_F_SYNC)) { > > -- > 2.36.0.rc1 >
On 4/21/22 9:19 PM, Gregory Farnum wrote: > On Thu, Apr 21, 2022 at 1:37 AM Xiubo Li <xiubli@redhat.com> wrote: >> Since we have held the Fr capibility it's safe to skip reading from >> Rados if the ki_pos is larger than or euqals to the file size. > I'm not sure this is correct, based on the patch description. If > you're in a mixed mode where there are writers and readers, they can > all have Frw and extend the file size by issuing rados writes up to > the max_size without updating each other. The client needs to have Fs > before it can rely on knowing the file size, so we should check that > before skipping a read, right? There still need some other fixes in kclient, that is fixing the __ceph_should_report_size() to report the size to the MDS when the i_size changed and dirty the Fw caps before check_caps() for each write. Then the MDS Locker code will issue new caps to all the clients. While I think there still has a gap with this fixing. Because the readers may need a little time to get the notification and to update the new file size. For now I will drop this change. -- Xiubo > -Greg > >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> fs/ceph/file.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ceph/file.c b/fs/ceph/file.c >> index 6c9e837aa1d3..330e42b3afec 100644 >> --- a/fs/ceph/file.c >> +++ b/fs/ceph/file.c >> @@ -1614,7 +1614,9 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) >> return ret; >> } >> >> - if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 || >> + if (unlikely(iocb->ki_pos >= i_size_read(inode))) { >> + ret = 0; >> + } else if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 || >> (iocb->ki_flags & IOCB_DIRECT) || >> (fi->flags & CEPH_F_SYNC)) { >> >> -- >> 2.36.0.rc1 >>
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 6c9e837aa1d3..330e42b3afec 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -1614,7 +1614,9 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) return ret; } - if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 || + if (unlikely(iocb->ki_pos >= i_size_read(inode))) { + ret = 0; + } else if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 || (iocb->ki_flags & IOCB_DIRECT) || (fi->flags & CEPH_F_SYNC)) {
Since we have held the Fr capibility it's safe to skip reading from Rados if the ki_pos is larger than or euqals to the file size. Signed-off-by: Xiubo Li <xiubli@redhat.com> --- fs/ceph/file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)