ANDROID: Incremental fs: Retry page faults on non-fatal errors

In order to not freeze on corrupt data, we need to turn off
FAULT_FLAG_ALLOW_RETRY. However, this means we no longer retry on EINTR,
so an interrupted read will lead to page faults.

The fault handler does not seem to allow dynamic decisions as to whether
to turn on or off this flag.

To resolve both issues, add a flag to indicate if there are corrupt
pages in a file, and only if there are turn off this flag.

Also fsanitize changed the behavior of mlock - mlock should fail if the
page reads fail, but with fsanitize it returns 0 then page faults on
access. This broke this test, and fsanitize offers little value on test
code, so disable it.

Test: incfs_test passes
Bug: 343532239
Change-Id: Id2ced4be3310109206d65dcc92dea05c05131182
Signed-off-by: Paul Lawrence <paullawrence@google.com>
This commit is contained in:
Paul Lawrence 2024-06-19 12:44:38 -07:00
parent 686e8cd4df
commit 08892fdf71
4 changed files with 38 additions and 3 deletions

View File

@ -314,6 +314,11 @@ struct backing_file_context {
* there is no need to get/put the creds
*/
const struct cred *bc_cred;
/*
* The file has a bad block, i.e. one that has failed checksumming.
*/
bool bc_has_bad_block;
};
struct metadata_handler {

View File

@ -35,6 +35,7 @@ DECLARE_FEATURE_FLAG(zstd);
DECLARE_FEATURE_FLAG(v2);
DECLARE_FEATURE_FLAG(bugfix_throttling);
DECLARE_FEATURE_FLAG(bugfix_inode_eviction);
DECLARE_FEATURE_FLAG(bugfix_retry_page_fault);
static struct attribute *attributes[] = {
&corefs_attr.attr,
@ -42,6 +43,7 @@ static struct attribute *attributes[] = {
&v2_attr.attr,
&bugfix_throttling_attr.attr,
&bugfix_inode_eviction_attr.attr,
&bugfix_retry_page_fault_attr.attr,
NULL,
};

View File

@ -115,7 +115,28 @@ static const struct address_space_operations incfs_address_space_ops = {
static vm_fault_t incfs_fault(struct vm_fault *vmf)
{
vmf->flags &= ~FAULT_FLAG_ALLOW_RETRY;
struct file *file = vmf->vma->vm_file;
struct data_file *df = get_incfs_data_file(file);
struct backing_file_context *bfc = df ? df->df_backing_file_context : NULL;
/*
* This is something of a kludge
* We want to retry if the read from the underlying file is interrupted,
* but not if the read fails because the stored data is corrupt since the
* latter causes an infinite loop.
*
* However, whether we wish to retry must be set before we call
* filemap_fault, *and* there is no way of getting the read error code out
* of filemap_fault.
*
* So unless there is a robust solution to both the above problems, we can
* solve the actual issues we have encoutered by retrying unless there is
* known corruption in the backing file. This does mean that we won't retry
* with a corrupt backing file if a (good) read is interrupted, but we
* don't really handle corruption well anyway at this time.
*/
if (bfc && bfc->bc_has_bad_block)
vmf->flags &= ~FAULT_FLAG_ALLOW_RETRY;
return filemap_fault(vmf);
}
@ -581,6 +602,13 @@ static int read_single_page(struct file *f, struct page *page)
else if (read_result < PAGE_SIZE)
zero_user(page, read_result, PAGE_SIZE - read_result);
if (result == -EBADMSG) {
struct backing_file_context *bfc = df ? df->df_backing_file_context : NULL;
if (bfc)
bfc->bc_has_bad_block = 1;
}
if (result == 0)
SetPageUptodate(page);
else

View File

@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS += -D_FILE_OFFSET_BITS=64 -Wall -Werror -I../.. -I../../../../.. -fno-omit-frame-pointer -fsanitize=address -g
LDLIBS := -llz4 -lzstd -lcrypto -lpthread -fsanitize=address
CFLAGS += -D_FILE_OFFSET_BITS=64 -Wall -Wno-deprecated-declarations -Werror -I../.. -I../../../../.. -fno-omit-frame-pointer -g
LDLIBS := -llz4 -lzstd -lcrypto -lpthread
TEST_GEN_PROGS := incfs_test incfs_stress incfs_perf
include ../../lib.mk