If a function throws ENOSPC halfway through an operation, the
transaction will get aborted so that the filesystem isn't left in an
inconsistent state. This means that the mount will go read-only, and
that the user will lose any uncommitted changes.
So, it's better if we detect ENOSPC before starting an operation. I
tried this before with commit a52b73ed97 ("Check free space before
starting a transaction"), but the code was messy and it would have
required constant updates, which I neglected of course. The problem was
that I wanted to be as accurate as possible with my calculations of
required space, but I'm not sure there is any point.
Instead, require a large fixed amount of space (512 KiB) for most
operations, and a much smaller but still almost certainly sufficient
amount of space (80 KiB) for deletions. Hopefully this will be enough
for users to maneuver around ENOSPC situations. It's not perfect, but no
CoW filesystem can be truly perfect in this regard.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
I've been working on making some very needed changes to how node splits
work. The plan is to always rerun queries after a split, to keep the
query chain valid. One problem I've encountered while implementing this
is that the search key is not really preserved by apfs_update_inode().
We have a pointer to it, but it was allocated in the stack of some other
function, so the data is just noise.
I've always thought it was silly to allocate both a query and a key
before a lookup. Instead, turn the search key into a field within the
query structure, and copy it to the children as the query gets run. This
will ensure that the key remains available for future reruns.
One thing to note is that, while this simplifies most callers a little
bit, some of them still allocate a key in the stack and later copy it to
the query. I didn't want to make any big changes and risk introducing
bugs at this point, since I'm busy with other stuff.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Currently, compressed files that are stored directly in the decmpfs
xattr are handled separately: they get entirely uncompressed on open,
and on reads the contents simply get copied. To get rid of this code
duplication, use a new apfs_compressed_data struct to read from both
inline and dstream xattrs, and let the regular resource-compressed code
handle reads for both types.
These changes weren't made exclusively for code quality. A compressed
file may be too big to fit inside decmpfs, but still small enough to fit
inside a xattr. If that happens, the resulting resource fork will be an
inline xattr, and the dstream read code implemented with e4bb06ecef
("Avoid reading whole compressed files into memory") will break down. So
the apfs_compressed_data struct is strictly necessary for the resource
case; using the same struct for both cases simplifies testing, because
inline resource compressed files are rare (I've seen them, but right now
I don't even have an image with one of them).
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
I wrote apfs_xattr_get_dstream() long before error messages were changed
in d4797823ea ("Improve error reporting"), but the code wasn't
actually merged until recently, and I forgot to update the error
messages to our current standards. Fix that now.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Currently, compressed files saved in xattrs are read into memory all at
once, and later decompressed as needed. This has always required a large
memory allocation, which can be a problem in general, but especially for
32-bit archs which can't request more than 128Mb from vmalloc(). As
mentioned in commit 1e3fa28c4f ("Increase limit for compressed size
of files"), we are beginning to encounter files bigger than that limit.
Using the new apfs_nonsparse_dstream_read() to read from arbitrary xattr
offsets, rework the compressed reads so that we only allocate memory for
one compressed block at a time. For now the change is only for resource
xattrs, since I don't expect inline xattrs to ever be this big.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Lately I'm encountering more and more compressed files that are too
large for their xattrs to be read into memory all at once, so I'll need
to be able to read xattrs from arbitrary offsets. The code for this is
very similar to what's already inside apfs_xattr_extents_read(), so spin
it into a new apfs_nonsparse_dstream_read() with a generic interface
that can later be reused from compress.c.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
The build appears to be broken for some configurations on the Linux 6.5
release candidate:
https://github.com/linux-apfs/linux-apfs-rw/issues/49
Some sort of static overflow check is going on for strcpy(), and several
of my calls are triggering false positives. Just be safe and switch them
all to strscpy(). The qstr stuff in particular looked a bit too weird
anyway.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Always free the allocated dstream in apfs_xattr_extents_read() before
returning.
Fixes: 362c4e32ab ("Allow internal reads of huge xattrs")
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
The driver is much closer to being usable, so I might start getting
subtler bug reports soon. To make them easier to handle, put error
messages all over the place. I should have done this from the beginning,
but I guess I didn't fully understand the need back then.
From now my general policy will be to use apfs_warn() for user errors or
unsupported features; apfs_err() for things that are probably corruption
or io errors; and apfs_alert() for things that are most likely bugs.
These last two should be rare, so the same error/alert will be thrown by
several layers in the callstack to provide as much information as
possible. Be careful and don't flood the console on normal situations.
Also, make messages with a log level lower than warning output their
function name and line number, which I think will help debugging more
than the actual messages.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Implement the FICLONE ioctl to be able to create clones of files. This
will never work the same as macos, because Linux requires that the user
first creates the target file, and only then it can be made into a
clone. A crash halfway throught would leave an empty file in place of
the clone.
This patch only duplicates the data stream, it does not deal with the
actual extents. Reads from clones should work correctly, but writes will
not. I'll get to that right away.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
The previous commit has caused regressions for a couple of mmap tests
from xfstests. I always assumed that all buffer heads mapping a physical
block would be the same object, with the same metadata, but that's only
true for buffer heads in the same mapping. So the apfs_sb_bread() inside
apfs_zero_dstream_tail() will not get the same bh that joined the
transaction via mmap, and two separate CoWs will happen, losing some of
the writes.
So, first of all, apfs_zero_dstream_tail() can't just read blocks from
the device like it used to, it needs to have a reference to the file
mapping and work with that. Put a pointer to the inode in the dstream
info for this purpose. Now, since individual buffer tails are already
zeroed during ->write_begin(), spin the part of that code that doesn't
deal with transactions into __apfs_write_begin() and __apfs_write_end(),
and use those here. Finally, reorder apfs_dstream_get_new_block() to
avoid problems with the extent cache during this recursion.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Milan P. Stanić reports via email that the submit_bh() api has changed
in Linux 6.0, and the driver no longer builds. Implement a simple macro
wrapper called apfs_submit_bh() to fix this without littering the code
with version checks.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
I'm trying to build with "-W" for the first time, and it reports that
apfs_free_query() doesn't use the 'sb' parameter. Get rid of it.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Add a 'data' field to in-memory objects and point it to the buffer
head's 'b_data'; from now on, always access the data through this field
instead of the bh. To make sure I remembered to clean up all instances,
rename the buffer head in the object from 'bh' to 'o_bh'.
The point is to allow nodes to work with a data block that exists only
in memory, and the next patch will take advantage of this to improve
apfs_node_split(). Since objects that are not nodes will not need this
feature, only adjust the cleanup done in apfs_node_release() for now,
though maybe object cleanups should all be done in one place.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Tweak apfs_dstream_get_new_block() so that it can be called without a
buffer head, and use this to get rid of the hacky fake bhs currently
used by apfs_create_xattr_dstream().
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Implement apfs_logic_to_phys_bno() to translate logical block addresses
into physical ones without going through __apfs_get_block(). Use this to
avoid the hacky use of fake buffer heads when all we want is the block
number.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Currently apfs_xattr_extents_read() loops once through all the blocks in
the dstream, reading each of them from disk and copying it to the
destination buffer right away. It's much more effective to do IO for
several blocks at once, similar to the readahead used by regular file
dstreams.
So use two loops instead: one to dispatch the read requests, and a
second to wait for each of them and copy the results to the buffer.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
The Linux xattr syscalls don't support values of more than 64k, but the
module still needs to use bigger xattrs internally to support compressed
files. When I refactored the xattr code in commit 9b71ea9b0f ("Use the
common dstream code to read dstream xattrs"), it seems that I thought I
could simplify an overflow check on the xattr size by simply checking
against this upper limit, but that broke compressed reads.
Revert the bad change, and put the xattr size check directly inside the
apfs_xattr_get() handler, where it won't bother internal code.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
A run of generic/013 of xfstests eventually corrupts a node's metadata,
later damaging the records it contains. The problem is triggered on the
special case that dstream xattr deletion and the dstream truncation it
requires both affect the same node. Since the query that found the xattr
is still in use while the dstream code spins its own queries, the node
structs in the original query will go stale, and eventually write back
that stale metadata to disk.
The problem is of course, that queries must never be nested, so rework
the code to avoid it. I should think of adding some metadata to queries
on debug builds, so assertions can be put in place to prevent this kind
of time-consuming mistakes.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Using the dstream code shared with inodes, implement creation and
deletion of large xattrs that don't fit inline. This is needed to be
able to run some of xfstests, for example generic/013, which attempts
to create symlinks to very long paths, currently aborting the
transaction.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Before we actually implement creation and deletion of dstream xattrs,
first convert the existing (read-only) functionality to use the common
code provided by the last patch.
The code is a little hacky in that it uses temporary buffer heads that
are immediately discarded. It could be done better, but I don't want to
put much effort into it because I plan to get rid of buffer heads soon.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
