Page MenuHomeFreeBSD

Add a lockless lookup mechanism that uses a SMR zone.
ClosedPublic

Authored by jeff on Feb 1 2020, 1:31 AM.
Tags
None
Referenced Files
F83291481: D23446.diff
Wed, May 8, 12:18 PM
Unknown Object (File)
Mon, Apr 22, 12:25 PM
Unknown Object (File)
Mon, Apr 22, 3:09 AM
Unknown Object (File)
Feb 2 2024, 1:16 AM
Unknown Object (File)
Jan 31 2024, 9:28 AM
Unknown Object (File)
Jan 29 2024, 1:40 AM
Unknown Object (File)
Dec 20 2023, 4:59 AM
Unknown Object (File)
Nov 27 2023, 12:53 PM
Subscribers

Details

Summary

This converts radix to use SMR to allow for lockless traversal. This will rely on pages being type stable/NOFREE. The caller will have to verify the page after busying it. The caller is responsible for retrying, potentially with a lock, if the result is invalid or NULL.

This relies on ordering writes to tree updates so that the tree is always valid for a reader. Very few writes needed to be ordered. Before a new leaf is inserted the pointers within that leaf must be valid. Similarly on removal.

The lookup uses a smr section and atomic loads. Given the weak guarantees made to the caller some of the barriers may be overkill but should provide fewer false positives. Another option here is to have a single __always_inline lookup function that takes the load method as a parameter and is specialized for the locked/unlocked cases to reduce code duplication. I am open to suggestions.

I am going to let the SMR implementation settle in HEAD before I implement any consumers but I want to keep the patches moving. I need to solve one annoying startup issue that I hacked around with prealloc() for now.

Diff Detail

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

Event Timeline

jeff added reviewers: alc, dougm, kib, markj, rlibby.
jeff set the repository for this revision to rS FreeBSD src repository - subversion.

Use a common inline for lookup and provide a more direct path to making
other lookup functions lockless. Resolve startup ordering issues.

sys/vm/vm_radix.c
439–440 ↗(On Diff #67991)

This is an error.

I have some comment cleanup to commit and one uma/smr fix to commit and then I feel comfortable going forward with this.

Fix a copy & paste comment error. Clean up some wording.

sys/vm/vm_radix.c
63 ↗(On Diff #68089)

smr.h sorts before vmmeter.h.

346 ↗(On Diff #68089)

Why are you adding ZINIT? keg_alloc_slab() automatically zeroes items when they are first imported into a zone. Are you just making that requirement explicit?

375 ↗(On Diff #68089)

Why not use atomic_load here instead of a mix of volatile accesses and atomic stores?

389 ↗(On Diff #68089)

"synchronize" should be capitalized.

475 ↗(On Diff #68089)

Missing parens.

501 ↗(On Diff #68089)

This should be an atomic load.

sys/vm/vm_radix.c
346 ↗(On Diff #68089)

I wrote some comments that are now lost. I will try again.

I had forgotten about this historic wrinkle from the old zone allocator. If you notice radix was doing this explicitly. I switched to the flag just to drop the unnecessary function. Both disable a certain amount of trash checking that may be useful with smr. I think it's safe to drop.

375 ↗(On Diff #68089)

I don't really need the volatile. Notice that this pointer is never loaded. I just changed the type for convenience because our atomic macros are not * types.

This could use the PLAIN variant of getnode() and possibly reduce some confusion.

501 ↗(On Diff #68089)

As noted on our call, this is probably unnecessary and we can drop the loop. The race is handled in the caller and amd64 pmap may someday want to use the lockless lookup which won't work with this check. I had hoped to somewhat amortize the smr section cost but it is unlikely to matter.

779 ↗(On Diff #68089)

I could make a setnode(). That may be nicer.

784 ↗(On Diff #68089)

It occurs to me that SMR does not want this.

If the reader has fetched a pointer to this page the page may still be valid but we are NULL'ing here for assertion sake. Better to allow them to dereference and loop.

This revision is now accepted and ready to land.Feb 14 2020, 6:59 PM

This makes two major changes:

  1. It converts to the new smr type safe accessors. I am still somewhat

experimenting with the API here. radix isn't a great fit because you can't
assert about the lock in the object.

  1. It handles a case where lookup could return NULL because remove collapsed

a level. This is a little ugly but not terrible. It could be removed from
dtor to ctor where it's more likely to be used again while in cache.

This revision now requires review to proceed.Feb 16 2020, 9:32 AM
This revision is now accepted and ready to land.Feb 19 2020, 6:57 PM