Skip to content

daxfs: Read holes as zeros instead of returning short #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
congwang-mk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

daxfs: Read holes as zeros instead of returning short #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 69 additions & 18 deletions daxfs/file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ static void daxfs_refresh_isize(struct inode *inode, struct daxfs_info *info)
* If data comes from pcache, the slot is pinned (refcount incremented).
* Caller MUST call daxfs_pcache_put_page(info, *pinned_slot) when done.
* pinned_slot is set to -1 when data does not come from pcache.
*
* Returns a pointer to the data, NULL if the range is a hole (nothing in the
* overlay and nothing behind it in the base image), or ERR_PTR on failure.
* The two are not interchangeable: a hole reads as zeros, an error must not.
*/
/*
* Look up an overlay page, checking the per-inode DRAM cache first.
Expand Down Expand Up @@ -133,7 +137,7 @@ void *daxfs_base_file_data(struct daxfs_info *info,

page = daxfs_pcache_get_page(info, ino, pgoff, pinned_slot);
if (IS_ERR(page))
return NULL;
return ERR_CAST(page);
intra = pcache_off & (PAGE_SIZE - 1);
if (out_len)
*out_len = min(len, (size_t)(PAGE_SIZE - intra));
Expand Down Expand Up @@ -246,9 +250,29 @@ static ssize_t daxfs_read_iter(struct kiocb *iocb, struct iov_iter *to)

src = daxfs_base_file_data(info, inode, pos, count,
&chunk, &pcslot);
if (IS_ERR(src))
return total ? total : PTR_ERR(src);

if (!src || chunk == 0) {
/*
* A hole inside i_size: nothing in the overlay and
* nothing behind it, which is what a file extended by
* truncate looks like. POSIX reads that as zeros;
* stopping here instead reports a false EOF mid-file.
* Zero a page at a time so data after the hole is
* still found on the next iteration.
*/
size_t hole = min(count, (size_t)(PAGE_SIZE -
(pos & (PAGE_SIZE - 1))));

daxfs_pcache_put_page(info, pcslot);
break;
if (iov_iter_zero(hole, to) != hole)
return total ? total : -EFAULT;

pos += hole;
count -= hole;
total += hole;
continue;
}

if (daxfs_copy_to_iter(info, src, chunk, to) != chunk) {
Expand Down Expand Up @@ -330,6 +354,15 @@ static void daxfs_write_prealloc(struct daxfs_info *info, struct inode *inode,
base_data = daxfs_base_file_data(info, inode,
(loff_t)pgoff << PAGE_SHIFT,
PAGE_SIZE, &base_len, &pcslot);
if (IS_ERR(base_data)) {
/*
* Leave the page unpublished rather than COW
* zeros over data we failed to read. The
* per-page path retries and reports the error.
*/
pages[i] = NULL;
continue;
}
if (base_data && base_len > 0)
memcpy(page, base_data,
min(base_len, (size_t)PAGE_SIZE));
Expand Down Expand Up @@ -417,6 +450,9 @@ static ssize_t daxfs_write_iter(struct kiocb *iocb, struct iov_iter *from)
inode,
(loff_t)pgoff << PAGE_SHIFT,
PAGE_SIZE, &base_len, &pcslot);
if (IS_ERR(base_data))
return total ? total :
PTR_ERR(base_data);
if (base_data && base_len > 0)
memcpy(page, base_data,
min(base_len, (size_t)PAGE_SIZE));
Expand Down Expand Up @@ -607,15 +643,24 @@ static vm_fault_t daxfs_dax_fault(struct vm_fault *vmf)
base = daxfs_base_file_data(info, inode,
pos, PAGE_SIZE,
&base_len, &pcslot);
daxfs_copy_page(data, base, base_len);
daxfs_pcache_put_page(info, pcslot);

/* Publish AFTER COW */
data = daxfs_overlay_publish_page(info,
inode->i_ino, pgoff, pool_off, data);
if (data)
xa_store(&DAXFS_I(inode)->ovl_pages,
pgoff, data, GFP_KERNEL);
if (IS_ERR(base)) {
/* Do not COW zeros over data we
* could not read; fall through to
* the SIGBUS below.
*/
data = NULL;
} else {
daxfs_copy_page(data, base, base_len);
daxfs_pcache_put_page(info, pcslot);

/* Publish AFTER COW */
data = daxfs_overlay_publish_page(info,
inode->i_ino, pgoff, pool_off,
data);
if (data)
xa_store(&DAXFS_I(inode)->ovl_pages,
pgoff, data, GFP_KERNEL);
}
}
}
sb_end_pagefault(inode->i_sb);
Expand All @@ -642,6 +687,8 @@ static vm_fault_t daxfs_dax_fault(struct vm_fault *vmf)

data = daxfs_base_file_data(info, inode, pos,
PAGE_SIZE, &len, &pcslot);
if (IS_ERR(data))
return VM_FAULT_SIGBUS;

/*
* MAP_SHARED with page-aligned data: use direct PFN mapping.
Expand Down Expand Up @@ -719,14 +766,18 @@ static vm_fault_t daxfs_dax_pfn_mkwrite(struct vm_fault *vmf)
base = daxfs_base_file_data(info, inode,
pos, PAGE_SIZE,
&base_len, &pcslot);
daxfs_copy_page(data, base, base_len);
daxfs_pcache_put_page(info, pcslot);
if (IS_ERR(base)) {
data = NULL;
} else {
daxfs_copy_page(data, base, base_len);
daxfs_pcache_put_page(info, pcslot);

data = daxfs_overlay_publish_page(info,
inode->i_ino, pgoff, pool_off, data);
if (data)
xa_store(&DAXFS_I(inode)->ovl_pages, pgoff,
data, GFP_KERNEL);
data = daxfs_overlay_publish_page(info,
inode->i_ino, pgoff, pool_off, data);
if (data)
xa_store(&DAXFS_I(inode)->ovl_pages,
pgoff, data, GFP_KERNEL);
}
}
}
sb_end_pagefault(inode->i_sb);
Expand Down
50 changes: 50 additions & 0 deletions tests/test_overlay.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,55 @@ test_overlay_truncate() {
pass "Truncate via overlay"
}

test_overlay_hole_reads() {
run_test "Holes read as zeros, not short"

local f="$MNT/hole.bin"
local pagesize=$(getconf PAGESIZE)
local size=$((pagesize * 4))

# A file extended by truncate has no overlay pages behind it at all.
: > "$f" || { fail "Hole reads" "Failed to create file"; return; }
truncate -s "$size" "$f" \
|| { fail "Hole reads" "Failed to extend"; return; }

local got
got=$(wc -c < "$f")
if [ "$got" -ne "$size" ]; then
fail "Hole reads" "stat size is $got, expected $size"
return
fi

# read() must return the whole range as zeros rather than EOF at 0
got=$(dd if="$f" bs="$size" count=1 status=none | wc -c)
if [ "$got" -ne "$size" ]; then
fail "Hole reads" "read returned $got bytes, expected $size"
return
fi

if [ -n "$(dd if="$f" bs="$size" count=1 status=none | tr -d '\0')" ]; then
fail "Hole reads" "hole did not read as zeros"
return
fi

# A hole followed by real data: the read must cross the hole and find it
printf 'TAIL' | dd of="$f" bs=1 seek=$((size - 4)) conv=notrunc status=none
got=$(dd if="$f" bs=1 skip=$((size - 4)) count=4 status=none)
if [ "$got" != "TAIL" ]; then
fail "Hole reads" "data after hole not readable: '$got'"
return
fi

got=$(dd if="$f" bs="$size" count=1 status=none | wc -c)
if [ "$got" -ne "$size" ]; then
fail "Hole reads" "full read after hole returned $got, expected $size"
return
fi

rm -f "$f"
pass "Holes read as zeros, not short"
}

#
# Empty mode tests
#
Expand Down Expand Up @@ -815,6 +864,7 @@ main() {
test_overlay_symlink
test_overlay_rename
test_overlay_truncate
test_overlay_hole_reads

# Empty mode tests
setup_empty
Expand Down