On 6/5/2023 6:11 AM, Matthew Wilcox wrote: > On Sun, Jun 04, 2023 at 11:29:52AM -0700, Darrick J. Wong wrote: >> On Fri, Jun 02, 2023 at 11:24:44PM +0100, Matthew Wilcox (Oracle) wrote: >>> - copied = copy_page_from_iter_atomic(page, offset, bytes, i); >>> + copied = copy_page_from_iter_atomic(&folio->page, offset, bytes, i); >> >> I think I've gotten lost in the weeds. Does copy_page_from_iter_atomic >> actually know how to deal with a multipage folio? AFAICT it takes a >> page, kmaps it, and copies @bytes starting at @offset in the page. If >> a caller feeds it a multipage folio, does that all work correctly? Or >> will the pagecache split multipage folios as needed to make it work >> right? > > It's a smidgen inefficient, but it does work. First, it calls > page_copy_sane() to check that offset & n fit within the compound page > (ie this all predates folios). > > ... Oh. copy_page_from_iter() handles this correctly. > copy_page_from_iter_atomic() doesn't. I'll have to fix this > first. Looks like Al fixed copy_page_from_iter() in c03f05f183cd > and didn't fix copy_page_from_iter_atomic(). > >> If we create a 64k folio at pos 0 and then want to write a byte at pos >> 40k, does __filemap_get_folio break up the 64k folio so that the folio >> returned by iomap_get_folio starts at 40k? Or can the iter code handle >> jumping ten pages into a 16-page folio and I just can't see it? > > Well ... it handles it fine unless it's highmem. p is kaddr + offset, > so if offset is 40k, it works correctly on !highmem. So is it better to have implementations for !highmem and highmem? And for !highmem, we don't need the kmap_local_page()/kunmap_local() and chunk size per copy is not limited to PAGE_SIZE. Thanks. Regards Yin, Fengwei
