* Re: [PATCH 1/1] fuse: fix direct io folio offset and length calculation
2024-12-11 20:55 ` [PATCH 1/1] " Joanne Koong
@ 2024-12-11 22:46 ` Bernd Schubert
2024-12-12 1:53 ` Jingbo Xu
2024-12-12 11:03 ` Miklos Szeredi
2 siblings, 0 replies; 5+ messages in thread
From: Bernd Schubert @ 2024-12-11 22:46 UTC (permalink / raw)
To: Joanne Koong, miklos, linux-fsdevel
Cc: josef, malte.schroeder, willy, kent.overstreet, jefflexu,
kernel-team
On 12/11/24 21:55, Joanne Koong wrote:
> For the direct io case, the pages from userspace may be part of a huge
> folio, even if all folios in the page cache for fuse are small.
>
> Fix the logic for calculating the offset and length of the folio for
> the direct io case, which currently incorrectly assumes that all folios
> encountered are one page size.
>
> Fixes: 3b97c3652d91 ("fuse: convert direct io to use folios")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
> fs/fuse/file.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 88d0946b5bc9..15b08d6a5739 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1557,18 +1557,22 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
>
> nbytes += ret;
>
> - ret += start;
> - /* Currently, all folios in FUSE are one page */
> - nfolios = DIV_ROUND_UP(ret, PAGE_SIZE);
> -
> - ap->descs[ap->num_folios].offset = start;
> - fuse_folio_descs_length_init(ap->descs, ap->num_folios, nfolios);
> - for (i = 0; i < nfolios; i++)
> - ap->folios[i + ap->num_folios] = page_folio(pages[i]);
> -
> - ap->num_folios += nfolios;
> - ap->descs[ap->num_folios - 1].length -=
> - (PAGE_SIZE - ret) & (PAGE_SIZE - 1);
> + nfolios = DIV_ROUND_UP(ret + start, PAGE_SIZE);
> +
> + for (i = 0; i < nfolios; i++) {
> + struct folio *folio = page_folio(pages[i]);
> + unsigned int offset = start +
> + (folio_page_idx(folio, pages[i]) << PAGE_SHIFT);
> + unsigned int len = min_t(unsigned int, ret, PAGE_SIZE - start);
> +
> + ap->descs[ap->num_folios].offset = offset;
> + ap->descs[ap->num_folios].length = len;
> + ap->folios[ap->num_folios] = folio;
> + start = 0;
> + ret -= len;
> + ap->num_folios++;
> + }
> +
> nr_pages += nfolios;
> }
> kfree(pages);
I had already looked at that yesterday. Had even created a
6.12 branch to remove the rather confusing
ap->descs[ap->num_pages - 1].length
line and replaced it with
- ap->descs[ap->num_pages - 1].length -=
- (PAGE_SIZE - ret) & (PAGE_SIZE - 1);
+ ap->descs[ap->num_pages - 1].length = offset_in_page(ret);
Anyway, thanks for the quick fix! Looks good to me.
Reviewed-by: Bernd Schubert <bschubert@ddn.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] fuse: fix direct io folio offset and length calculation
2024-12-11 20:55 ` [PATCH 1/1] " Joanne Koong
2024-12-11 22:46 ` Bernd Schubert
@ 2024-12-12 1:53 ` Jingbo Xu
2024-12-12 11:03 ` Miklos Szeredi
2 siblings, 0 replies; 5+ messages in thread
From: Jingbo Xu @ 2024-12-12 1:53 UTC (permalink / raw)
To: Joanne Koong, miklos, linux-fsdevel
Cc: josef, bernd.schubert, malte.schroeder, willy, kent.overstreet,
kernel-team
On 12/12/24 4:55 AM, Joanne Koong wrote:
> For the direct io case, the pages from userspace may be part of a huge
> folio, even if all folios in the page cache for fuse are small.
>
> Fix the logic for calculating the offset and length of the folio for
> the direct io case, which currently incorrectly assumes that all folios
> encountered are one page size.
>
> Fixes: 3b97c3652d91 ("fuse: convert direct io to use folios")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
> fs/fuse/file.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 88d0946b5bc9..15b08d6a5739 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1557,18 +1557,22 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
>
> nbytes += ret;
>
> - ret += start;
> - /* Currently, all folios in FUSE are one page */
> - nfolios = DIV_ROUND_UP(ret, PAGE_SIZE);
> -
> - ap->descs[ap->num_folios].offset = start;
> - fuse_folio_descs_length_init(ap->descs, ap->num_folios, nfolios);
> - for (i = 0; i < nfolios; i++)
> - ap->folios[i + ap->num_folios] = page_folio(pages[i]);
> -
> - ap->num_folios += nfolios;
> - ap->descs[ap->num_folios - 1].length -=
> - (PAGE_SIZE - ret) & (PAGE_SIZE - 1);
> + nfolios = DIV_ROUND_UP(ret + start, PAGE_SIZE);
> +
> + for (i = 0; i < nfolios; i++) {
> + struct folio *folio = page_folio(pages[i]);
> + unsigned int offset = start +
> + (folio_page_idx(folio, pages[i]) << PAGE_SHIFT);
> + unsigned int len = min_t(unsigned int, ret, PAGE_SIZE - start);
> +
> + ap->descs[ap->num_folios].offset = offset;
> + ap->descs[ap->num_folios].length = len;
> + ap->folios[ap->num_folios] = folio;
> + start = 0;
> + ret -= len;
> + ap->num_folios++;
> + }
> +
> nr_pages += nfolios;
> }
> kfree(pages);
LGTM.
Reviewed-by: Jingbo Xu <jefflexu@linux.alibaba.com>
--
Thanks,
Jingbo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] fuse: fix direct io folio offset and length calculation
2024-12-11 20:55 ` [PATCH 1/1] " Joanne Koong
2024-12-11 22:46 ` Bernd Schubert
2024-12-12 1:53 ` Jingbo Xu
@ 2024-12-12 11:03 ` Miklos Szeredi
2 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2024-12-12 11:03 UTC (permalink / raw)
To: Joanne Koong
Cc: linux-fsdevel, josef, bernd.schubert, malte.schroeder, willy,
kent.overstreet, jefflexu, kernel-team
On Wed, 11 Dec 2024 at 21:58, Joanne Koong <joannelkoong@gmail.com> wrote:
>
> For the direct io case, the pages from userspace may be part of a huge
> folio, even if all folios in the page cache for fuse are small.
>
> Fix the logic for calculating the offset and length of the folio for
> the direct io case, which currently incorrectly assumes that all folios
> encountered are one page size.
>
> Fixes: 3b97c3652d91 ("fuse: convert direct io to use folios")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Applied to fuse.git#for-next.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 5+ messages in thread