diff mbox series

[v3,2/2] ceph: set the correct mask for getattr reqeust for read

Message ID 20240319002925.1228063-3-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series ceph: skip copying the data extends the file EOF | expand

Commit Message

Xiubo Li March 19, 2024, 12:29 a.m. UTC
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(-)

Comments

Ilya Dryomov March 19, 2024, 1:35 p.m. UTC | #1
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
Xiubo Li March 19, 2024, 11:53 p.m. UTC | #2
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
>
diff mbox series

Patch

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;