| Message ID | 20241211205556.1754646-2-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: fix direct io folio offset and length calculation | expand |
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>
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>
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
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);
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(-)