Page MenuHomeFreeBSD

linuxkpi: Add `folio` and `folio_batch` APIs
Needs ReviewPublic

Authored by dumbbell on Jan 31 2025, 2:08 PM.
Tags
None
Referenced Files
F111815624: D48743.diff
Sat, Mar 8, 6:47 PM
Unknown Object (File)
Wed, Mar 5, 10:00 PM
Unknown Object (File)
Fri, Feb 28, 12:44 PM
Unknown Object (File)
Thu, Feb 27, 4:32 AM
Unknown Object (File)
Mon, Feb 17, 2:12 AM
Unknown Object (File)
Thu, Feb 13, 7:20 AM
Unknown Object (File)
Thu, Feb 13, 6:52 AM
Unknown Object (File)
Wed, Feb 12, 6:58 PM
Subscribers

Details

Reviewers
bz
Group Reviewers
linuxkpi
Summary

They are used by the i915 DRM driver in Linux 6.6 (although this change was only backported with Linux 6.7 DRM drivers...).

struct folio simply wraps struct page for now.

struct folio_batch is the same as struct pagevec but it works with struct folio instead of struct page directly.

This is part of the update of DRM drivers to Linux 6.7.

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

(For next time please upload with context, e.g. -U999999)

Add diff context. No other changes.

Manu told me the same, so while I’m uploading patches to address bz feedback, I’m updating existing patch with their context.

bz requested changes to this revision.Sat, Feb 8, 8:37 PM
bz added a subscriber: bz.
bz added inline comments.
sys/compat/linuxkpi/common/include/linux/gfp.h
137

Can we please move the alloc (and free) functions into C files so we can change them in the future without breaking the KBI?

sys/compat/linuxkpi/common/include/linux/mm.h
169

return ()

173

This indicates there is some kind of reference counting on struct folio? Are you sure you can always just free here? Hmm given there is no folio_get yet you may indeed still be able to get away with it... but more sophisticated use will likely require this one day.

451

return ()

457

return ()

463

return ()

475

return ()

sys/compat/linuxkpi/common/include/linux/mm_types.h
83
/*
 * text
 * more text
 */
84

Also __folio_batch_release() relies on this currently.

sys/compat/linuxkpi/common/include/linux/pagevec.h
93

return ()

99

return ()

106

return ()

113

This also has the assumption that struct folio is struct page.

sys/compat/linuxkpi/common/include/linux/shmem_fs.h
64

return ()

sys/compat/linuxkpi/common/include/linux/swap.h
63

XXX-BZ look this up as we should not do that anymore; there should be a page_* KPI for this? It'll break when we will have a real struct page and not just a typedef.

Can we try calling mark_page_accessed() for the page? That way we contain the problem to the one function for now?

This revision now requires changes to proceed.Sat, Feb 8, 8:37 PM
sys/compat/linuxkpi/common/include/linux/mm.h
173

You are right. I will rework the patch to fix it.

I spent some time to review the folio code and the existing page allocation code (alloc_pages(), __free_pages(), put_page(), release_pages(), ...).

Pages from alloc_pages() are not attached to a VM object (unmanaged). Our code assumes that a page allocated this way will be freed using __free_pages() which takes care of the unwiring and if 0, caliing vm_page_free().

put_page() and release_pages() assume that the page was allocated by something else and pages are managed and that something else will take care of the page free. They don’t do anything special when the wiring count reaches zero.

Now from folio_batch_release(), I wonder if I should check the VPO_MANAGED flag to determine if the page should simply be unwired (same as put_page()) or if it should call __free_pages(). In other words, I wonder if this flag can be used to unify the put_page(), release_pages() and folio_batch_release() functions.

FYI, on Linux, put_page() calls folio_put() (which I will add) and folio_batch_release() calls release_pages() (which accept a struct page ** or a struct folio ** through an anonymous union and assumes that a page can be casted to a folio). When the refcount reaches zero, both code paths call the same code to free pages.

What do you think, @bz and @kib?

I reworked the patch to unify the put_page() and __free_pages() code paths. Now, all put/freed pages are handled by __free_pages().

release_pages() accepts either a list of struct page or a list of struct folio, like Linux.