[PATCH]Add erase count tracking support in JFFS2
Jörn Engel
joern at wohnheim.fh-wedel.de
Wed Aug 24 08:05:35 EDT 2005
On Wed, 24 August 2005 18:43:00 +0800, zhao forrest wrote:
>
> I refined the patch according to your comments. In particular
> I removed some unnecessary #ifdef, reconstruct function
> jffs2_mark_erased_block() to make it shorter and more clean.
Thou shalt not toppost.
Anyway, let's have another look...
> diff -auNrp ./mtd/fs/jffs2/build.c ./mtd_new/fs/jffs2/build.c
> --- ./mtd/fs/jffs2/build.c 2005-07-31 06:00:11.000000000 +0800
> +++ ./mtd_new/fs/jffs2/build.c 2005-08-18 13:33:55.000000000 +0800
> @@ -336,6 +336,10 @@ int jffs2_do_mount_fs(struct jffs2_sb_in
> c->blocks[i].first_node = NULL;
> c->blocks[i].last_node = NULL;
> c->blocks[i].bad_count = 0;
> +#ifdef CONFIG_JFFS2_FS_ERASE_COUNT_TRACKING
> + c->blocks[i].has_erase_count_node = 0;
> + c->blocks[i].erase_count = 0;
> +#endif
sizeof(struct jffs2_eraseblock) is growing with your code enabled.
That means we have yet another reason to use a fixed 1:1 physical
block : virtual block mapping.
Which means that your patch can't go in before someone has written the
necessary 1:1 mapping code.
Back to your patch, you should remove this #ifdef as well. Either
spend the memory for the two variables unconditionally and always
initialize them here, or create a helper function.
> }
>
> INIT_LIST_HEAD(&c->clean_list);
> diff -auNrp ./mtd/fs/jffs2/erase.c ./mtd_new/fs/jffs2/erase.c
> --- ./mtd/fs/jffs2/erase.c 2005-07-23 06:00:13.000000000 +0800
> +++ ./mtd_new/fs/jffs2/erase.c 2005-08-24 18:26:14.000000000 +0800
> @@ -209,7 +209,12 @@ static void jffs2_erase_callback(struct
> jffs2_erase_failed(priv->c, priv->jeb, instr->fail_addr);
> } else {
> jffs2_erase_succeeded(priv->c, priv->jeb);
> - }
> + }
> +
> +#ifdef CONFIG_JFFS2_FS_ERASE_COUNT_TRACKING
> + priv->jeb->erase_count++;
> + priv->c->total_erase_count++;
> +#endif
See above. Helper function or unconditional code.
> kfree(instr);
> }
> #endif /* !__ECOS */
> @@ -349,6 +354,188 @@ fail:
> return ret;
> }
>
> +#ifdef CONFIG_JFFS2_FS_ERASE_COUNT_TRACKING
> +static int write_erase_count_node(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
> +{
> + struct jffs2_raw_node_ref *erase_count_ref = NULL;
> + struct kvec vecs[1];
> + int ret;
> + size_t retlen;
> + struct jffs2_erase_count_node erase_count_node = {
> + .magic = cpu_to_je16(JFFS2_MAGIC_BITMASK),
> + .nodetype = cpu_to_je16(JFFS2_NODETYPE_ERASE_COUNT),
> + .totlen = cpu_to_je32(c->erasecount_size),
> + .erase_count = cpu_to_je32(jeb->erase_count)
> + };
> +
> + erase_count_ref = jffs2_alloc_raw_node_ref();
> + if (!erase_count_ref) {
> + printk(KERN_WARNING "Failed to allocate raw node ref for erase count node.\n");
> + return ENOMEM;
^^
Looks like a bug.
> + }
> + erase_count_node.hdr_crc = cpu_to_je32(crc32(0, &erase_count_node,
> + sizeof(struct jffs2_unknown_node)-4));
> + erase_count_node.node_crc = cpu_to_je32(crc32(0, &erase_count_node,
> + sizeof(struct jffs2_erase_count_node)-4));
> + vecs[0].iov_base = (unsigned char *) &erase_count_node;
> + vecs[0].iov_len = sizeof(erase_count_node);
> + ret = jffs2_flash_direct_writev(c, vecs, 1, jeb->offset + PAD(c->cleanmarker_size), &retlen);
Ferenc did the same thing before, now he has created a helper function
for this. I guess you should take a look at his patch.
Since the helper function would be useful for both patches, you might
want to add the helper function in a seperate patch which imo could go
right in.
> +
> + if (ret || retlen != sizeof(erase_count_node)) {
> + if (ret)
> + printk(KERN_WARNING "Write erase count to block at 0x%08x failed: %d\n",
> + jeb->offset, ret);
> + else
> + printk(KERN_WARNING "Short write to newly-erased block at 0x%08x: Wanted %zd, got %zd\n",
> + jeb->offset, sizeof(erase_count_node), retlen);
How much of this information is needed to debug problems? I think
jeb->offset, ret and retlen. So combine the two branches into a
single printk. Saves both code size and reading time.
> + jffs2_free_raw_node_ref(erase_count_ref);
> + return ret;
> + }
> +
> + erase_count_ref->next_in_ino = NULL;
> + erase_count_ref->next_phys = NULL;
> + erase_count_ref->flash_offset = (jeb->offset + PAD(c->cleanmarker_size)) | REF_NORMAL;
> + erase_count_ref->__totlen = PAD(c->erasecount_size);
> +
> + if (!jeb->first_node)
> + jeb->first_node = erase_count_ref;
> + if (jeb->last_node)
> + jeb->last_node->next_phys = erase_count_ref;
> + jeb->last_node = erase_count_ref;
> +
> + return 0;
> +}
> +
> +static void jffs2_mark_erased_block(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
> +{
> + struct jffs2_raw_node_ref *marker_ref = NULL;
> + size_t retlen;
> + int ret;
> + uint32_t bad_offset;
> +
> + switch (jffs2_block_check_erase(c, jeb, &bad_offset)) {
> + case -EAGAIN: goto refile;
> + case -EIO: goto filebad;
> + }
> +
> + /* Write the erase complete marker */
> + D1(printk(KERN_DEBUG "Writing erased marker to block at 0x%08x\n", jeb->offset));
> + bad_offset = jeb->offset;
> +
> + /* Cleanmarker in oob area or no cleanmarker at all ? */
> + if (jffs2_cleanmarker_oob(c) || c->cleanmarker_size == 0) {
> + if (jffs2_cleanmarker_oob(c)) {
> + if (jffs2_write_nand_cleanmarker(c, jeb))
> + goto filebad;
> +
> + jeb->first_node = jeb->last_node = NULL;
> + jeb->free_size = c->sector_size;
> + jeb->used_size = 0;
> + jeb->dirty_size = 0;
> + jeb->wasted_size = 0;
> + } else {
> + ret = write_erase_count_node(c, jeb);
> +
> + if (ret == ENOMEM)
^^
Ok, maybe the thing above was not a but, but general convention is to
return -ENOMEM and friends.
^
> + goto refile;
> + else if (ret)
> + goto filebad;
> +
> + jeb->free_size = c->sector_size - PAD(c->erasecount_size);
> + jeb->used_size = PAD(c->erasecount_size);
> + jeb->dirty_size = 0;
> + jeb->wasted_size = 0;
> + }
> + } else {
> +
> + struct kvec vecs[1];
> + struct jffs2_unknown_node marker = {
> + .magic = cpu_to_je16(JFFS2_MAGIC_BITMASK),
> + .nodetype = cpu_to_je16(JFFS2_NODETYPE_CLEANMARKER),
> + .totlen = cpu_to_je32(c->cleanmarker_size)
> + };
> +
> + marker_ref = jffs2_alloc_raw_node_ref();
> + if (!marker_ref) {
> + printk(KERN_WARNING "Failed to allocate raw node ref for clean marker. Refiling\n");
> + goto refile;
> + }
> +
> + marker.hdr_crc = cpu_to_je32(crc32(0, &marker, sizeof(struct jffs2_unknown_node)-4));
> +
> + vecs[0].iov_base = (unsigned char *) ▮
> + vecs[0].iov_len = sizeof(marker);
> + ret = jffs2_flash_direct_writev(c, vecs, 1, jeb->offset, &retlen);
> +
> + if (ret || retlen != sizeof(marker)) {
> + if (ret)
> + printk(KERN_WARNING "Write clean marker to block at 0x%08x failed: %d\n",
> + jeb->offset, ret);
> + else
> + printk(KERN_WARNING "Short write to newly-erased block at 0x%08x: Wanted %zd, got %zd\n",
> + jeb->offset, sizeof(marker), retlen);
> +
> + jffs2_free_raw_node_ref(marker_ref);
> + goto filebad;
> + }
> +
> + marker_ref->next_in_ino = NULL;
> + marker_ref->next_phys = NULL;
> + marker_ref->flash_offset = jeb->offset | REF_NORMAL;
> + marker_ref->__totlen = c->cleanmarker_size;
> +
> + jeb->first_node = jeb->last_node = marker_ref;
> +
> + ret = write_erase_count_node(c, jeb);
> +
> + if (ret) {
> + jffs2_free_raw_node_ref(marker_ref);
> + if (ret == ENOMEM)
^^
> + goto refile;
> + else if (ret)
> + goto filebad;
> + }
> +
> + jeb->free_size = c->sector_size - c->cleanmarker_size - PAD(c->erasecount_size);
> + jeb->used_size = c->cleanmarker_size + PAD(c->erasecount_size);
> + jeb->dirty_size = 0;
> + jeb->wasted_size = 0;
> + }
> +
> + spin_lock(&c->erase_completion_lock);
> + c->erasing_size -= c->sector_size;
> + c->free_size += jeb->free_size;
> + c->used_size += jeb->used_size;
> +
> + jffs2_dbg_acct_sanity_check_nolock(c,jeb);
> + jffs2_dbg_acct_paranoia_check_nolock(c, jeb);
> +
> + list_add_tail(&jeb->list, &c->free_list);
> + c->nr_erasing_blocks--;
> + c->nr_free_blocks++;
> + spin_unlock(&c->erase_completion_lock);
> + wake_up(&c->erase_wait);
> + return;
> +
> +filebad:
> + spin_lock(&c->erase_completion_lock);
> + /* Stick it on a list (any list) so erase_failed can take it
> + right off again. Silly, but shouldn't happen often. */
> + list_add(&jeb->list, &c->erasing_list);
> + spin_unlock(&c->erase_completion_lock);
> + jffs2_erase_failed(c, jeb, bad_offset);
> + return;
> +
> +refile:
> + /* Stick it back on the list from whence it came and come back later */
> + jffs2_erase_pending_trigger(c);
> + spin_lock(&c->erase_completion_lock);
> + list_add(&jeb->list, &c->erase_complete_list);
> + spin_unlock(&c->erase_completion_lock);
> + return;
> +}
> +#else
> +
> static void jffs2_mark_erased_block(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
> {
> struct jffs2_raw_node_ref *marker_ref = NULL;
> @@ -367,7 +554,6 @@ static void jffs2_mark_erased_block(stru
>
> /* Cleanmarker in oob area or no cleanmarker at all ? */
> if (jffs2_cleanmarker_oob(c) || c->cleanmarker_size == 0) {
> -
Can you create a seperate patch for the whitespace cleanups?
> if (jffs2_cleanmarker_oob(c)) {
> if (jffs2_write_nand_cleanmarker(c, jeb))
> goto filebad;
> @@ -378,7 +564,6 @@ static void jffs2_mark_erased_block(stru
> jeb->used_size = 0;
> jeb->dirty_size = 0;
> jeb->wasted_size = 0;
> -
> } else {
>
> struct kvec vecs[1];
> @@ -457,3 +642,4 @@ refile:
> spin_unlock(&c->erase_completion_lock);
> return;
> }
> +#endif
...
> diff -auNrp ./mtd/include/linux/jffs2_fs_sb.h ./mtd_new/include/linux/jffs2_fs_sb.h
> --- ./mtd/include/linux/jffs2_fs_sb.h 2005-05-20 06:00:14.000000000 +0800
> +++ ./mtd_new/include/linux/jffs2_fs_sb.h 2005-08-18 13:28:36.000000000 +0800
> @@ -40,7 +40,9 @@ struct jffs2_sb_info {
> out-of-order writing of nodes. And GC. */
> uint32_t cleanmarker_size; /* Size of an _inline_ CLEANMARKER
> (i.e. zero for OOB CLEANMARKER */
> -
> +#ifdef CONFIG_JFFS2_FS_ERASE_COUNT_TRACKING
> + uint32_t erasecount_size; /* size of an _inline_ erase-count node */
> +#endif
> uint32_t flash_size;
> uint32_t used_size;
> uint32_t dirty_size;
> @@ -89,6 +91,10 @@ struct jffs2_sb_info {
> wait_queue_head_t inocache_wq;
> struct jffs2_inode_cache **inocache_list;
> spinlock_t inocache_lock;
> +
> +#ifdef CONFIG_JFFS2_FS_ERASE_COUNT_TRACKING
> + uint32_t total_erase_count; /* The summary erase count of all erase blocks on a Flash */
> +#endif
This field is write-only? What did you plan to do with it?
>
> /* Sem to allow jffs2_garbage_collect_deletion_dirent to
> drop the erase_completion_lock while it's holding a pointer
> diff -auNrp ./mtd/include/linux/jffs2.h ./mtd_new/include/linux/jffs2.h
> --- ./mtd/include/linux/jffs2.h 2005-07-27 06:00:09.000000000 +0800
> +++ ./mtd_new/include/linux/jffs2.h 2005-08-24 18:25:46.000000000 +0800
> @@ -60,6 +60,8 @@
> #define JFFS2_NODETYPE_CLEANMARKER (JFFS2_FEATURE_RWCOMPAT_DELETE | JFFS2_NODE_ACCURATE | 3)
> #define JFFS2_NODETYPE_PADDING (JFFS2_FEATURE_RWCOMPAT_DELETE | JFFS2_NODE_ACCURATE | 4)
>
> +#define JFFS2_NODETYPE_ERASE_COUNT (JFFS2_FEATURE_RWCOMPAT_DELETE | JFFS2_NODE_ACCURATE | 5)
> +
> // Maybe later...
> //#define JFFS2_NODETYPE_CHECKPOINT (JFFS2_FEATURE_RWCOMPAT_DELETE | JFFS2_NODE_ACCURATE | 3)
> //#define JFFS2_NODETYPE_OPTIONS (JFFS2_FEATURE_RWCOMPAT_COPY | JFFS2_NODE_ACCURATE | 4)
> @@ -146,10 +148,28 @@ struct jffs2_raw_inode
> uint8_t data[0];
> } __attribute__((packed));
>
> +struct jffs2_erase_count_node
> +{
> + jint16_t magic; /* A constant magic number. */
> + jint16_t nodetype; /* == JFFS2_NODETYPE_ERASE_COUNT */
> + jint32_t totlen;
> + jint32_t hdr_crc;
> + jint32_t erase_count; /* used to track the erase counts of each erase block */
> + jint32_t node_crc;
> +} __attribute__((packed));
I hope Artem is peeking here. What we could do is split this patch
into two parts:
1. data defitions
2. code to use that data
Then 1) could go in rather quickly and the existance of a struct
jffs2_erase_count_node on flash can be used as an indication of a 1:1
mapping. We merge the code to deal with a 1:1 mapping, let it settle
for a while and then merge 2).
Comments?
> +struct jffs2_erase_count_node_oob
> +{
> + jint16_t magic; /* A constant magic number. */
> + jint16_t nodetype; /* == JFFS2_NODETYPE_ERASE_COUNT */
> + jint32_t erase_count; /* used to track the erase counts of each erase block */
> +} __attribute__((packed));
> +
> union jffs2_node_union {
> struct jffs2_raw_inode i;
> struct jffs2_raw_dirent d;
> struct jffs2_unknown_node u;
> + struct jffs2_erase_count_node e;
> };
>
> #endif /* __LINUX_JFFS2_H__ */
Jörn
--
Public Domain - Free as in Beer
General Public - Free as in Speech
BSD License - Free as in Enterprise
Shared Source - Free as in "Work will make you..."
More information about the linux-mtd
mailing list