* [PATCH v2 0/2] ceph: skip copying the data extends the file EOF
@ 2024-02-22 13:18 xiubli
2024-02-22 13:18 ` [PATCH v2 1/2] " xiubli
2024-02-22 13:19 ` [PATCH v2 2/2] ceph: set the correct mask for getattr reqeust for read xiubli
0 siblings, 2 replies; 7+ messages in thread
From: xiubli @ 2024-02-22 13:18 UTC (permalink / raw
To: ceph-devel; +Cc: idryomov, jlayton, vshankar, mchangir, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
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 | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] ceph: skip copying the data extends the file EOF
2024-02-22 13:18 [PATCH v2 0/2] ceph: skip copying the data extends the file EOF xiubli
@ 2024-02-22 13:18 ` xiubli
2024-02-26 3:45 ` 回覆: " Frank Hsiao 蕭法宣
2024-03-18 21:02 ` Ilya Dryomov
2024-02-22 13:19 ` [PATCH v2 2/2] ceph: set the correct mask for getattr reqeust for read xiubli
1 sibling, 2 replies; 7+ messages in thread
From: xiubli @ 2024-02-22 13:18 UTC (permalink / raw
To: ceph-devel
Cc: idryomov, jlayton, vshankar, mchangir, Xiubo Li,
Frank Hsiao 蕭法宣
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>
---
fs/ceph/file.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 71d29571712d..2b2b07a0a61b 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1195,7 +1195,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
}
idx = 0;
- left = ret > 0 ? ret : 0;
+ left = ret > 0 ? umin(ret, i_size) : 0;
while (left > 0) {
size_t plen, copied;
@@ -1224,15 +1224,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 v2 2/2] ceph: set the correct mask for getattr reqeust for read
2024-02-22 13:18 [PATCH v2 0/2] ceph: skip copying the data extends the file EOF xiubli
2024-02-22 13:18 ` [PATCH v2 1/2] " xiubli
@ 2024-02-22 13:19 ` xiubli
2024-02-26 3:46 ` 回覆: " Frank Hsiao 蕭法宣
1 sibling, 1 reply; 7+ messages in thread
From: xiubli @ 2024-02-22 13:19 UTC (permalink / raw
To: ceph-devel
Cc: idryomov, jlayton, vshankar, mchangir, Xiubo Li,
Frank Hsiao 蕭法宣
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>
---
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 2b2b07a0a61b..08c918aa403e 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -2181,14 +2181,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);
@@ -2229,7 +2231,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 v2 1/2] ceph: skip copying the data extends the file EOF
2024-02-22 13:18 ` [PATCH v2 1/2] " xiubli
@ 2024-02-26 3:45 ` Frank Hsiao 蕭法宣
2024-03-18 21:02 ` Ilya Dryomov
1 sibling, 0 replies; 7+ messages in thread
From: Frank Hsiao 蕭法宣 @ 2024-02-26 3:45 UTC (permalink / raw
To: xiubli@redhat.com; +Cc: ceph-devel@vger.kernel.org
Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com>
________________________________________
寄件者: xiubli@redhat.com <xiubli@redhat.com>
寄件日期: 2024年2月22日 下午 09:18
收件者: ceph-devel@vger.kernel.org
副本: idryomov@gmail.com; jlayton@kernel.org; vshankar@redhat.com; mchangir@redhat.com; Xiubo Li; Frank Hsiao 蕭法宣
主旨: [PATCH v2 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>
---
fs/ceph/file.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 71d29571712d..2b2b07a0a61b 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1195,7 +1195,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
}
idx = 0;
- left = ret > 0 ? ret : 0;
+ left = ret > 0 ? umin(ret, i_size) : 0;
while (left > 0) {
size_t plen, copied;
@@ -1224,15 +1224,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 v2 2/2] ceph: set the correct mask for getattr reqeust for read
2024-02-22 13:19 ` [PATCH v2 2/2] ceph: set the correct mask for getattr reqeust for read xiubli
@ 2024-02-26 3:46 ` Frank Hsiao 蕭法宣
0 siblings, 0 replies; 7+ messages in thread
From: Frank Hsiao 蕭法宣 @ 2024-02-26 3:46 UTC (permalink / raw
To: xiubli@redhat.com; +Cc: ceph-devel@vger.kernel.org
Tested-by: Frank Hsiao 蕭法宣 <frankhsiao@qnap.com>
________________________________________
寄件者: xiubli@redhat.com <xiubli@redhat.com>
寄件日期: 2024年2月22日 下午 09:19
收件者: ceph-devel@vger.kernel.org
副本: idryomov@gmail.com; jlayton@kernel.org; vshankar@redhat.com; mchangir@redhat.com; Xiubo Li; Frank Hsiao 蕭法宣
主旨: [PATCH v2 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>
---
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 2b2b07a0a61b..08c918aa403e 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -2181,14 +2181,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);
@@ -2229,7 +2231,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 v2 1/2] ceph: skip copying the data extends the file EOF
2024-02-22 13:18 ` [PATCH v2 1/2] " xiubli
2024-02-26 3:45 ` 回覆: " Frank Hsiao 蕭法宣
@ 2024-03-18 21:02 ` Ilya Dryomov
2024-03-19 0:18 ` Xiubo Li
1 sibling, 1 reply; 7+ messages in thread
From: Ilya Dryomov @ 2024-03-18 21:02 UTC (permalink / raw
To: xiubli
Cc: ceph-devel, jlayton, vshankar, mchangir,
Frank Hsiao 蕭法宣
On Thu, Feb 22, 2024 at 2:21 PM <xiubli@redhat.com> wrote:
>
> 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>
> ---
> fs/ceph/file.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 71d29571712d..2b2b07a0a61b 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1195,7 +1195,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
> }
>
> idx = 0;
> - left = ret > 0 ? ret : 0;
> + left = ret > 0 ? umin(ret, i_size) : 0;
Hi Xiubo,
Can ret (i.e. the number of bytes actually read) be compared to i_size
without taking the offset into account? How does this a handle a case
where e.g.
off = 20
ret = 10
i_size = 25
Did you intend the copy_page_to_iter() loop to go over 10 bytes and
therefore advance the iovc ("to") by 10 instead of 5 bytes here?
Thanks,
Ilya
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] ceph: skip copying the data extends the file EOF
2024-03-18 21:02 ` Ilya Dryomov
@ 2024-03-19 0:18 ` Xiubo Li
0 siblings, 0 replies; 7+ messages in thread
From: Xiubo Li @ 2024-03-19 0:18 UTC (permalink / raw
To: Ilya Dryomov
Cc: ceph-devel, jlayton, vshankar, mchangir,
Frank Hsiao 蕭法宣
On 3/19/24 05:02, Ilya Dryomov wrote:
> On Thu, Feb 22, 2024 at 2:21 PM <xiubli@redhat.com> wrote:
>> 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>
>> ---
>> fs/ceph/file.c | 18 ++++++++----------
>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 71d29571712d..2b2b07a0a61b 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -1195,7 +1195,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
>> }
>>
>> idx = 0;
>> - left = ret > 0 ? ret : 0;
>> + left = ret > 0 ? umin(ret, i_size) : 0;
> Hi Xiubo,
>
> Can ret (i.e. the number of bytes actually read) be compared to i_size
> without taking the offset into account? How does this a handle a case
> where e.g.
>
> off = 20
> ret = 10
> i_size = 25
>
> Did you intend the copy_page_to_iter() loop to go over 10 bytes and
> therefore advance the iovc ("to") by 10 instead of 5 bytes here?
Good catch!
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 142deb242196..531874935c21 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1204,7 +1204,12 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
}
idx = 0;
- left = ret > 0 ? umin(ret, i_size) : 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;
Let me fix this in V3.
Thanks Ilya!
- Xiubo
> Thanks,
>
> Ilya
>
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-19 0:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-22 13:18 [PATCH v2 0/2] ceph: skip copying the data extends the file EOF xiubli
2024-02-22 13:18 ` [PATCH v2 1/2] " xiubli
2024-02-26 3:45 ` 回覆: " Frank Hsiao 蕭法宣
2024-03-18 21:02 ` Ilya Dryomov
2024-03-19 0:18 ` Xiubo Li
2024-02-22 13:19 ` [PATCH v2 2/2] ceph: set the correct mask for getattr reqeust for read xiubli
2024-02-26 3:46 ` 回覆: " Frank Hsiao 蕭法宣
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 read-only IMAP folder(s) and NNTP newsgroup(s).