Page MenuHomeFreeBSD

Add per-freepool page caches.
ClosedPublic

Authored by markj on Jul 4 2019, 7:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 21 2023, 2:34 PM
Unknown Object (File)
Dec 20 2023, 7:30 AM
Unknown Object (File)
Nov 7 2023, 8:16 AM
Unknown Object (File)
Aug 18 2023, 9:23 PM
Unknown Object (File)
Aug 2 2023, 1:48 AM
Unknown Object (File)
Aug 2 2023, 1:46 AM
Unknown Object (File)
Aug 2 2023, 1:46 AM
Unknown Object (File)
Aug 2 2023, 1:45 AM
Subscribers

Details

Summary

We must pack the domain and pool indices into the zone argument, which
is kind of ugly.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/vm/vm_page.c
188 ↗(On Diff #59412)

I wonder if a struct with bitfields to name the parts of the integer might be a little less ugly.

sys/vm/vm_page.c
188 ↗(On Diff #59412)

I like it better than what I did, but would involve casting a structure to a pointer, which is a somewhat odd thing to do.

I think we can instead add a new structure containing the domain and pool indices and zone pointer, and embed VM_NFREEPOOL such structures into struct vm_domain.

sys/vm/vm_page.c
206 ↗(On Diff #59412)

Is there a code change linked to this comment change, or is this just fixing a broken comment?

markj marked an inline comment as done.Jul 4 2019, 8:49 PM
markj added inline comments.
sys/vm/vm_page.c
206 ↗(On Diff #59412)

I updated it because with this change may cache up to 256 * mp_ncpus * VM_NFREEPOOL pages per domain, and VM_NFREEPOOL is 2 on most platforms. I will change the calculation to apply the old limit across VM_NFREEPOOL zones.

markj marked an inline comment as done.
  • Allow no more than 0.25% of a domain's pages to be cached in any per-CPU cache.
  • Use a structure to provide parameters to the cache zones' import and release functions.
sys/vm/vm_page.c
190 ↗(On Diff #59426)

This struct type isn't used anywhere.

sys/vm/vm_page.c
192 ↗(On Diff #59426)

I believe it is impossible to get bit-width of the bitfield at compile time, which explains why you wrote the CTASSERT that way. Would it be better to change this to runtime assert, where you can e.g. assign -1 to pool and then compare it with VM_NFREEPOOL ?

1821 ↗(On Diff #59426)

() are excessive.

3507 ↗(On Diff #59426)

How is PG_PCPU_CACHE flag is ever cleared ? By flushing the zone, and then by vm_page_alloc() ?

Might be add a comment explaining this.

sys/vm/vm_pagequeue.h
107 ↗(On Diff #59426)

How is this member used (I did not found).

markj marked 2 inline comments as done.

Address feedback.

sys/vm/vm_page.c
192 ↗(On Diff #59426)

This is left over from a previous attempt. Now we do not need to pack the pool, so the KASSERT is no longer needed at all.

3507 ↗(On Diff #59426)

The flags field is reinitialized by vm_page_alloc_domain_after(), vm_page_alloc_contig_domain(), and vm_page_alloc_freelist_domain(). In particular, the flag remains set until the page is allocated again.

I added a comment above the definition of PG_CACHE_ALLOC.

sys/vm/vm_pagequeue.h
107 ↗(On Diff #59426)

In vm_page_zone_{import,release}() we have vmd = VM_DOMAIN(pgcache->domain).

This revision is now accepted and ready to land.Jul 5 2019, 7:19 PM
This revision was automatically updated to reflect the committed changes.