* [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 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 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 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 read-only IMAP folder(s) and NNTP newsgroup(s).