Readers can read a set of items that is stale with respect to items that
were dirtied and written under a local cluster lock after the read
started.
The active reader machanism addressed this by refusing to shrink pages
that could contain items that were dirtied while any readers were in
flight. Under the right circumstances this can result in refusing to
shrink quite a lot of pages indeed.
This changes the mechanism to allow pages to be reclaimed, and instead
forces stale readers to retry. The gamble is that reads are much faster
than writes. A small fraction should have to retry, and when they do
they can be satisfied by the block cache.
Signed-off-by: Zach Brown <zab@versity.com>
scoutfs_item_write_done() acquires the cinf dirty_lock and pg rwlock out
of order. It uses a trylock to detect failure and back off of both
before retrying.
sparse seems to have some peculiar sensitivity to following the else
branch from a failed trylock while already in a context. Doing that
consistently triggered the spurious mismatched context warning.
This refactors the loop to always drop and reacquire the dirty_lock
after attemping the trylock. It's not great, but this shouldn't be very
contended because the transaction write has serialized write lock
holderse that would be trying to dirty items. The silly lock noise will
be mostly cached.
Signed-off-by: Zach Brown <zab@versity.com>
v5.19-rc4-52-ge33c267ab70d Adds shrinker names to the registration
call to aid with shrinker debugging, which is highly opaque.
To enable you'll have to recompile the kernel with
CONFIG_SHRINKER_DEBUG=y though, since it's disabled by default in
OSV kernels.
Signed-off-by: Auke Kok <auke.kok@versity.com>
v5.12-rc6-9-g4f0f586bf0c8
All list_sort functions use the list_cmp_func_t type, which compares
list_head member types. These are now required to be `const` as the
compiler will now check them. This propagates into our callers.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Add a lookup variant that returns an error if the item value is larger
than the caller's value buffer size and which zeros the rest of the
caller's buffer if the returned value is smaller.
Signed-off-by: Zach Brown <zab@versity.com>
The item cache has a bit of safety checks that make sure that an
operation is performed while holding a lock that covers the item. It
dumped a stack trace via WARN when that wasn't true, but it didn't
include any details about the keys or lock modes involved.
This adds a message that's printed once which includes the keys and
modes when an operation is attempted that isn't protected.
Signed-off-by: Zach Brown <zab@versity.com>
scoutfs_item_create() was checking that its lock had a read mode, when
it should have been checking for a write mode. This worked out because
callers with write mode locks are also protecting reads.
Signed-off-by: Zach Brown <zab@versity.com>
v4.9-12228-g530e9b76ae8f Drops all (un)register_(hot)cpu_notifier()
API functions. From here on we need to use the new cpuhp_* API.
We avoid this entirely for now, at the cost of leaking pages until
the filesystem is unmounted.
Signed-off-by: Auke Kok <auke.kok@versity.com>
Move to the more recent interfaces for counting and scanning cached
objects to shrink.
Signed-off-by: Zach Brown <zab@versity.com>
Signed-off-by: Auke Kok <auke.kok@versity.com>
FS items are deleted by logging a deletion item that has a greater item
version than the item to delete. The versions are usually maintained by
the write_seq of the exclusive write lock that protects the item. Any
newer write hold will have a greater version than all previous write
holds so any items created under the lock will have a greater vers than
all previous items under the lock. All deletion items will be merged
with the older item and both will be dropped.
This doesn't work for concurrent write-only locks. The write-only locks
match with each other so their write_seqs are asssigned in the order
that they are granted. That grant order can be mismatched with item
creation order. We can get deletion items with lesser versions than the
item to delete because of when each creation's write-only lock was
granted.
Write only locks are used to maintain consistency between concurrent
writers and readers, not between writers. Consistency between writers
is done with another primary write lock. For example, if you're writing
seq items to a write-only region you need to have the write lock on the
inode for the specific seq item you're writing.
The fix, then, is to pass these primary write locks down to the item
cache so that it can chose an item version that is the greatest amongst
the transaction, the write-only lock, and the primary lock. This now
ensures that the primary lock's increasing write_seq makes it down to
the item, bringing item version ordering in line with exclusive holds of
the primary lock.
All of this to fix concurrent inode updates sometimes leaving behind
duplicate meta_seq items because old seq item deletions ended up with
older versions than the seq item they tried to delete, nullifying the
deletion.
Signed-off-by: Zach Brown <zab@versity.com>
The max_seq and active reader mechanisms in the item cache stop readers
from reading old items and inserting them in the cache after newer items
have been reclaimed by memory pressure. The max_seq field in the pages
must reflect the greatest seq of the items in the page so that reclaim
knows that the page contains items newer than old readers and must not
be removed.
We update the page max_seq as items are inserted or as they're dirtied
in the page. There's an additional subtle effect that the max_seq can
also protect items which have been erased. Deletion items are erased
from the pages as a commit completes. The max_seq in that page will
still protect it from being reclaimed even though no items have that seq
value themselves.
That protection fails if the range of keys containing the erased item is
moved to another page with a lower max_seq. The item mover only
updated the destination page's max_seq for each item that was moved. It
missed that the empty space between the items might have a larger
max_seq from an erased item. We don't know where the erased item is so
we have to assume that a larger max_seq in the source page must be set
on the destination page.
This could explain very rare item cache corruption where nodes were
seeing deleted directory entry items reappearing. It would take a
specific sequence of events involving large directories with an isolated
removal, a delayed item cache reader, a commit, and then enough
insertions to split the page all happening in precisely the wrong
sequence.
Signed-off-by: Zach Brown <zab@versity.com>
Add the .totl. xattr tag. When the tag is set the end of the name
specifies a total name with 3 encoded u64s separated by dots. The value
of the xattr is a u64 that is added to the named total. An ioctl is
added to read the totals.
Signed-off-by: Zach Brown <zab@versity.com>
The fs log btrees have values that start with a header that stores the
item's seq and flags. There's a lot of sketchy code that manipulates
the value header as items are passed around.
This adds the seq and flags as core item fields in the btree. They're
only set by the interfaces that are used to store fs items: _insert_list
and _merge. The rest of the btree items that use the main interface
don't work with the fields.
This was done to help delta items discover when logged items have been
merged before the finalized lob btrees are deleted and the code ends up
being quite a bit cleaner.
Signed-off-by: Zach Brown <zab@versity.com>
The forest reader reads items from the fs_root and all log btrees and
gives them to the caller who tracks them to resolve version differences.
The reads can run into stale blocks which have been overwritten. The
forest reader was implementing the retry under the item state in the
caller. This can corrupt items that are only seen firest in an old fs
root before a merge and then only seen in the fs_root after a merge. In
this case the item won't have any versioning and the existing version
from the old fs_root is preferred. This is particularly bad when the
new version was deleted -- in that case we have no metadata which would
tell us to drop the old item that was read from the old fs_root.
This is fixed by pushing the retry up to callers who wipe the item state
before each retry. Now each set of items is related to a single
snapshot of the fs_root and logs at one point in time.
I haven't seen definitive evidence of this happening in practice. I
found this problem after putting on my craziest thinking toque and
auditing the code for places where we could lose item updates.
Signed-off-by: Zach Brown <zab@versity.com>
The item cache has to be careful not to insert stale read items when
previously dirty items have been written and invalidated while a read
was in flight.
This was previously done by recording the possible range of items that a
reader could see based on the key range of its lock. This is
disasterous when a workload operates entirely within one lock. I ran
into this when testing a small number of files with massive amounts of
xattrs. While any reader is in flight all pages can't be invalidated
because they all intersect with the one lock that covers all the items
in use.
The fix is to more naturally reflect the problem by tracking the
greatest item seq in pages and the earliest seq that any readers
can't see. This lets invalidate only skip pages with items
that weren't visible to the earliest reader.
This more naturally reflects that the problem is due to the age of the
items, not their position in the key space. Now only a few of the most
recently modified pages could be skipped and they'll be at the end
of the LRU and won't typically be visited. As an added benefit it's
now much cheaper to add, delete, and test the active readers.
This fix stopped rm -rf of a full system's worth of xattrs from taking
minutes constantly spinning skipping all pages in the LRU to seconds of
doing real removal work.
Signed-off-by: Zach Brown <zab@versity.com>
Some item_val_len() callers were applying alignment twice, which isn't
needed.
And additions to erased_bytes as value lengths change didn't take
alignment into account. They could end up double counting if val_len
changes within the alignment are then accounted for again as the full
item and alignment is later deleted. Additions to erased_bytes based on
val_len should always take alignment into account.
Signed-off-by: Zach Brown <zab@versity.com>
The item cache allocates a page and a little tracking struct for each
cached page. If the page allocation fails it might try to free a null
page pointer, which isn't allowed.
Signed-off-by: Zach Brown <zab@versity.com>
Rename the item version to seq and set it to the max of the transaction
seq and the lock's write_seq. This lets btree item merging chose a seq
at which all dirty items written in future commits must have greater
seqs. It can drop the seqs from items written to the fs tree during
btree merging knowing that there aren't any older items out in
transactions that could be mistaken for newer items.
Signed-off-by: Zach Brown <zab@versity.com>
Rename the write_version lock field to write_seq and get it from the
core seq in the super block.
We're doing this to create a relationship between a client transaction's
seq and a lock's write_seq. New transactions will have a greater seq
than all previously granted write locks and new write locks will have a
greater seq than all open transactions. This will be used to resolve
ambiguities in item merging as transaction seqs are written out of order
and write locks span transactions.
Signed-off-by: Zach Brown <zab@versity.com>
The item cache page life cycle is tricky. There are no proper page
reference counts, everthing is done by nesting the page rwlock inside
item_cache_info rwlock. The intent is that you can only reference pages
while you hold the rwlocks appropriately. The per-cpu page references
are outside that locking regime so they add a reference count. Now
there are reference counts for the main cache index reference and for
each per-cpu reference.
The end result of all this is that you can only reference pages outside
of locks if you're protected by references.
Lock invalidation messed this up by trying to add its right split page
to the lru after it was unlocked. Its page reference wasn't protected
at this point. Shrinking could be freeing that page, and so it could be
putting a freed page's memory back on the lru.
Shrinking had a little bug that it was using list_move to move an
initialized lru_head list_head. It turns out to be harmless (list_del
will just follow pointers to itself and set itself as next and prev all
over again), but boy does it catch one's eye. Let's remove all
confusion and drop the reference while holding the cinf->rwlock instead
of trying to optimize freeing outside locks.
Finally, the big one: inserting a read item after compacting the page to
make room was inserting into stale parent pointers into the old
pre-compacted page, rather than the new page that was swapped in by
compaction. This left references to a freed page in the page rbtree and
hilarity ensued.
Signed-off-by: Zach Brown <zab@versity.com>
Dirty items in a client transaction are stored in OS pages. When the
transaction is committed each item is stored in its position in a dirty
btree block in the client's existing log btree. Allocators are refilled
between transaction commits so a given commit must have sufficient meta
allocator space (avail blocks and unused freed entries) for all the
btree blocks that are dirtied.
The number of btree blocks that are written, thus the number of cow
allocations and frees, depends on the number of blocks in the log btree
and the distribution of dirty items amongst those blocks. In a typical
load items will be near each other and many dirty items in smaller
kernel pages will be stored in fewer larger btree blocks.
But with the right circumstances, the ratio of dirty pages to dirty
blocks can be much smaller. With a very large directory and random
entry renames you can easily have 1 btree block dirtied for every page
of dirty items.
Our existing allocator meta allocator fill targets and the number of
dirty item cache pages we allowed did not properly take this in to
account. It was possible (and, it turned out, relatively easy to test
for with a hgue directory and random renames) to run out of meta avail
blocks while storing dirty items in dirtied btree blocks.
This rebalances our targets and thresholds to make it more likely that
we'll have enough allocator resources to commit dirty items. Instead of
having an arbitrary limit on the number of dirty item cache pages, we
require that a given number of dirty item cache pages have a given
number of allocator blocks available.
We require a decent number of avialable blocks for each dirty page, so
we increase the server's target number of blocks to give the client so
that it can still build large transactions.
This code is conservative and should not be a problem in practice, but
it's theoretically possible to build a log btree and set of dirty items
that would dirty more blocks that this code assumes. We will probably
revisit this as we add proper support for ENOSPC.
Signed-off-by: Zach Brown <zab@versity.com>
Prefer named to anonymous enums. This helps readability a little.
Use enum as param type if possible (a couple spots).
Remove unused enum in lock_server.c.
Define enum spbm_flags using shift notation for consistency.
Rename get_file_block()'s "gfb" parameter to "flags" for consistency.
Signed-off-by: Andy Grover <agrover@versity.com>
Write locks are given an increasing version number as they're granted
which makes its way into items in the log btrees and is used to find the
most recent version of an item.
The initialization of the lock server's next write_version for granted
locks dates back to the initial prototype of the forest of log btrees.
It is only initialized to zero as the module is loaded. This means that
reloading the module, perhaps by rebooting, resets all the item versions
to 0 and can lead to newly written items being ignored in favour of
older existing items with greater versions from a previous mount.
To fix this we initialize the lock server's write_version to the
greatest of all the versions in items in log btrees. We add a field to
the log_trees struct which records the greatest version which is
maintained as we write out items in transactions. These are read by the
server as it starts.
Then lock recovery needs to include the write_version so that the
lock_server can be sure to set the next write_version past the greatest
version in the currently granted locks.
Signed-off-by: Zach Brown <zab@versity.com>
This will ensure structs, which are internally 8 byte aligned, will remain
so when in the item cache.
16 bytes alignment doesn't seem like it's needed so just do 8.
Signed-off-by: Andy Grover <agrover@versity.com>
The algorithm for choosing the split key assumed that there were
multiple items in the page. That wasn't always true and it could result
in choosing the first item as the split key, which could end up
decrementing the left page's end key before it's start key.
We've since added compaction to the paths that split pages so we now
guarantee that we have at least two items in the page being split. With
that we can be sure to use the second item's key and ensure that we're
never creating invalid keys for the pages created by the split.
Signed-off-by: Zach Brown <zab@versity.com>
The tests for the various page range intersections were out of order.
The edge overlap case could trigger before the bisection case and we'd
fail to remove the initial items in the page. That would leave items
before the start key which would later be used as a midpoint for a
split, causing all kinds of chaos.
Rework the cases so that the overlap cases are last. The unique bisect
case will be caught before we can mistake it for an edge overlap case.
And minimize the number of comparisons we calculate by storing the
handful that all the cases need.
Signed-off-by: Zach Brown <zab@versity.com>
The first pass of the item cache didn't try to reclaim freed space at
all. It would leave behind very sparse pages. The oldest of which
would be reclaimed by memory pressure.
While this worked, it created much more stress on the system than is
necessary. Splitting a page with one key also makes it hard to
calculate the boundaries of the split pages, given that the start and
end keys could be the single item.
This adds a header field which tracks the free space in item cache
pgaes. Free space is created before the alloc offset by removing items
from the rbtree, but also from shrinking item values when updating or
deleting items.
If we try to split a page with sufficient free space to insert the
largest possible item then we compact the page instead of splitting it.
We copy the items into the front of an unused page and swap the pages.
Signed-off-by: Zach Brown <zab@versity.com>
Add a quick function that walks the rbtree and makes sure it doesn't see
any obvious key errors. This is far too expensive to use regularly but
it's handy to have around and add calls to when debugging.
Signed-off-by: Zach Brown <zab@versity.com>
Add an item cache between fs callers and the forest of btrees. Calling
out to the btrees for every item operation was far too expensive. This
gives us a flexible in-memory structure for working with items that
isn't bound by the constrants of persistent block IO. We can rarely
stream large groups of items to and from the btrees and then use
efficient kernel memory structures for more frequent item operations.
This adds the infrastructure, nothing is calling it yet.
Signed-off-by: Zach Brown <zab@versity.com>
Convert client locking to call the server's lock service instead of
using a fs/dlm lockspace.
The client code gets some shims to send and receive lock messages to and
from the server. Callers use our lock mode constants instead of the
DLM's.
Locks are now identified by their starting key instead of an additional
scoped lock name so that we don't have more mapping structures to track.
The global rename lock uses keys that are defined by the format as only
used for locking.
The biggest change is in the client lock state machine. Instead of
calling the dlm and getting callbacks we send messages to our server and
get called from incoming message processing. We don't have everything
come through a per-lock work queue. Instead we send requests either
from the blocking lock caller or from a shrink work queue. Incoming
messages are called in the net layer's blocking work contexts so we
don't need to do any more work to defer to other contexts.
The different processing contexts leads to a slightly different lock
life cycle. We refactor and seperate allocation and freeing from
tracking and removing locks in data structures. We add a _get and _put
to track active use of locks and then async references to locks by
holders and requests are tracked seperately.
Our lock service's rules are a bit simpler in that we'll only ever send
one request at a time and the server will only ever send one request at
a time. We do have to do a bit of work to make sure we process back to
back grant reponses and invalidation requests from the server.
As of this change the lock setup and destruction paths are a little
wobbly. They'll be shored up as we add lock recovery between the client
and server.
Signed-off-by: Zach Brown <zab@versity.com>
Add scoutfs_item_prev() for searching for an item before a given key.
This wasn't initially implemented because it's rarely needed and for a
long time the segment reading and item cache populating code had a
strong bias for iterating forward from the given search key.
Since we've added limiting item cache reading to the keys covered by
locks and reading in entire segments it's now very easy to iterate
backwards through keys just like scoutfs_item_next() iterates forwards.
The only remaining forward iteration bias was in check_range(). It had
to give callers the start of the cached range that it found.
Signed-off-by: Zach Brown <zab@versity.com>
Items deleted from the item cache would always write deletion items to
segments. We need to write deletion items so that compaction can
eventually combine them with the existing item and remove both. We
don't need them for items that were only created in the current
transaction. Writing a deletion item for them only results in a lot of
extra work compacting the item down to the final segment level so that
it can be removed.
The upcoming extent code really demonstrated the cost of this overhead.
It happens to create and delete quite a lot of temporary extent items
during the transaction as all the different kinds of indexed extents
change.
This change tracks whether a given item in the cache reflects an item
that is present in the persistent storage. This lets us free items
that have only existed in the current transaction.
This made a meaningful difference when writing a 4MB file with the
current block mapping items, but it made an enormous difference when
writing that same file with the extent items. It went from writing 1024
deletion items for 11 real items to only writing those real items.
items deletions
block mappings before: 25 5
block mappings after: 25 0
extents before: 11 1024
extents after: 11 0
Signed-off-by: Zach Brown <zab@versity.com>
Variable length keys lead to having a key struct point to the buffer
that contains the key. With dirents and xattrs now using small keys we
can convert everyone to using a single key struct and significantly
simplify the system.
We no longer have a seperate generic key buf struct that points to
specific per-type key storage. All items use the key struct and fill
out the appropriate fields. All the code that paired a generic key buf
struct and a specific key type struct is collapsed down to a key struct.
There's no longer the difference between a key buf that shares a
read-only key, has it's own precise allocation, or has a max size
allocation for incrementing and decrementing.
Each key user now has an init function fills out its fields. It looks a
lot like the old pattern but we no longer have seperate key storage that
the buf points to.
A bunch of code now takes the address of static key storage instead of
managing allocated keys. Conversely, swapping now uses the full keys
instead of pointers to the keys.
We don't need all the functions that worked on the generic key buf
struct because they had different lengths. Copy, clone, length init,
memcpy, all of that goes away.
The item API had some functions that tested the length of keys and
values. The key length tests vanish, and that gets rid of the _same()
call. The _same_min() call only had one user who didn't also test for
the value length being too large. Let's leave caller key constraints in
callers instead of trying to hide them on the other side of a bunch of
item calls.
We no longer have to track the number of key bytes when calculating if
an item population will fit in segments. This removes the key length
from reservations, transactions, and segment writing.
The item cache key querying ioctls no longer have to deal with variable
length keys. The simply specify the start key, the ioctls return the
number of keys copied instead of bytes, and the caller is responsible
for incrementing the next search key.
The segment no longer has to store the key length. It stores the key
struct in the item header.
The fancy variable length key formatting and printing can be removed.
We have a single format for the universal key struct. The SK_ wrappers
that bracked calls to use preempt safe per cpu buffers can turn back
into their normal calls.
Manifest entries are now a fixed size. We can simply split them between
btree keys and values and initialize them instead of allocating them.
This means that level 0 entries don't have their own format that sorts
by the seq. They're sorted by the key like all the other levels.
Compaction needs to sweep all of them looking for the oldest and read
can stop sweeping once it can no longer overlap. This makes rare
compaction more expensive and common reading less expensive, which is
the right tradeoff.
Signed-off-by: Zach Brown <zab@versity.com>
Originally the item interfaces were written with full support for
vectored keys and values. Callers constructed keys and values made up
of header structs and data buffers. Segments supported much larger
values which could span pages when stored in memory.
But over time we've pulled that support back. Keys are described by a
key struct instead of a multi-element kvec. Values are now much smaller
and don't span pages. The item interfaces still use the kvec arrays but
everyone only uses a single element.
So let's make the world a whole lot less awful but having the item
interfaces only supporting a single value buffer specified by a kvec. A
bunch of code disappears and the result is much easier to understand.
Signed-off-by: Zach Brown <zab@versity.com>
Every caller of scoutfs_item_lookup_exact() provided a size that matches
the value buffer. Let's remove the redundant arg and use the value
buffer length as the exact size to match.
Signed-off-by: Zach Brown <zab@versity.com>
Iterating over items backwards would result in a lot of extra work.
When an item isn't present in the cache we go and search the segments
for the item. Once we find the item in its stack of segments we also
read in and cache all the items from the missing item to the end of all
the segments.
This reduced complexity a bit but had very bad worst case performance.
If you read items backwards you constantly get cache misses that each
search the segments for the item and then try to cache everything to the
end of the segment. You're essentially working uncached and are doing
quite a lot of work to get that single missed item cached each time.
This adds the complexity to cache all the items in the segment stack
around the missed item, not just after the missed item. Now reverse
iteration hits cached items for everything in the segment after the
initial miss.
To make this work we have to pass the full lock coverage range to the
item reading path. Then we search the manifest for segments that
contain the missing key and use those segment's ranges to determine the
full range of items that we'll cache. Then we again search the manifest
for all the level 0 segments that intersect that range.
That range extension is only for cached reads, it doesn't apply to the
'next' call which ignores caching. That operation is getting different
enough that we pull it out into its own function.
Signed-off-by: Zach Brown <zab@versity.com>
scoutfs_item_set_batch() has a rocky history of being a giant pain in
the butt. It's been a lot simpler to have callers use individual item
ops instead of trying to describe a compound item operation to sometihng
like _set_batch().
Its last user has gone away so we can remove it and never speak of it
again. And there was much rejoycing.
Signed-off-by: Zach Brown <zab@versity.com>
Add item cache functions for saving and restoring items. This lets
callers more easily undo changes while they have transactions pinned.
Signed-off-by: Zach Brown <zab@versity.com>
We had an absolute ton of open coding of testing an item's dirty flag.
Let's hide it off in a helper so we're less likely to mess it up.
Signed-off-by: Zach Brown <zab@versity.com>
It was silly to hand off deleted values to callers to free. We can just
free as we delete and save a bunch of caller value manipulation.
Signed-off-by: Zach Brown <zab@versity.com>
Some callers may want to just test if an item is present and not
necessarily want to setup storage for copying the value in.
Signed-off-by: Zach Brown <zab@versity.com>
Auke Kok
Zach Brown
Andy GroverandZach Brown
