[f2fs] Correct error handling in Write() and Truncate()
It adds handling for corner cases related to no space and max size.
Also, since f2fs needs at least 100MiB device size,
device_block_count in fs_test is reconfigured.
Test: fx test fs-tests large-fs-tests
Prerequisite:
FUCHSIA_DIR$ patch -p1 < \
third_party/f2fs/patches/0004-f2fs-Configure-deivce-size-as-minimum-size-for-f2fs.patch
Change-Id: Id1653554fe2e64becb270532d48b49dfec156257
Reviewed-on: https://fuchsia-review.googlesource.com/c/third_party/f2fs/+/539562
Reviewed-by: Brett Wilson <brettw@google.com>
diff --git a/data.cc b/data.cc
index 7c70972..bd36b40 100644
--- a/data.cc
+++ b/data.cc
@@ -77,18 +77,18 @@
#endif
}
-int VnodeF2fs::ReserveNewBlock(DnodeOfData *dn) {
+zx_status_t VnodeF2fs::ReserveNewBlock(DnodeOfData *dn) {
SbInfo &sbi = fs_->GetSbInfo();
if (IsInodeFlagSet(&dn->vnode->fi_, InodeInfoFlag::kNoAlloc))
- return -EPERM;
+ return ZX_ERR_ACCESS_DENIED;
if (!IncValidBlockCount(&sbi, dn->vnode, 1))
- return -ENOSPC;
+ return ZX_ERR_NO_SPACE;
SetDataBlkaddr(dn, kNewAddr);
dn->data_blkaddr = kNewAddr;
fs_->Nodemgr().SyncInodePage(dn);
- return 0;
+ return ZX_OK;
}
#if 0 // porting needed
@@ -295,9 +295,9 @@
return err;
if (dn.data_blkaddr == kNullAddr) {
- if (ReserveNewBlock(&dn)) {
+ if (zx_status_t ret = ReserveNewBlock(&dn); ret != ZX_OK) {
F2fsPutDnode(&dn);
- return ZX_ERR_NO_SPACE;
+ return ret;
}
}
F2fsPutDnode(&dn);
diff --git a/file.cc b/file.cc
index 5817b38..98029f5 100644
--- a/file.cc
+++ b/file.cc
@@ -375,33 +375,65 @@
uint64_t blk_end = (offset + len) / kBlockSize;
size_t off_in_block = offset % kBlockSize;
size_t off_in_buf = 0;
- Page *data_page = nullptr;
void *data_buf = nullptr;
- uint64_t n;
size_t left = len;
- for (n = blk_start; n <= blk_end; n++) {
+ if (len == 0)
+ return ZX_OK;
+
+ if (offset + len > static_cast<size_t>(Vfs()->MaxFileSize(Vfs()->RawSb().log_blocksize)))
+ return ZX_ERR_INVALID_ARGS;
+
+ std::vector<Page *> data_pages(blk_end - blk_start + 1, nullptr);
+
+ for (uint64_t n = blk_start; n <= blk_end; n++) {
+ uint64_t index = n - blk_start;
size_t cur_len = std::min(static_cast<size_t>(kBlockSize - off_in_block), left);
- if (zx_status_t ret = WriteBegin(n * kBlockSize + off_in_block, cur_len, &data_page);
+
+ ZX_ASSERT(index < data_pages.size());
+
+ if (zx_status_t ret = WriteBegin(n * kBlockSize + off_in_block, cur_len, &data_pages[index]);
ret != ZX_OK) {
- if (off_in_buf > 0) {
- clock_gettime(CLOCK_REALTIME, &i_mtime_);
- i_ctime_ = i_mtime_;
-
- MarkInodeDirty(this);
+ // WriteBegin() tries to prepare in-memory buffers and direct node blocks for storing |data|
+ // If it succeeds, data_pages[index] has a page of valid virtual memory.
+ // If it fails with any reasons such as no space/memory,
+ // every data_pages[less than index] is released, and DoWrite() returns with the err code.
+ for (uint64_t m = 0; m < index; m++) {
+ if (data_pages[m] != nullptr)
+ F2fsPutPage(data_pages[m], 1);
}
- *out_actual = off_in_buf;
+ *out_actual = 0;
return ret;
}
+ off_in_block = 0;
+ off_in_buf += cur_len;
+ left -= cur_len;
+
+ if (left == 0)
+ break;
+ }
+
+ off_in_block = offset % kBlockSize;
+ off_in_buf = 0;
+ left = len;
+
+ for (uint64_t n = blk_start; n <= blk_end; n++) {
+ uint64_t index = n - blk_start;
+ size_t cur_len = std::min(static_cast<size_t>(kBlockSize - off_in_block), left);
+
+ ZX_ASSERT(index < data_pages.size());
+
+ Page *data_page = data_pages[index];
+
data_buf = PageAddress(data_page);
memcpy(static_cast<char *>(data_buf) + off_in_block,
static_cast<const char *>(data) + off_in_buf, cur_len);
+ off_in_block = 0;
off_in_buf += cur_len;
left -= cur_len;
- off_in_block = 0;
i_size_ = std::max(static_cast<size_t>(i_size_), offset + off_in_buf);
#if 0 // porting needed
@@ -410,7 +442,7 @@
FlushDirtyDataPage(Vfs(), data_page);
#endif
F2fsPutPage(data_page, 1);
- data_page = nullptr;
+ data_pages[index] = nullptr;
if (left == 0)
break;
@@ -449,9 +481,7 @@
if (len > static_cast<size_t>(Vfs()->MaxFileSize(Vfs()->RawSb().log_blocksize)))
return ZX_ERR_INVALID_ARGS;
- i_size_ = len;
-
- return DoTruncate();
+ return DoTruncate(len);
}
} // namespace f2fs
diff --git a/node.cc b/node.cc
index 3553fe6..1d416d3 100644
--- a/node.cc
+++ b/node.cc
@@ -674,7 +674,7 @@
* The maximum depth is four.
* Offset[0] will have raw inode offset.
*/
-int NodeMgr::GetNodePath(long block, int offset[4], uint32_t noffset[4]) {
+zx::status<int> NodeMgr::GetNodePath(long block, int offset[4], uint32_t noffset[4]) {
const long direct_index = kAddrsPerInode;
const long direct_blks = kAddrsPerBlock;
const long dptrs_per_blk = kNidsPerBlock;
@@ -738,10 +738,10 @@
level = 3;
goto got;
} else {
- ZX_ASSERT(0);
+ zx::error(ZX_ERR_NOT_FOUND);
}
got:
- return level;
+ return zx::ok(level);
}
/*
@@ -757,7 +757,11 @@
int level, i;
zx_status_t err = 0;
- level = GetNodePath(index, offset, noffset);
+ auto node_path = GetNodePath(index, offset, noffset);
+ if (node_path.is_error())
+ return node_path.error_value();
+
+ level = *node_path;
nids[0] = dn->vnode->Ino();
npage[0] = nullptr;
@@ -1028,7 +1032,11 @@
Page *page = nullptr;
zx_status_t err = 0;
- level = GetNodePath(from, offset, reinterpret_cast<uint32_t *>(noffset));
+ auto node_path = GetNodePath(from, offset, reinterpret_cast<uint32_t *>(noffset));
+ if (node_path.is_error())
+ return node_path.error_value();
+
+ level = *node_path;
err = GetNodePage(vnode->Ino(), &page);
if (err)
diff --git a/node.h b/node.h
index 83b6ce3..bb842f4 100644
--- a/node.h
+++ b/node.h
@@ -177,7 +177,7 @@
void SetNodeAddr(NodeInfo *ni, block_t new_blkaddr);
int TryToFreeNats(int nr_shrink);
- int GetNodePath(long block, int offset[4], uint32_t noffset[4]);
+ zx::status<int> GetNodePath(long block, int offset[4], uint32_t noffset[4]);
void TruncateNode(DnodeOfData *dn);
zx_status_t TruncateDnode(DnodeOfData *dn);
zx_status_t TruncateNodes(DnodeOfData *dn, uint32_t nofs, int ofs, int depth);
diff --git a/patches/0004-f2fs-Configure-deivce-size-as-minimum-size-for-f2fs.patch b/patches/0004-f2fs-Configure-deivce-size-as-minimum-size-for-f2fs.patch
new file mode 100644
index 0000000..ed8ece6
--- /dev/null
+++ b/patches/0004-f2fs-Configure-deivce-size-as-minimum-size-for-f2fs.patch
@@ -0,0 +1,68 @@
+From f748670fa7d6a1c8af5f53f33a5e4ab091b94bbe Mon Sep 17 00:00:00 2001
+From: Dongjin Kim <dongjin_.kim@samsung.com>
+Date: Mon, 7 Jun 2021 15:38:25 +0900
+Subject: [PATCH] [f2fs] Configure deivce size as minimum size for f2fs
+
+Since f2fs needs device size more than 100MiB,
+ configure device_block_count in rw-tests to provide the minimum size.
+
+Change-Id: Ia4a949fa9c505fa7142eaa8170b7e7487c592e1e
+---
+ src/storage/fs_test/directory_large.cc | 6 ++++++
+ src/storage/fs_test/rw.cc | 4 ++++
+ src/storage/fs_test/truncate_large.cc | 7 ++++++-
+ 3 files changed, 16 insertions(+), 1 deletion(-)
+
+diff --git a/src/storage/fs_test/directory_large.cc b/src/storage/fs_test/directory_large.cc
+index c397bbdcfc2..80689686d4a 100644
+--- a/src/storage/fs_test/directory_large.cc
++++ b/src/storage/fs_test/directory_large.cc
+@@ -59,6 +59,12 @@ INSTANTIATE_TEST_SUITE_P(
+ // memory is the limiting factor).
+ if (options.filesystem->GetTraits().in_memory)
+ return std::nullopt;
++
++ if (options.filesystem->GetTraits().name == "f2fs") {
++ options.device_block_count = 204800;
++ return options;
++ }
++
+ if (!options.filesystem->GetTraits().has_directory_size_limit) {
+ // Fatfs is slow and, other than the root directory on FAT12/16, is limited by the size
+ // of the ram-disk rather than a directory size limit, so use a small ram-disk to keep
+diff --git a/src/storage/fs_test/rw.cc b/src/storage/fs_test/rw.cc
+index c5fa0256b8b..380b50f1f10 100644
+--- a/src/storage/fs_test/rw.cc
++++ b/src/storage/fs_test/rw.cc
+@@ -209,6 +209,10 @@ INSTANTIATE_TEST_SUITE_P(
+ return std::nullopt;
+ }
+ // Run on a smaller ram-disk to keep run-time reasonable.
++ if (options.filesystem->GetTraits().name == "f2fs") {
++ options.device_block_count = 204800;
++ return options;
++ }
+ options.device_block_count = 8192;
+ options.fvm_slice_size = 32768;
+ return options;
+diff --git a/src/storage/fs_test/truncate_large.cc b/src/storage/fs_test/truncate_large.cc
+index 3d88727cf82..e07f2eb17d7 100644
+--- a/src/storage/fs_test/truncate_large.cc
++++ b/src/storage/fs_test/truncate_large.cc
+@@ -24,7 +24,12 @@ std::vector<LargeTruncateTestParamType> GetTestCombinations(
+ // Fatfs is slow, so skip larger buffer sizes.
+ continue;
+ }
+- options.device_block_count = 3 * (1LLU << 16);
++
++ if (options.filesystem->GetTraits().name == "f2fs") {
++ options.device_block_count = 204800;
++ } else {
++ options.device_block_count = 3 * (1LLU << 16);
++ }
+ options.device_block_size = 1LLU << 9;
+ options.fvm_slice_size = 1LLU << 23;
+ test_combinations.push_back(std::make_tuple(options, variation));
+--
+2.25.1
+
diff --git a/recovery.cc b/recovery.cc
index 24e37e2..b459b6c 100644
--- a/recovery.cc
+++ b/recovery.cc
@@ -283,9 +283,8 @@
if (src != dest && dest != kNewAddr && dest != kNullAddr) {
if (src == kNullAddr) {
- int err = vnode->ReserveNewBlock(&dn);
- /* We should not get -ENOSPC */
- ZX_ASSERT(!err);
+ zx_status_t err = vnode->ReserveNewBlock(&dn);
+ ZX_ASSERT(err == ZX_OK);
}
/* Check the previous node page having this index */
diff --git a/vnode.cc b/vnode.cc
index b2ba304..88af9db 100644
--- a/vnode.cc
+++ b/vnode.cc
@@ -373,10 +373,11 @@
return ZX_OK;
}
-zx_status_t VnodeF2fs::DoTruncate() {
+zx_status_t VnodeF2fs::DoTruncate(size_t len) {
zx_status_t ret;
- if (ret = TruncateBlocks(i_size_); ret == ZX_OK) {
+ if (ret = TruncateBlocks(len); ret == ZX_OK) {
+ i_size_ = len;
clock_gettime(CLOCK_REALTIME, &i_mtime_);
i_ctime_ = i_mtime_;
MarkInodeDirty(this);
@@ -446,11 +447,13 @@
SbInfo &sbi = Vfs()->GetSbInfo();
unsigned int blocksize = sbi.blocksize;
DnodeOfData dn;
- pgoff_t free_from;
int count = 0;
zx_status_t err;
- free_from = static_cast<pgoff_t>((from + blocksize - 1) >> (sbi.log_blocksize));
+ if (from > i_size_)
+ return ZX_OK;
+
+ pgoff_t free_from = static_cast<pgoff_t>((from + blocksize - 1) >> (sbi.log_blocksize));
mutex_lock_op(&sbi, LockType::kDataTrunc);
diff --git a/vnode.h b/vnode.h
index d1df008..56fd0b4 100644
--- a/vnode.h
+++ b/vnode.h
@@ -64,7 +64,7 @@
static zx_status_t Vget(F2fs *fs, uint64_t ino, fbl::RefPtr<VnodeF2fs> *out);
void UpdateInode(Page *node_page);
zx_status_t WriteInode(WritebackControl *wbc);
- zx_status_t DoTruncate();
+ zx_status_t DoTruncate(size_t len);
int TruncateDataBlocksRange(DnodeOfData *dn, int count);
void TruncateDataBlocks(DnodeOfData *dn);
void TruncatePartialDataPage(uint64_t from);
@@ -76,7 +76,7 @@
#endif
void SetDataBlkaddr(DnodeOfData *dn, block_t new_addr);
- int ReserveNewBlock(DnodeOfData *dn);
+ zx_status_t ReserveNewBlock(DnodeOfData *dn);
#if 0 // porting needed
// static int CheckExtentCache(inode *inode, pgoff_t pgofs,
