* [PATCH v3 0/2] ceph: skip copying the data extends the file EOF @ 2024-03-19 0:29 xiubli 2024-03-19 0:29 ` [PATCH v3 1/2] " xiubli 2024-03-19 0:29 ` [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read xiubli 0 siblings, 2 replies; 7+ messages in thread From: xiubli @ 2024-03-19 0:29 UTC (permalink / raw) To: ceph-devel; +Cc: idryomov, jlayton, vshankar, mchangir, frankhsiao, Xiubo Li From: Xiubo Li <xiubli@redhat.com> V3: - Fixed incorrectly handling the left bug reported by Ilya. V2: - append a new patch to improve the getattr code Xiubo Li (2): ceph: skip copying the data extends the file EOF ceph: set the correct mask for getattr reqeust for read fs/ceph/file.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] ceph: skip copying the data extends the file EOF 2024-03-19 0:29 [PATCH v3 0/2] ceph: skip copying the data extends the file EOF xiubli @ 2024-03-19 0:29 ` xiubli 2024-03-19 8:02 ` 回覆: " Frank Hsiao 蕭法宣 2024-03-19 0:29 ` [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read xiubli 1 sibling, 1 reply; 7+ messages in thread From: xiubli @ 2024-03-19 0:29 UTC (permalink / raw) To: ceph-devel; +Cc: idryomov, jlayton, vshankar, mchangir, frankhsiao, Xiubo Li From: Xiubo Li <xiubli@redhat.com> If hits the EOF it will revise the return value to the i_size instead of the real length read, but it will advance the offset of iovc, then for the next try it may be incorrectly skipped. This will just skip advancing the iovc's offset more than i_size. URL: https://patchwork.kernel.org/project/ceph-devel/list/?series=819323 Reported-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> Signed-off-by: Xiubo Li <xiubli@redhat.com> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> --- fs/ceph/file.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 24a003eaa5e0..c35878427985 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -1200,7 +1200,12 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos, } idx = 0; - left = ret > 0 ? ret : 0; + if (ret <= 0) + left = 0; + else if (off + ret > i_size) + left = i_size - off; + else + left = ret; while (left > 0) { size_t plen, copied; @@ -1229,15 +1234,13 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos, } if (ret > 0) { - if (off > *ki_pos) { - if (off >= i_size) { - *retry_op = CHECK_EOF; - ret = i_size - *ki_pos; - *ki_pos = i_size; - } else { - ret = off - *ki_pos; - *ki_pos = off; - } + if (off >= i_size) { + *retry_op = CHECK_EOF; + ret = i_size - *ki_pos; + *ki_pos = i_size; + } else { + ret = off - *ki_pos; + *ki_pos = off; } if (last_objver) -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* 回覆: [PATCH v3 1/2] ceph: skip copying the data extends the file EOF 2024-03-19 0:29 ` [PATCH v3 1/2] " xiubli @ 2024-03-19 8:02 ` Frank Hsiao 蕭法宣 0 siblings, 0 replies; 7+ messages in thread From: Frank Hsiao 蕭法宣 @ 2024-03-19 8:02 UTC (permalink / raw) To: xiubli@redhat.com; +Cc: ceph-devel@vger.kernel.org, idryomov@gmail.com Thanks Ilya for pointing this out. I've tested the new patch and it looks good. Reviewed-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> ________________________________________ 寄件者: xiubli@redhat.com <xiubli@redhat.com> 寄件日期: 2024年3月19日 上午 08:29 收件者: ceph-devel@vger.kernel.org 副本: idryomov@gmail.com; jlayton@kernel.org; vshankar@redhat.com; mchangir@redhat.com; Frank Hsiao 蕭法宣; Xiubo Li 主旨: [PATCH v3 1/2] ceph: skip copying the data extends the file EOF From: Xiubo Li <xiubli@redhat.com> If hits the EOF it will revise the return value to the i_size instead of the real length read, but it will advance the offset of iovc, then for the next try it may be incorrectly skipped. This will just skip advancing the iovc's offset more than i_size. URL: https://patchwork.kernel.org/project/ceph-devel/list/?series=819323 Reported-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> Signed-off-by: Xiubo Li <xiubli@redhat.com> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> --- fs/ceph/file.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 24a003eaa5e0..c35878427985 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -1200,7 +1200,12 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos, } idx = 0; - left = ret > 0 ? ret : 0; + if (ret <= 0) + left = 0; + else if (off + ret > i_size) + left = i_size - off; + else + left = ret; while (left > 0) { size_t plen, copied; @@ -1229,15 +1234,13 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos, } if (ret > 0) { - if (off > *ki_pos) { - if (off >= i_size) { - *retry_op = CHECK_EOF; - ret = i_size - *ki_pos; - *ki_pos = i_size; - } else { - ret = off - *ki_pos; - *ki_pos = off; - } + if (off >= i_size) { + *retry_op = CHECK_EOF; + ret = i_size - *ki_pos; + *ki_pos = i_size; + } else { + ret = off - *ki_pos; + *ki_pos = off; } if (last_objver) -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read 2024-03-19 0:29 [PATCH v3 0/2] ceph: skip copying the data extends the file EOF xiubli 2024-03-19 0:29 ` [PATCH v3 1/2] " xiubli @ 2024-03-19 0:29 ` xiubli 2024-03-19 8:02 ` 回覆: " Frank Hsiao 蕭法宣 2024-03-19 13:35 ` Ilya Dryomov 1 sibling, 2 replies; 7+ messages in thread From: xiubli @ 2024-03-19 0:29 UTC (permalink / raw) To: ceph-devel; +Cc: idryomov, jlayton, vshankar, mchangir, frankhsiao, Xiubo Li From: Xiubo Li <xiubli@redhat.com> In case of hitting the file EOF the ceph_read_iter() needs to retrieve the file size from MDS, and the Fr caps is not a neccessary. URL: https://patchwork.kernel.org/project/ceph-devel/list/?series=819323 Reported-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> Signed-off-by: Xiubo Li <xiubli@redhat.com> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> --- 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 c35878427985..a85f95c941fc 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -2191,14 +2191,16 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) int statret; struct page *page = NULL; loff_t i_size; + int mask = CEPH_STAT_CAP_SIZE; if (retry_op == READ_INLINE) { page = __page_cache_alloc(GFP_KERNEL); if (!page) return -ENOMEM; } - statret = __ceph_do_getattr(inode, page, - CEPH_STAT_CAP_INLINE_DATA, !!page); + if (retry_op == READ_INLINE) + mask = CEPH_STAT_CAP_INLINE_DATA; + statret = __ceph_do_getattr(inode, page, mask, !!page); if (statret < 0) { if (page) __free_page(page); @@ -2239,7 +2241,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) /* hit EOF or hole? */ if (retry_op == CHECK_EOF && iocb->ki_pos < i_size && ret < len) { - doutc(cl, "hit hole, ppos %lld < size %lld, reading more\n", + doutc(cl, "may hit hole, ppos %lld < size %lld, reading more\n", iocb->ki_pos, i_size); read += ret; -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* 回覆: [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read 2024-03-19 0:29 ` [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read xiubli @ 2024-03-19 8:02 ` Frank Hsiao 蕭法宣 2024-03-19 13:35 ` Ilya Dryomov 1 sibling, 0 replies; 7+ messages in thread From: Frank Hsiao 蕭法宣 @ 2024-03-19 8:02 UTC (permalink / raw) To: xiubli@redhat.com; +Cc: ceph-devel@vger.kernel.org, idryomov@gmail.com Reviewed-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> ________________________________________ 寄件者: xiubli@redhat.com <xiubli@redhat.com> 寄件日期: 2024年3月19日 上午 08:29 收件者: ceph-devel@vger.kernel.org 副本: idryomov@gmail.com; jlayton@kernel.org; vshankar@redhat.com; mchangir@redhat.com; Frank Hsiao 蕭法宣; Xiubo Li 主旨: [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read From: Xiubo Li <xiubli@redhat.com> In case of hitting the file EOF the ceph_read_iter() needs to retrieve the file size from MDS, and the Fr caps is not a neccessary. URL: https://patchwork.kernel.org/project/ceph-devel/list/?series=819323 Reported-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> Signed-off-by: Xiubo Li <xiubli@redhat.com> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> --- 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 c35878427985..a85f95c941fc 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -2191,14 +2191,16 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) int statret; struct page *page = NULL; loff_t i_size; + int mask = CEPH_STAT_CAP_SIZE; if (retry_op == READ_INLINE) { page = __page_cache_alloc(GFP_KERNEL); if (!page) return -ENOMEM; } - statret = __ceph_do_getattr(inode, page, - CEPH_STAT_CAP_INLINE_DATA, !!page); + if (retry_op == READ_INLINE) + mask = CEPH_STAT_CAP_INLINE_DATA; + statret = __ceph_do_getattr(inode, page, mask, !!page); if (statret < 0) { if (page) __free_page(page); @@ -2239,7 +2241,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) /* hit EOF or hole? */ if (retry_op == CHECK_EOF && iocb->ki_pos < i_size && ret < len) { - doutc(cl, "hit hole, ppos %lld < size %lld, reading more\n", + doutc(cl, "may hit hole, ppos %lld < size %lld, reading more\n", iocb->ki_pos, i_size); read += ret; -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read 2024-03-19 0:29 ` [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read xiubli 2024-03-19 8:02 ` 回覆: " Frank Hsiao 蕭法宣 @ 2024-03-19 13:35 ` Ilya Dryomov 2024-03-19 23:53 ` Xiubo Li 1 sibling, 1 reply; 7+ messages in thread From: Ilya Dryomov @ 2024-03-19 13:35 UTC (permalink / raw) To: xiubli; +Cc: ceph-devel, jlayton, vshankar, mchangir, frankhsiao On Tue, Mar 19, 2024 at 1:32 AM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > In case of hitting the file EOF the ceph_read_iter() needs to > retrieve the file size from MDS, and the Fr caps is not a neccessary. > > URL: https://patchwork.kernel.org/project/ceph-devel/list/?series=819323 > Reported-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> > Signed-off-by: Xiubo Li <xiubli@redhat.com> > Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> > --- > 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 c35878427985..a85f95c941fc 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -2191,14 +2191,16 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) > int statret; > struct page *page = NULL; > loff_t i_size; > + int mask = CEPH_STAT_CAP_SIZE; > if (retry_op == READ_INLINE) { > page = __page_cache_alloc(GFP_KERNEL); > if (!page) > return -ENOMEM; > } > > - statret = __ceph_do_getattr(inode, page, > - CEPH_STAT_CAP_INLINE_DATA, !!page); > + if (retry_op == READ_INLINE) > + mask = CEPH_STAT_CAP_INLINE_DATA; Hi Xiubo, This introduces an additional retry_op == READ_INLINE branch right below an existing one. Should this be: int mask = CEPH_STAT_CAP_SIZE; if (retry_op == READ_INLINE) { page = __page_cache_alloc(GFP_KERNEL); if (!page) return -ENOMEM; mask = CEPH_STAT_CAP_INLINE_DATA; } Thanks, Ilya ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read 2024-03-19 13:35 ` Ilya Dryomov @ 2024-03-19 23:53 ` Xiubo Li 0 siblings, 0 replies; 7+ messages in thread From: Xiubo Li @ 2024-03-19 23:53 UTC (permalink / raw) To: Ilya Dryomov; +Cc: ceph-devel, jlayton, vshankar, mchangir, frankhsiao On 3/19/24 21:35, Ilya Dryomov wrote: > On Tue, Mar 19, 2024 at 1:32 AM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> In case of hitting the file EOF the ceph_read_iter() needs to >> retrieve the file size from MDS, and the Fr caps is not a neccessary. >> >> URL: https://patchwork.kernel.org/project/ceph-devel/list/?series=819323 >> Reported-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com> >> --- >> 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 c35878427985..a85f95c941fc 100644 >> --- a/fs/ceph/file.c >> +++ b/fs/ceph/file.c >> @@ -2191,14 +2191,16 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to) >> int statret; >> struct page *page = NULL; >> loff_t i_size; >> + int mask = CEPH_STAT_CAP_SIZE; >> if (retry_op == READ_INLINE) { >> page = __page_cache_alloc(GFP_KERNEL); >> if (!page) >> return -ENOMEM; >> } >> >> - statret = __ceph_do_getattr(inode, page, >> - CEPH_STAT_CAP_INLINE_DATA, !!page); >> + if (retry_op == READ_INLINE) >> + mask = CEPH_STAT_CAP_INLINE_DATA; > Hi Xiubo, > > This introduces an additional retry_op == READ_INLINE branch right > below an existing one. Should this be: > > int mask = CEPH_STAT_CAP_SIZE; > if (retry_op == READ_INLINE) { > page = __page_cache_alloc(GFP_KERNEL); > if (!page) > return -ENOMEM; > > mask = CEPH_STAT_CAP_INLINE_DATA; > } Look good to me. Thanks Ilya. - Xiubo > Thanks, > > Ilya > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-19 23:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-19 0:29 [PATCH v3 0/2] ceph: skip copying the data extends the file EOF xiubli 2024-03-19 0:29 ` [PATCH v3 1/2] " xiubli 2024-03-19 8:02 ` 回覆: " Frank Hsiao 蕭法宣 2024-03-19 0:29 ` [PATCH v3 2/2] ceph: set the correct mask for getattr reqeust for read xiubli 2024-03-19 8:02 ` 回覆: " Frank Hsiao 蕭法宣 2024-03-19 13:35 ` Ilya Dryomov 2024-03-19 23:53 ` Xiubo Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).