On Tue, May 13, 2025 at 5:18 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Mon, May 12, 2025 at 03:58:30PM -0700, Joanne Koong wrote:
> > @@ -1126,22 +1127,22 @@ static int fuse_copy_page(struct fuse_copy_state *cs, struct page **pagep,
> > return err;
> > }
> > }
> > - if (page) {
> > - void *mapaddr = kmap_local_page(page);
> > - void *buf = mapaddr + offset;
> > + if (folio) {
> > + void *mapaddr = kmap_local_folio(folio, offset);
> > + void *buf = mapaddr;
> > offset += fuse_copy_do(cs, &buf, &count);
> > kunmap_local(mapaddr);
>
> kmap_local_folio() only maps the page which contains 'offset'.
> following what the functions in highmem.h do, i'd suggest something
> like:
>
> if (folio) {
> void *mapaddr = kmap_local_folio(folio, offset);
> void *buf = mapaddr;
>
> if (folio_test_highmem(folio) &&
> size > PAGE_SIZE - offset_in_page(offset))
> size = PAGE_SIZE - offset_in_page(offset);
> offset += fuse_copy_do(cs, &buf, &count);
> kunmap_local(mapaddr);
>
Ahh okay, I see, thanks. Do you think it makes sense to change
kmap_local_folio() to kmap all the pages in the folio if the folio is
in highmem instead of the caller needing to do that for each page in
the folio one by one? We would need a kunmap_local_folio() where we
pass in the folio so that we know how many pages need to be unmapped,
but it seems to me like with large folios, every caller will be
running into this issue, so maybe we should just have
kmap_local_folio() handle it?
Thanks,
Joanne
