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/04/19 23:33:43 UTC
[2/3] incubator-mynewt-core git commit: nffs - delete cached blocks
after gc.
nffs - delete cached blocks after gc.
Garbage collection deletes and recreates data blocks. Any cached data
blocks are potentially invalid after a garbage collection cycle.
Garbage collection does not delete any inodes, but it may move them
around in flash.
This fix implements the following changes:
* After GC is performed, delete all cached data blocks.
* After GC is performed, refresh all cached inodes (recache them).
* For higher level operations that use pointers to cached data
blocks, detect when garbage collection occurs, and reset the
pointers as necessary.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/2f430d3c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/2f430d3c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/2f430d3c
Branch: refs/heads/develop
Commit: 2f430d3c9cf6014e2fd1928af07358c5d96abbea
Parents: 3abe740
Author: Christopher Collins <cc...@apache.org>
Authored: Tue Apr 19 14:01:55 2016 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Tue Apr 19 14:32:23 2016 -0700
----------------------------------------------------------------------
fs/nffs/src/nffs_cache.c | 85 +++++++++++++++++++++++++++++++++++++------
fs/nffs/src/nffs_gc.c | 34 +++++++++++++++++
fs/nffs/src/nffs_priv.h | 2 +
fs/nffs/src/nffs_write.c | 32 +++++++++++++---
4 files changed, 137 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/2f430d3c/fs/nffs/src/nffs_cache.c
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_cache.c b/fs/nffs/src/nffs_cache.c
index 580d52e..51ebc30 100644
--- a/fs/nffs/src/nffs_cache.c
+++ b/fs/nffs/src/nffs_cache.c
@@ -26,7 +26,7 @@ TAILQ_HEAD(nffs_cache_inode_list, nffs_cache_inode);
static struct nffs_cache_inode_list nffs_cache_inode_list =
TAILQ_HEAD_INITIALIZER(nffs_cache_inode_list);
-static void nffs_cache_collect_blocks(void);
+static void nffs_cache_reclaim_blocks(void);
static struct nffs_cache_block *
nffs_cache_block_alloc(void)
@@ -56,7 +56,7 @@ nffs_cache_block_acquire(void)
cache_block = nffs_cache_block_alloc();
if (cache_block == NULL) {
- nffs_cache_collect_blocks();
+ nffs_cache_reclaim_blocks();
cache_block = nffs_cache_block_alloc();
}
@@ -91,8 +91,7 @@ nffs_cache_inode_alloc(void)
entry = os_memblock_get(&nffs_cache_inode_pool);
if (entry != NULL) {
memset(entry, 0, sizeof *entry);
- entry->nci_block_list = (struct nffs_cache_block_list)
- TAILQ_HEAD_INITIALIZER(entry->nci_block_list);
+ TAILQ_INIT(&entry->nci_block_list);
}
return entry;
@@ -216,7 +215,7 @@ nffs_cache_inode_range(const struct nffs_cache_inode *cache_inode,
}
static void
-nffs_cache_collect_blocks(void)
+nffs_cache_reclaim_blocks(void)
{
struct nffs_cache_inode *cache_inode;
@@ -279,6 +278,73 @@ done:
}
/**
+ * Recaches all cached inodes. All cached blocks are deleted from the cache
+ * during this operation. This function should be called after garbage
+ * collection occurs to ensure the cache is consistent.
+ *
+ * @return 0 on success; nonzero on failure.
+ */
+int
+nffs_cache_inode_refresh(void)
+{
+ struct nffs_cache_inode *cache_inode;
+ struct nffs_inode_entry *inode_entry;
+ int rc;
+
+ TAILQ_FOREACH(cache_inode, &nffs_cache_inode_list, nci_link) {
+ /* Clear entire block list. */
+ nffs_cache_inode_free_blocks(cache_inode);
+
+ inode_entry = cache_inode->nci_inode.ni_inode_entry;
+ rc = nffs_inode_from_entry(&cache_inode->nci_inode, inode_entry);
+ if (rc != 0) {
+ return rc;
+ }
+
+ /* File size remains valid. */
+ }
+
+ return 0;
+}
+
+static void
+nffs_cache_log_block(struct nffs_cache_inode *cache_inode,
+ struct nffs_cache_block *cache_block)
+{
+ NFFS_LOG(DEBUG, "id=%u inode=%u flash_off=0x%08x "
+ "file_off=%u len=%d (entry=%p)\n",
+ cache_block->ncb_block.nb_hash_entry->nhe_id,
+ cache_inode->nci_inode.ni_inode_entry->nie_hash_entry.nhe_id,
+ cache_block->ncb_block.nb_hash_entry->nhe_flash_loc,
+ cache_block->ncb_file_offset,
+ cache_block->ncb_block.nb_data_len,
+ cache_block->ncb_block.nb_hash_entry);
+}
+
+static void
+nffs_cache_log_insert_block(struct nffs_cache_inode *cache_inode,
+ struct nffs_cache_block *cache_block,
+ int tail)
+{
+ NFFS_LOG(DEBUG, "caching block (%s): ", tail ? "tail" : "head");
+ nffs_cache_log_block(cache_inode, cache_block);
+}
+
+void
+nffs_cache_insert_block(struct nffs_cache_inode *cache_inode,
+ struct nffs_cache_block *cache_block,
+ int tail)
+{
+ if (tail) {
+ TAILQ_INSERT_TAIL(&cache_inode->nci_block_list, cache_block, ncb_link);
+ } else {
+ TAILQ_INSERT_HEAD(&cache_inode->nci_block_list, cache_block, ncb_link);
+ }
+
+ nffs_cache_log_insert_block(cache_inode, cache_block, tail);
+}
+
+/**
* Finds the data block containing the specified offset within a file inode.
* If the block is not yet cached, it gets cached as a result of this
* operation. This function modifies the inode's cached block list according
@@ -364,8 +430,7 @@ nffs_cache_seek(struct nffs_cache_inode *cache_inode, uint32_t seek_offset,
return rc;
}
- TAILQ_INSERT_HEAD(&cache_inode->nci_block_list, cache_block,
- ncb_link);
+ nffs_cache_insert_block(cache_inode, cache_block, 0);
}
/* Calculate the file offset of the start of this block. This is used
@@ -406,12 +471,10 @@ nffs_cache_seek(struct nffs_cache_inode *cache_inode, uint32_t seek_offset,
if (last_cached_entry != NULL &&
last_cached_entry == pred_entry) {
- TAILQ_INSERT_TAIL(&cache_inode->nci_block_list,
- cache_block, ncb_link);
+ nffs_cache_insert_block(cache_inode, cache_block, 1);
} else {
nffs_cache_inode_free_blocks(cache_inode);
- TAILQ_INSERT_HEAD(&cache_inode->nci_block_list,
- cache_block, ncb_link);
+ nffs_cache_insert_block(cache_inode, cache_block, 0);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/2f430d3c/fs/nffs/src/nffs_gc.c
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_gc.c b/fs/nffs/src/nffs_gc.c
index b11c1aa..01882ea 100644
--- a/fs/nffs/src/nffs_gc.c
+++ b/fs/nffs/src/nffs_gc.c
@@ -24,6 +24,13 @@
#include "nffs_priv.h"
#include "nffs/nffs.h"
+/**
+ * Keeps track of the number of garbage collections performed. The exact
+ * number is not important, but it is useful to compare against an older copy
+ * to determine if garbage collection occurred.
+ */
+unsigned int nffs_gc_count;
+
static int
nffs_gc_copy_object(struct nffs_hash_entry *entry, uint16_t object_size,
uint8_t to_area_idx)
@@ -420,6 +427,18 @@ nffs_gc_inode_blocks(struct nffs_inode_entry *inode_entry,
* number is incremented prior to rewriting the header. This area is now
* the new scratch sector.
*
+ * NOTE:
+ * Garbage collection invalidates all cached data blocks. Whenever this
+ * function is called, all existing nffs_cache_block pointers are rendered
+ * invalid. If you maintain any such pointers, you need to reset them
+ * after calling this function. Cached inodes are not invalidated by
+ * garbage collection.
+ *
+ * If a parent function potentially calls this function, the caller of the
+ * parent function needs to explicitly check if garbage collection
+ * occurred. This is done by inspecting the nffs_gc_count variable before
+ * and after calling the function.
+ *
* @param out_area_idx On success, the ID of the cleaned up area gets
* written here. Pass null if you do not need
* this information.
@@ -502,6 +521,21 @@ nffs_gc(uint8_t *out_area_idx)
nffs_scratch_area_idx = from_area_idx;
+ /* Garbage collection renders the cache invalid:
+ * o All cached blocks are now invalid; drop them.
+ * o Flash locations of inodes may have changed; the cached inodes need
+ * updated to reflect this.
+ */
+ rc = nffs_cache_inode_refresh();
+ if (rc != 0) {
+ return rc;
+ }
+
+ /* Increment the garbage collection counter so that client code knows to
+ * reset its pointers to cached objects.
+ */
+ nffs_gc_count++;
+
return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/2f430d3c/fs/nffs/src/nffs_priv.h
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_priv.h b/fs/nffs/src/nffs_priv.h
index 7b1159a..a83256d 100644
--- a/fs/nffs/src/nffs_priv.h
+++ b/fs/nffs/src/nffs_priv.h
@@ -231,6 +231,7 @@ extern struct nffs_area *nffs_areas;
extern uint8_t nffs_num_areas;
extern uint8_t nffs_scratch_area_idx;
extern uint16_t nffs_block_max_data_sz;
+extern unsigned int nffs_gc_count;
#define NFFS_FLASH_BUF_SZ 256
extern uint8_t nffs_flash_buf[NFFS_FLASH_BUF_SZ];
@@ -278,6 +279,7 @@ int nffs_block_read_data(const struct nffs_block *block, uint16_t offset,
void nffs_cache_inode_delete(const struct nffs_inode_entry *inode_entry);
int nffs_cache_inode_ensure(struct nffs_cache_inode **out_entry,
struct nffs_inode_entry *inode_entry);
+int nffs_cache_inode_refresh(void);
void nffs_cache_inode_range(const struct nffs_cache_inode *cache_inode,
uint32_t *out_start, uint32_t *out_end);
int nffs_cache_seek(struct nffs_cache_inode *cache_inode, uint32_t to,
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/2f430d3c/fs/nffs/src/nffs_write.c
----------------------------------------------------------------------
diff --git a/fs/nffs/src/nffs_write.c b/fs/nffs/src/nffs_write.c
index 5bcc655..8888016 100644
--- a/fs/nffs/src/nffs_write.c
+++ b/fs/nffs/src/nffs_write.c
@@ -210,7 +210,7 @@ nffs_write_over_block(struct nffs_hash_entry *entry, uint16_t left_copy_len,
*/
static int
nffs_write_append(struct nffs_cache_inode *cache_inode, const void *data,
- uint16_t len)
+ uint16_t len)
{
struct nffs_inode_entry *inode_entry;
struct nffs_hash_entry *entry;
@@ -271,10 +271,12 @@ nffs_write_append(struct nffs_cache_inode *cache_inode, const void *data,
* @return 0 on success; nonzero on failure.
*/
static int
-nffs_write_chunk(struct nffs_cache_inode *cache_inode, uint32_t file_offset,
- const void *data, uint16_t data_len)
+nffs_write_chunk(struct nffs_inode_entry *inode_entry, uint32_t file_offset,
+ const void *data, uint16_t data_len)
{
+ struct nffs_cache_inode *cache_inode;
struct nffs_cache_block *cache_block;
+ unsigned int gc_count;
uint32_t append_len;
uint32_t data_offset;
uint32_t block_end;
@@ -285,6 +287,11 @@ nffs_write_chunk(struct nffs_cache_inode *cache_inode, uint32_t file_offset,
assert(data_len <= nffs_block_max_data_sz);
+ rc = nffs_cache_inode_ensure(&cache_inode, inode_entry);
+ if (rc != 0) {
+ return rc;
+ }
+
/** Handle the simple append case first. */
if (file_offset == cache_inode->nci_file_size) {
rc = nffs_write_append(cache_inode, data, data_len);
@@ -324,6 +331,12 @@ nffs_write_chunk(struct nffs_cache_inode *cache_inode, uint32_t file_offset,
chunk_sz += (int)(dst_off - block_end);
}
+ /* Remember the current garbage collection count. If the count
+ * increases during the write, then the block cache has been
+ * invalidated and we need to reset our pointers.
+ */
+ gc_count = nffs_gc_count;
+
data_offset = cache_block->ncb_file_offset + chunk_off - file_offset;
rc = nffs_write_over_block(cache_block->ncb_block.nb_hash_entry,
chunk_off, data + data_offset, chunk_sz);
@@ -332,7 +345,16 @@ nffs_write_chunk(struct nffs_cache_inode *cache_inode, uint32_t file_offset,
}
dst_off -= chunk_sz;
- cache_block = TAILQ_PREV(cache_block, nffs_cache_block_list, ncb_link);
+
+ if (gc_count != nffs_gc_count) {
+ /* Garbage collection occurred; the current cached block pointer is
+ * invalid, so reset it. The cached inode is still valid.
+ */
+ cache_block = NULL;
+ } else {
+ cache_block = TAILQ_PREV(cache_block, nffs_cache_block_list,
+ ncb_link);
+ }
} while (data_offset > 0);
cache_inode->nci_file_size += append_len;
@@ -385,7 +407,7 @@ nffs_write_to_file(struct nffs_file *file, const void *data, int len)
chunk_size = len;
}
- rc = nffs_write_chunk(cache_inode, file->nf_offset, data_ptr,
+ rc = nffs_write_chunk(file->nf_inode_entry, file->nf_offset, data_ptr,
chunk_size);
if (rc != 0) {
return rc;