On Mon, Dec 09, 2024 at 09:49:48AM -0500, Josef Bacik wrote: > > Ha! This time I bisected from f03b296e8b51 to d1dfb5f52ffc. I ended up > > with 3b97c3652d91 as the culprit. > > Willy, I've looked at this code and it does indeed look like a 1:1 conversion, > EXCEPT I'm fuzzy about how how this works with large folios. Previously, if we > got a hugepage in, we'd get each individual struct page back for the whole range > of the hugepage, so if for example we had a 2M hugepage, we'd fill in the > ->offset for each "middle" struct page as 0, since obviously we're consuming > PAGE_SIZE chunks at a time. > > But now we're doing this > > for (i = 0; i < nfolios; i++) > ap->folios[i + ap->num_folios] = page_folio(pages[i]); > > So if userspace handed us a 2M hugepage, page_folio() on each of the > intermediary struct page's would return the same folio, correct? So we'd end up > with the wrong offsets for our fuse request, because they should be based from > the start of the folio, correct? I think you're 100% right. We could put in some nice asserts to check this is what's happening, but it does seem like a rather incautious conversion. Yes, all folios _in the page cache_ for fuse are small, but that's not guaranteed to be the case for folios found in userspace for directio. At least the comment is wrong, and I'd suggest the code is too.
