You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2016/02/24 20:25:16 UTC
[3/3] incubator-mynewt-larva git commit: MYNEWT-160 NFFS - Scattered
blocks; restore fails.
MYNEWT-160 NFFS - Scattered blocks; restore fails.
This is an initial inefficient fix. The proper fix is scheduled for
0.9.0.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/commit/8d16fadc
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/tree/8d16fadc
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/diff/8d16fadc
Branch: refs/heads/develop
Commit: 8d16fadc074f492a6f20d9ac2cff79d403feab88
Parents: dd337a9
Author: Christopher Collins <cc...@gmail.com>
Authored: Wed Feb 24 11:23:26 2016 -0800
Committer: Christopher Collins <cc...@gmail.com>
Committed: Wed Feb 24 11:23:26 2016 -0800
----------------------------------------------------------------------
fs/nffs/src/nffs_block.c | 44 +++++++++
fs/nffs/src/nffs_priv.h | 2 +
fs/nffs/src/nffs_restore.c | 149 +++++++++++++++++++++++++----
fs/nffs/src/test/arch/sim/nffs_test.c | 50 +++++++++-
4 files changed, 226 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/8d16fadc/fs/nffs/src/nffs_block.c
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_block.c b/fs/nffs/src/nffs_block.c
index 1288dc6..12fe21d 100644
--- a/fs/nffs/src/nffs_block.c
+++ b/fs/nffs/src/nffs_block.c
@@ -209,6 +209,50 @@ nffs_block_delete_from_ram(struct nffs_hash_entry *block_entry)
}
/**
+ * Determines if a particular block can be found in RAM by following a chain of
+ * previous block pointers, starting with the specified hash entry.
+ *
+ * @param start The block entry at which to start the search.
+ * @param sought_id The ID of the block to search for.
+ *
+ * @return 0 if the sought after ID was found;
+ * FS_ENOENT if the ID was not found;
+ * Other FS code on error.
+ */
+int
+nffs_block_find_predecessor(struct nffs_hash_entry *start, uint32_t sought_id)
+{
+ struct nffs_hash_entry *entry;
+ struct nffs_disk_block disk_block;
+ uint32_t area_offset;
+ uint8_t area_idx;
+ int rc;
+
+ entry = start;
+ while (entry != NULL && entry->nhe_id != sought_id) {
+ nffs_flash_loc_expand(entry->nhe_flash_loc, &area_idx, &area_offset);
+ rc = nffs_block_read_disk(area_idx, area_offset, &disk_block);
+ if (rc != 0) {
+ return rc;
+ }
+
+ if (disk_block.ndb_prev_id == NFFS_ID_NONE) {
+ entry = NULL;
+ } else {
+ entry = nffs_hash_find(disk_block.ndb_prev_id);
+ }
+ }
+
+ if (entry == NULL) {
+ rc = FS_ENOENT;
+ } else {
+ rc = 0;
+ }
+
+ return rc;
+}
+
+/**
* Constructs a full data block representation from the specified minimal
* block entry. However, the resultant block's pointers are set to null,
* rather than populated via hash table lookups. This behavior is useful when
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/8d16fadc/fs/nffs/src/nffs_priv.h
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_priv.h b/fs/nffs/src/nffs_priv.h
index 119b5fa..061964c 100644
--- a/fs/nffs/src/nffs_priv.h
+++ b/fs/nffs/src/nffs_priv.h
@@ -265,6 +265,8 @@ void nffs_block_delete_list_from_disk(const struct nffs_block *first,
const struct nffs_block *last);
void nffs_block_to_disk(const struct nffs_block *block,
struct nffs_disk_block *out_disk_block);
+int nffs_block_find_predecessor(struct nffs_hash_entry *start,
+ uint32_t sought_id);
int nffs_block_from_hash_entry_no_ptrs(struct nffs_block *out_block,
struct nffs_hash_entry *entry);
int nffs_block_from_hash_entry(struct nffs_block *out_block,
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/8d16fadc/fs/nffs/src/nffs_restore.c
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_restore.c b/fs/nffs/src/nffs_restore.c
index 3e45cb9..81b59a7 100644
--- a/fs/nffs/src/nffs_restore.c
+++ b/fs/nffs/src/nffs_restore.c
@@ -478,10 +478,14 @@ nffs_restore_inode(const struct nffs_disk_inode *disk_inode, uint8_t area_idx,
}
if (nffs_hash_id_is_file(inode_entry->nie_hash_entry.nhe_id)) {
+ NFFS_LOG(DEBUG, "restoring file; id=%d\n",
+ inode_entry->nie_hash_entry.nhe_id);
if (inode_entry->nie_hash_entry.nhe_id >= nffs_hash_next_file_id) {
nffs_hash_next_file_id = inode_entry->nie_hash_entry.nhe_id + 1;
}
} else {
+ NFFS_LOG(DEBUG, "restoring dir; id=%d\n",
+ inode_entry->nie_hash_entry.nhe_id);
if (inode_entry->nie_hash_entry.nhe_id >= nffs_hash_next_dir_id) {
nffs_hash_next_dir_id = inode_entry->nie_hash_entry.nhe_id + 1;
}
@@ -597,6 +601,24 @@ nffs_restore_block(const struct nffs_disk_block *disk_block, uint8_t area_idx,
/* The block is ready to be inserted into the hash. */
+ nffs_hash_insert(entry);
+
+ if (disk_block->ndb_id >= nffs_hash_next_block_id) {
+ nffs_hash_next_block_id = disk_block->ndb_id + 1;
+ }
+
+ /* Make sure the maximum block data size is not set lower than the size of
+ * an existing block.
+ */
+ if (disk_block->ndb_data_len > nffs_restore_largest_block_data_len) {
+ nffs_restore_largest_block_data_len = disk_block->ndb_data_len;
+ }
+
+ NFFS_LOG(DEBUG, "restoring block; id=%u seq=%u inode_id=%u prev_id=%u "
+ "data_len=%u\n", disk_block->ndb_id, disk_block->ndb_seq,
+ disk_block->ndb_inode_id, disk_block->ndb_prev_id,
+ disk_block->ndb_data_len);
+
inode_entry = nffs_hash_find_inode(disk_block->ndb_inode_id);
if (inode_entry == NULL) {
rc = nffs_restore_dummy_inode(disk_block->ndb_inode_id, &inode_entry);
@@ -605,23 +627,58 @@ nffs_restore_block(const struct nffs_disk_block *disk_block, uint8_t area_idx,
}
}
- if (inode_entry->nie_last_block_entry == NULL ||
- inode_entry->nie_last_block_entry->nhe_id == disk_block->ndb_prev_id) {
-
+ /* Make sure the parent inode (file) points to the latest data block
+ * restored so far.
+ *
+ * XXX: This is an O(n) operation (n = # of blocks in the file), and is
+ * horribly inefficient for large files. MYNEWT-161 has been opened to
+ * address this.
+ */
+ if (inode_entry->nie_last_block_entry == NULL) {
+ /* This is the first data block restored for this file. */
inode_entry->nie_last_block_entry = entry;
- }
+ NFFS_LOG(DEBUG, "setting last block: %u\n", entry->nhe_id);
+ } else {
+ /* Determine if this this data block comes after our current idea of
+ * the file's last data block.
+ */
- nffs_hash_insert(entry);
+ rc = nffs_block_find_predecessor(
+ entry, inode_entry->nie_last_block_entry->nhe_id);
+ switch (rc) {
+ case 0:
+ /* The currently-last block is a predecessor of the new block; the
+ * new block comes later.
+ */
+ NFFS_LOG(DEBUG, "replacing last block: %u --> %u\n",
+ inode_entry->nie_last_block_entry->nhe_id,
+ entry->nhe_id);
+ inode_entry->nie_last_block_entry = entry;
+ break;
- if (disk_block->ndb_id >= nffs_hash_next_block_id) {
- nffs_hash_next_block_id = disk_block->ndb_id + 1;
- }
+ case FS_ENOENT:
+ /* The currently-last block is not a known predecessor of the new
+ * block. There are two possibilities:
+ * (1) The new block comes before the currently-last block.
+ * (2) The new block comes later, but the two blocks are not
+ * chained in RAM yet because an intervening block has yet
+ * to be restored.
+ * If we can find the new block by searching backwards from the
+ * currently-last black, then (1) is true. Otherwise, (2) is true.
+ */
+ rc = nffs_block_find_predecessor(
+ inode_entry->nie_last_block_entry, entry->nhe_id);
+ if (rc == FS_ENOENT) {
+ inode_entry->nie_last_block_entry = entry;
+ NFFS_LOG(DEBUG, "replacing last block: %u --> %u\n",
+ inode_entry->nie_last_block_entry->nhe_id,
+ entry->nhe_id);
+ }
+ break;
- /* Make sure the maximum block data size is not set lower than the size of
- * an existing block.
- */
- if (disk_block->ndb_data_len > nffs_restore_largest_block_data_len) {
- nffs_restore_largest_block_data_len = disk_block->ndb_data_len;
+ default:
+ return rc;
+ }
}
return 0;
@@ -649,14 +706,14 @@ nffs_restore_object(const struct nffs_disk_object *disk_object)
switch (disk_object->ndo_type) {
case NFFS_OBJECT_TYPE_INODE:
rc = nffs_restore_inode(&disk_object->ndo_disk_inode,
- disk_object->ndo_area_idx,
- disk_object->ndo_offset);
+ disk_object->ndo_area_idx,
+ disk_object->ndo_offset);
break;
case NFFS_OBJECT_TYPE_BLOCK:
rc = nffs_restore_block(&disk_object->ndo_disk_block,
- disk_object->ndo_area_idx,
- disk_object->ndo_offset);
+ disk_object->ndo_area_idx,
+ disk_object->ndo_offset);
break;
default:
@@ -896,6 +953,61 @@ nffs_restore_corrupt_scratch(void)
return 0;
}
+static void
+nffs_log_contents(void)
+{
+ struct nffs_inode_entry *inode_entry;
+ struct nffs_hash_entry *entry;
+ struct nffs_block block;
+ struct nffs_inode inode;
+ int rc;
+ int i;
+
+ NFFS_HASH_FOREACH(entry, i) {
+ if (nffs_hash_id_is_block(entry->nhe_id)) {
+ rc = nffs_block_from_hash_entry(&block, entry);
+ assert(rc == 0);
+ NFFS_LOG(DEBUG, "block; id=%u inode_id=", entry->nhe_id);
+ if (block.nb_inode_entry == NULL) {
+ NFFS_LOG(DEBUG, "null ");
+ } else {
+ NFFS_LOG(DEBUG, "%u ",
+ block.nb_inode_entry->nie_hash_entry.nhe_id);
+ }
+
+ NFFS_LOG(DEBUG, "prev_id=");
+ if (block.nb_prev == NULL) {
+ NFFS_LOG(DEBUG, "null ");
+ } else {
+ NFFS_LOG(DEBUG, "%u ", block.nb_prev->nhe_id);
+ }
+
+ NFFS_LOG(DEBUG, "data_len=%u\n", block.nb_data_len);
+ } else {
+ inode_entry = (void *)entry;
+ rc = nffs_inode_from_entry(&inode, inode_entry);
+ assert(rc == 0);
+
+ if (nffs_hash_id_is_file(entry->nhe_id)) {
+ NFFS_LOG(DEBUG, "file; id=%u name=%.3s block_id=",
+ entry->nhe_id, inode.ni_filename);
+ if (inode_entry->nie_last_block_entry == NULL) {
+ NFFS_LOG(DEBUG, "null");
+ } else {
+ NFFS_LOG(DEBUG, "%u",
+ inode_entry->nie_last_block_entry->nhe_id);
+ }
+ NFFS_LOG(DEBUG, "\n");
+ } else {
+ inode_entry = (void *)entry;
+ NFFS_LOG(DEBUG, "dir; id=%u name=%.3s\n", entry->nhe_id,
+ inode.ni_filename);
+
+ }
+ }
+ }
+}
+
/**
* Searches for a valid nffs file system among the specified areas. This
* function succeeds if a file system is detected among any subset of the
@@ -1029,6 +1141,9 @@ nffs_restore_full(const struct nffs_area_desc *area_descs)
goto err;
}
+ NFFS_LOG(DEBUG, "CONTENTS\n");
+ nffs_log_contents();
+
return 0;
err:
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/8d16fadc/fs/nffs/src/test/arch/sim/nffs_test.c
----------------------------------------------------------------------
diff --git a/fs/nffs/src/test/arch/sim/nffs_test.c b/fs/nffs/src/test/arch/sim/nffs_test.c
index f27cfaa..a0f4f1a 100644
--- a/fs/nffs/src/test/arch/sim/nffs_test.c
+++ b/fs/nffs/src/test/arch/sim/nffs_test.c
@@ -123,7 +123,7 @@ nffs_test_util_assert_cache_is_sane(const char *filename)
static void
nffs_test_util_assert_contents(const char *filename, const char *contents,
- int contents_len)
+ int contents_len)
{
struct fs_file *file;
uint32_t bytes_read;
@@ -561,7 +561,7 @@ nffs_test_assert_system_once(const struct nffs_test_file_desc *root_dir)
static void
nffs_test_assert_system(const struct nffs_test_file_desc *root_dir,
- const struct nffs_area_desc *area_descs)
+ const struct nffs_area_desc *area_descs)
{
int rc;
@@ -2361,6 +2361,51 @@ TEST_CASE(nffs_test_readdir)
TEST_ASSERT(rc == FS_ENOENT);
}
+TEST_CASE(nffs_test_split_file)
+{
+ static char data[24 * 1024];
+ int rc;
+ int i;
+
+ /*** Setup. */
+ static const struct nffs_area_desc area_descs_two[] = {
+ { 0x00000000, 16 * 1024 },
+ { 0x00004000, 16 * 1024 },
+ { 0x00008000, 16 * 1024 },
+ { 0, 0 },
+ };
+
+ rc = nffs_format(area_descs_two);
+ TEST_ASSERT(rc == 0);
+
+ for (i = 0; i < sizeof data; i++) {
+ data[i] = i;
+ }
+
+ for (i = 0; i < 256; i++) {
+ nffs_test_util_create_file("/myfile.txt", data, sizeof data);
+ rc = fs_unlink("/myfile.txt");
+ TEST_ASSERT(rc == 0);
+ }
+
+ nffs_test_util_create_file("/myfile.txt", data, sizeof data);
+
+ struct nffs_test_file_desc *expected_system =
+ (struct nffs_test_file_desc[]) { {
+ .filename = "",
+ .is_dir = 1,
+ .children = (struct nffs_test_file_desc[]) { {
+ .filename = "myfile.txt",
+ .contents = data,
+ .contents_len = sizeof data,
+ }, {
+ .filename = NULL,
+ } },
+ } };
+
+ nffs_test_assert_system(expected_system, area_descs_two);
+}
+
TEST_SUITE(nffs_suite_cache)
{
int rc;
@@ -2406,6 +2451,7 @@ nffs_test_gen(void)
nffs_test_large_system();
nffs_test_lost_found();
nffs_test_readdir();
+ nffs_test_split_file();
}
TEST_SUITE(gen_1_1)