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@xxxxxxxxx>
> ---
> 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@xxxxxxx>
