* [PATCH 1/2] jffs:2 Move erasing from write_super to GC. @ 2010-02-15 16:03 Joakim Tjernlund 2010-02-15 16:03 ` [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ Joakim Tjernlund 2010-02-16 8:57 ` [PATCH 1/2] jffs:2 Move erasing from write_super to GC Artem Bityutskiy 0 siblings, 2 replies; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-15 16:03 UTC (permalink / raw) To: linux-mtd; +Cc: Joakim Tjernlund Erasing blocks is a form of GC and therefor it should live in the GC task. By moving it there two problems will be solved: 1) umounting will not hang until all pending blocks has been erased. 2) Erasing can be paused by sending a SIGSTOP to the GC thread which allowes for time critical tasks work in peace. Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> --- fs/jffs2/background.c | 1 + fs/jffs2/erase.c | 5 +++++ fs/jffs2/nodemgmt.c | 4 ++++ fs/jffs2/super.c | 1 - 4 files changed, 10 insertions(+), 1 deletions(-) diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c index 3ff50da..a8e0140 100644 --- a/fs/jffs2/background.c +++ b/fs/jffs2/background.c @@ -146,6 +146,7 @@ static int jffs2_garbage_collect_thread(void *_c) disallow_signal(SIGHUP); D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): pass\n")); + jffs2_erase_pending_blocks(c, 0); if (jffs2_garbage_collect_pass(c) == -ENOSPC) { printk(KERN_NOTICE "No space for garbage collection. Aborting GC thread\n"); goto die; diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c index b47679b..1ca2559 100644 --- a/fs/jffs2/erase.c +++ b/fs/jffs2/erase.c @@ -114,6 +114,11 @@ void jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count) while (!list_empty(&c->erase_complete_list) || !list_empty(&c->erase_pending_list)) { + if (signal_pending(current)) { + spin_unlock(&c->erase_completion_lock); + mutex_unlock(&c->erase_free_sem); + goto done; + } if (!list_empty(&c->erase_complete_list)) { jeb = list_entry(c->erase_complete_list.next, struct jffs2_eraseblock, list); list_move(&jeb->list, &c->erase_checking_list); diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c index 21a0529..155fd63 100644 --- a/fs/jffs2/nodemgmt.c +++ b/fs/jffs2/nodemgmt.c @@ -733,6 +733,10 @@ int jffs2_thread_should_wake(struct jffs2_sb_info *c) int nr_very_dirty = 0; struct jffs2_eraseblock *jeb; + if (!list_empty(&c->erase_complete_list) || + !list_empty(&c->erase_pending_list)) + return 1; + if (c->unchecked_size) { D1(printk(KERN_DEBUG "jffs2_thread_should_wake(): unchecked_size %d, checked_ino #%d\n", c->unchecked_size, c->checked_ino)); diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c index 9a80e8e..5162329 100644 --- a/fs/jffs2/super.c +++ b/fs/jffs2/super.c @@ -64,7 +64,6 @@ static void jffs2_write_super(struct super_block *sb) if (!(sb->s_flags & MS_RDONLY)) { D1(printk(KERN_DEBUG "jffs2_write_super()\n")); jffs2_garbage_collect_trigger(c); - jffs2_erase_pending_blocks(c, 0); jffs2_flush_wbuf_gc(c, 0); } -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-15 16:03 [PATCH 1/2] jffs:2 Move erasing from write_super to GC Joakim Tjernlund @ 2010-02-15 16:03 ` Joakim Tjernlund 2010-02-16 8:59 ` Artem Bityutskiy 2010-02-16 8:57 ` [PATCH 1/2] jffs:2 Move erasing from write_super to GC Artem Bityutskiy 1 sibling, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-15 16:03 UTC (permalink / raw) To: linux-mtd; +Cc: Joakim Tjernlund Since erasing is done in GC now, trigger GC instead. jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and used by wbuf. Remove call jffs2_garbage_collect_trigger() in write_super() Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> --- fs/jffs2/erase.c | 4 +--- fs/jffs2/gc.c | 2 +- fs/jffs2/nodemgmt.c | 4 ++-- fs/jffs2/os-linux.h | 2 +- fs/jffs2/scan.c | 2 +- fs/jffs2/super.c | 1 - fs/jffs2/wbuf.c | 8 ++++---- 7 files changed, 10 insertions(+), 13 deletions(-) diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c index 1ca2559..fdf9418 100644 --- a/fs/jffs2/erase.c +++ b/fs/jffs2/erase.c @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblo list_move_tail(&jeb->list, &c->erase_complete_list); spin_unlock(&c->erase_completion_lock); mutex_unlock(&c->erase_free_sem); - /* Ensure that kupdated calls us again to mark them clean */ - jffs2_erase_pending_trigger(c); } static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb, uint32_t bad_offset) @@ -492,7 +490,7 @@ filebad: refile: /* Stick it back on the list from whence it came and come back later */ - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); mutex_lock(&c->erase_free_sem); spin_lock(&c->erase_completion_lock); list_move(&jeb->list, &c->erase_complete_list); diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c index 3b6f2fa..005659f 100644 --- a/fs/jffs2/gc.c +++ b/fs/jffs2/gc.c @@ -435,7 +435,7 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c) list_add_tail(&c->gcblock->list, &c->erase_pending_list); c->gcblock = NULL; c->nr_erasing_blocks++; - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); } spin_unlock(&c->erase_completion_lock); diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c index 155fd63..190b5ce 100644 --- a/fs/jffs2/nodemgmt.c +++ b/fs/jffs2/nodemgmt.c @@ -218,7 +218,7 @@ static int jffs2_find_nextblock(struct jffs2_sb_info *c) ejeb = list_entry(c->erasable_list.next, struct jffs2_eraseblock, list); list_move_tail(&ejeb->list, &c->erase_pending_list); c->nr_erasing_blocks++; - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); D1(printk(KERN_DEBUG "jffs2_find_nextblock: Triggering erase of erasable block at 0x%08x\n", ejeb->offset)); } @@ -612,7 +612,7 @@ void jffs2_mark_node_obsolete(struct jffs2_sb_info *c, struct jffs2_raw_node_ref D1(printk(KERN_DEBUG "...and adding to erase_pending_list\n")); list_add_tail(&jeb->list, &c->erase_pending_list); c->nr_erasing_blocks++; - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); } else { /* Sometimes, however, we leave it elsewhere so it doesn't get immediately reused, and we spread the load a bit. */ diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h index a7f03b7..4d03ce0 100644 --- a/fs/jffs2/os-linux.h +++ b/fs/jffs2/os-linux.h @@ -141,7 +141,7 @@ void jffs2_nor_wbuf_flash_cleanup(struct jffs2_sb_info *c); #endif /* WRITEBUFFER */ /* erase.c */ -static inline void jffs2_erase_pending_trigger(struct jffs2_sb_info *c) +static inline void jffs2_dirty_trigger(struct jffs2_sb_info *c) { OFNI_BS_2SFFJ(c)->s_dirt = 1; } diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c index 2a0681b..d6bdcfc 100644 --- a/fs/jffs2/scan.c +++ b/fs/jffs2/scan.c @@ -260,7 +260,7 @@ int jffs2_scan_medium(struct jffs2_sb_info *c) ret = -EIO; goto out; } - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); } ret = 0; out: diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c index 5162329..511e2d6 100644 --- a/fs/jffs2/super.c +++ b/fs/jffs2/super.c @@ -63,7 +63,6 @@ static void jffs2_write_super(struct super_block *sb) if (!(sb->s_flags & MS_RDONLY)) { D1(printk(KERN_DEBUG "jffs2_write_super()\n")); - jffs2_garbage_collect_trigger(c); jffs2_flush_wbuf_gc(c, 0); } diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c index 5ef7bac..07ee154 100644 --- a/fs/jffs2/wbuf.c +++ b/fs/jffs2/wbuf.c @@ -84,7 +84,7 @@ static void jffs2_wbuf_dirties_inode(struct jffs2_sb_info *c, uint32_t ino) struct jffs2_inodirty *new; /* Mark the superblock dirty so that kupdated will flush... */ - jffs2_erase_pending_trigger(c); + jffs2_dirty_trigger(c); if (jffs2_wbuf_pending_for_ino(c, ino)) return; @@ -121,7 +121,7 @@ static inline void jffs2_refile_wbuf_blocks(struct jffs2_sb_info *c) D1(printk(KERN_DEBUG "...and adding to erase_pending_list\n")); list_add_tail(&jeb->list, &c->erase_pending_list); c->nr_erasing_blocks++; - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); } else { /* Sometimes, however, we leave it elsewhere so it doesn't get immediately reused, and we spread the load a bit. */ @@ -152,7 +152,7 @@ static void jffs2_block_refile(struct jffs2_sb_info *c, struct jffs2_eraseblock D1(printk("Refiling block at %08x to erase_pending_list\n", jeb->offset)); list_add(&jeb->list, &c->erase_pending_list); c->nr_erasing_blocks++; - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); } if (!jffs2_prealloc_raw_node_refs(c, jeb, 1)) { @@ -543,7 +543,7 @@ static void jffs2_wbuf_recover(struct jffs2_sb_info *c) D1(printk(KERN_DEBUG "Failing block at %08x is now empty. Moving to erase_pending_list\n", jeb->offset)); list_move(&jeb->list, &c->erase_pending_list); c->nr_erasing_blocks++; - jffs2_erase_pending_trigger(c); + jffs2_garbage_collect_trigger(c); } jffs2_dbg_acct_sanity_check_nolock(c, jeb); -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-15 16:03 ` [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ Joakim Tjernlund @ 2010-02-16 8:59 ` Artem Bityutskiy 2010-02-16 9:32 ` Joakim Tjernlund 2010-02-16 14:27 ` Joakim Tjernlund 0 siblings, 2 replies; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-16 8:59 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > Since erasing is done in GC now, trigger GC instead. > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > write_super() > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > --- > fs/jffs2/erase.c | 4 +--- > fs/jffs2/gc.c | 2 +- > fs/jffs2/nodemgmt.c | 4 ++-- > fs/jffs2/os-linux.h | 2 +- > fs/jffs2/scan.c | 2 +- > fs/jffs2/super.c | 1 - > fs/jffs2/wbuf.c | 8 ++++---- > 7 files changed, 10 insertions(+), 13 deletions(-) > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > index 1ca2559..fdf9418 100644 > --- a/fs/jffs2/erase.c > +++ b/fs/jffs2/erase.c > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblo > list_move_tail(&jeb->list, &c->erase_complete_list); > spin_unlock(&c->erase_completion_lock); > mutex_unlock(&c->erase_free_sem); > - /* Ensure that kupdated calls us again to mark them clean */ > - jffs2_erase_pending_trigger(c); > } > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb, uint32_t bad_offset) > @@ -492,7 +490,7 @@ filebad: > > refile: > /* Stick it back on the list from whence it came and come back later */ > - jffs2_erase_pending_trigger(c); > + jffs2_garbage_collect_trigger(c); But then you make the code more confusing. Indeed, readability becomes worse. I would just change 'jffs2_erase_pending_trigger()' and make it wake up the GC thread, just like 'jffs2_garbage_collect_trigger()'... -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-16 8:59 ` Artem Bityutskiy @ 2010-02-16 9:32 ` Joakim Tjernlund 2010-02-16 14:27 ` Joakim Tjernlund 1 sibling, 0 replies; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-16 9:32 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:59:49: > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > Since erasing is done in GC now, trigger GC instead. > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > > write_super() > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > --- > > fs/jffs2/erase.c | 4 +--- > > fs/jffs2/gc.c | 2 +- > > fs/jffs2/nodemgmt.c | 4 ++-- > > fs/jffs2/os-linux.h | 2 +- > > fs/jffs2/scan.c | 2 +- > > fs/jffs2/super.c | 1 - > > fs/jffs2/wbuf.c | 8 ++++---- > > 7 files changed, 10 insertions(+), 13 deletions(-) > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > index 1ca2559..fdf9418 100644 > > --- a/fs/jffs2/erase.c > > +++ b/fs/jffs2/erase.c > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > *c, struct jffs2_eraseblo > > list_move_tail(&jeb->list, &c->erase_complete_list); > > spin_unlock(&c->erase_completion_lock); > > mutex_unlock(&c->erase_free_sem); > > - /* Ensure that kupdated calls us again to mark them clean */ > > - jffs2_erase_pending_trigger(c); > > } > > > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct > jffs2_eraseblock *jeb, uint32_t bad_offset) > > @@ -492,7 +490,7 @@ filebad: > > > > refile: > > /* Stick it back on the list from whence it came and come back later */ > > - jffs2_erase_pending_trigger(c); > > + jffs2_garbage_collect_trigger(c); > > But then you make the code more confusing. Indeed, readability becomes > worse. > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up > the GC thread, just like 'jffs2_garbage_collect_trigger()'... Depends on your point of view, I think erasing blocks is GC so to me it makes sense. However I acknowledge that erasing is different from other forms of GC so I can change it back if you really want me too. I will have to keep jffs2_dirty_trigger() though as wbuf needs to a way to flush its buffer. Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-16 8:59 ` Artem Bityutskiy 2010-02-16 9:32 ` Joakim Tjernlund @ 2010-02-16 14:27 ` Joakim Tjernlund 2010-02-17 7:18 ` Artem Bityutskiy 1 sibling, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-16 14:27 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:59:49: > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > Since erasing is done in GC now, trigger GC instead. > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > > write_super() > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > --- > > fs/jffs2/erase.c | 4 +--- > > fs/jffs2/gc.c | 2 +- > > fs/jffs2/nodemgmt.c | 4 ++-- > > fs/jffs2/os-linux.h | 2 +- > > fs/jffs2/scan.c | 2 +- > > fs/jffs2/super.c | 1 - > > fs/jffs2/wbuf.c | 8 ++++---- > > 7 files changed, 10 insertions(+), 13 deletions(-) > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > index 1ca2559..fdf9418 100644 > > --- a/fs/jffs2/erase.c > > +++ b/fs/jffs2/erase.c > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > *c, struct jffs2_eraseblo > > list_move_tail(&jeb->list, &c->erase_complete_list); > > spin_unlock(&c->erase_completion_lock); > > mutex_unlock(&c->erase_free_sem); > > - /* Ensure that kupdated calls us again to mark them clean */ > > - jffs2_erase_pending_trigger(c); > > } > > > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct > jffs2_eraseblock *jeb, uint32_t bad_offset) > > @@ -492,7 +490,7 @@ filebad: > > > > refile: > > /* Stick it back on the list from whence it came and come back later */ > > - jffs2_erase_pending_trigger(c); > > + jffs2_garbage_collect_trigger(c); > > But then you make the code more confusing. Indeed, readability becomes > worse. > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up > the GC thread, just like 'jffs2_garbage_collect_trigger()'... Here we go then: >From 96a4a9dc054f2dbd57e180e202f69c9645536e0d Mon Sep 17 00:00:00 2001 From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> Date: Tue, 16 Feb 2010 15:18:31 +0100 Subject: [PATCH] jffs2: Make jffs2_erase_pending_trigger() initiate GC. Since erasing is now in the GC thread, erases should trigger the GC task instead. wbuf.c still wants to flush its buffer via write_super so invent jffs2_dirty_trigger() and use that in wbuf. Remove surplus call to jffs2_erase_pending_trigger() in erase.c and remove jffs2_garbage_collect_trigger() from write_super as of now write_super() should only commit dirty data to disk. Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> --- fs/jffs2/erase.c | 2 -- fs/jffs2/os-linux.h | 9 +++++++-- fs/jffs2/super.c | 1 - fs/jffs2/wbuf.c | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c index 1ca2559..5616658 100644 --- a/fs/jffs2/erase.c +++ b/fs/jffs2/erase.c @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblo list_move_tail(&jeb->list, &c->erase_complete_list); spin_unlock(&c->erase_completion_lock); mutex_unlock(&c->erase_free_sem); - /* Ensure that kupdated calls us again to mark them clean */ - jffs2_erase_pending_trigger(c); } static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb, uint32_t bad_offset) diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h index a7f03b7..5d26362 100644 --- a/fs/jffs2/os-linux.h +++ b/fs/jffs2/os-linux.h @@ -140,8 +140,7 @@ void jffs2_nor_wbuf_flash_cleanup(struct jffs2_sb_info *c); #endif /* WRITEBUFFER */ -/* erase.c */ -static inline void jffs2_erase_pending_trigger(struct jffs2_sb_info *c) +static inline void jffs2_dirty_trigger(struct jffs2_sb_info *c) { OFNI_BS_2SFFJ(c)->s_dirt = 1; } @@ -151,6 +150,12 @@ int jffs2_start_garbage_collect_thread(struct jffs2_sb_info *c); void jffs2_stop_garbage_collect_thread(struct jffs2_sb_info *c); void jffs2_garbage_collect_trigger(struct jffs2_sb_info *c); +/* erase.c */ +static inline void jffs2_erase_pending_trigger(struct jffs2_sb_info *c) +{ + jffs2_garbage_collect_trigger(c); +} + /* dir.c */ extern const struct file_operations jffs2_dir_operations; extern const struct inode_operations jffs2_dir_inode_operations; diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c index 5162329..511e2d6 100644 --- a/fs/jffs2/super.c +++ b/fs/jffs2/super.c @@ -63,7 +63,6 @@ static void jffs2_write_super(struct super_block *sb) if (!(sb->s_flags & MS_RDONLY)) { D1(printk(KERN_DEBUG "jffs2_write_super()\n")); - jffs2_garbage_collect_trigger(c); jffs2_flush_wbuf_gc(c, 0); } diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c index 5ef7bac..f319efc 100644 --- a/fs/jffs2/wbuf.c +++ b/fs/jffs2/wbuf.c @@ -84,7 +84,7 @@ static void jffs2_wbuf_dirties_inode(struct jffs2_sb_info *c, uint32_t ino) struct jffs2_inodirty *new; /* Mark the superblock dirty so that kupdated will flush... */ - jffs2_erase_pending_trigger(c); + jffs2_dirty_trigger(c); if (jffs2_wbuf_pending_for_ino(c, ino)) return; -- 1.6.4.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-16 14:27 ` Joakim Tjernlund @ 2010-02-17 7:18 ` Artem Bityutskiy 2010-02-17 7:35 ` Joakim Tjernlund 0 siblings, 1 reply; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-17 7:18 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Tue, 2010-02-16 at 15:27 +0100, Joakim Tjernlund wrote: > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:59:49: > > > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > > Since erasing is done in GC now, trigger GC instead. > > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > > > write_super() > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > --- > > > fs/jffs2/erase.c | 4 +--- > > > fs/jffs2/gc.c | 2 +- > > > fs/jffs2/nodemgmt.c | 4 ++-- > > > fs/jffs2/os-linux.h | 2 +- > > > fs/jffs2/scan.c | 2 +- > > > fs/jffs2/super.c | 1 - > > > fs/jffs2/wbuf.c | 8 ++++---- > > > 7 files changed, 10 insertions(+), 13 deletions(-) > > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > > index 1ca2559..fdf9418 100644 > > > --- a/fs/jffs2/erase.c > > > +++ b/fs/jffs2/erase.c > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > > *c, struct jffs2_eraseblo > > > list_move_tail(&jeb->list, &c->erase_complete_list); > > > spin_unlock(&c->erase_completion_lock); > > > mutex_unlock(&c->erase_free_sem); > > > - /* Ensure that kupdated calls us again to mark them clean */ > > > - jffs2_erase_pending_trigger(c); > > > } > > > > > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct > > jffs2_eraseblock *jeb, uint32_t bad_offset) > > > @@ -492,7 +490,7 @@ filebad: > > > > > > refile: > > > /* Stick it back on the list from whence it came and come back later */ > > > - jffs2_erase_pending_trigger(c); > > > + jffs2_garbage_collect_trigger(c); > > > > But then you make the code more confusing. Indeed, readability becomes > > worse. > > > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up > > the GC thread, just like 'jffs2_garbage_collect_trigger()'... > > Here we go then: > > From 96a4a9dc054f2dbd57e180e202f69c9645536e0d Mon Sep 17 00:00:00 2001 > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > Date: Tue, 16 Feb 2010 15:18:31 +0100 > Subject: [PATCH] jffs2: Make jffs2_erase_pending_trigger() initiate GC. > > Since erasing is now in the GC thread, erases should trigger > the GC task instead. > wbuf.c still wants to flush its buffer via write_super so > invent jffs2_dirty_trigger() and use that in wbuf. > Remove surplus call to jffs2_erase_pending_trigger() in erase.c > and remove jffs2_garbage_collect_trigger() from write_super as > of now write_super() should only commit dirty data to disk. > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > --- > fs/jffs2/erase.c | 2 -- > fs/jffs2/os-linux.h | 9 +++++++-- > fs/jffs2/super.c | 1 - > fs/jffs2/wbuf.c | 2 +- > 4 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > index 1ca2559..5616658 100644 > --- a/fs/jffs2/erase.c > +++ b/fs/jffs2/erase.c > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblo > list_move_tail(&jeb->list, &c->erase_complete_list); > spin_unlock(&c->erase_completion_lock); > mutex_unlock(&c->erase_free_sem); > - /* Ensure that kupdated calls us again to mark them clean */ > - jffs2_erase_pending_trigger(c); > } Looks like BGT should be triggered from here in order to write the clean marker. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-17 7:18 ` Artem Bityutskiy @ 2010-02-17 7:35 ` Joakim Tjernlund 2010-02-17 7:48 ` Artem Bityutskiy 0 siblings, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-17 7:35 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/17 08:18:23: > > On Tue, 2010-02-16 at 15:27 +0100, Joakim Tjernlund wrote: > > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:59:49: > > > > > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > > > Since erasing is done in GC now, trigger GC instead. > > > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > > > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > > > > write_super() > > > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > > --- > > > > fs/jffs2/erase.c | 4 +--- > > > > fs/jffs2/gc.c | 2 +- > > > > fs/jffs2/nodemgmt.c | 4 ++-- > > > > fs/jffs2/os-linux.h | 2 +- > > > > fs/jffs2/scan.c | 2 +- > > > > fs/jffs2/super.c | 1 - > > > > fs/jffs2/wbuf.c | 8 ++++---- > > > > 7 files changed, 10 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > > > index 1ca2559..fdf9418 100644 > > > > --- a/fs/jffs2/erase.c > > > > +++ b/fs/jffs2/erase.c > > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > > > *c, struct jffs2_eraseblo > > > > list_move_tail(&jeb->list, &c->erase_complete_list); > > > > spin_unlock(&c->erase_completion_lock); > > > > mutex_unlock(&c->erase_free_sem); > > > > - /* Ensure that kupdated calls us again to mark them clean */ > > > > - jffs2_erase_pending_trigger(c); > > > > } > > > > > > > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct > > > jffs2_eraseblock *jeb, uint32_t bad_offset) > > > > @@ -492,7 +490,7 @@ filebad: > > > > > > > > refile: > > > > /* Stick it back on the list from whence it came and come back later */ > > > > - jffs2_erase_pending_trigger(c); > > > > + jffs2_garbage_collect_trigger(c); > > > > > > But then you make the code more confusing. Indeed, readability becomes > > > worse. > > > > > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up > > > the GC thread, just like 'jffs2_garbage_collect_trigger()'... > > > > Here we go then: > > > > From 96a4a9dc054f2dbd57e180e202f69c9645536e0d Mon Sep 17 00:00:00 2001 > > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > Date: Tue, 16 Feb 2010 15:18:31 +0100 > > Subject: [PATCH] jffs2: Make jffs2_erase_pending_trigger() initiate GC. > > > > Since erasing is now in the GC thread, erases should trigger > > the GC task instead. > > wbuf.c still wants to flush its buffer via write_super so > > invent jffs2_dirty_trigger() and use that in wbuf. > > Remove surplus call to jffs2_erase_pending_trigger() in erase.c > > and remove jffs2_garbage_collect_trigger() from write_super as > > of now write_super() should only commit dirty data to disk. > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > --- > > fs/jffs2/erase.c | 2 -- > > fs/jffs2/os-linux.h | 9 +++++++-- > > fs/jffs2/super.c | 1 - > > fs/jffs2/wbuf.c | 2 +- > > 4 files changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > index 1ca2559..5616658 100644 > > --- a/fs/jffs2/erase.c > > +++ b/fs/jffs2/erase.c > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > *c, struct jffs2_eraseblo > > list_move_tail(&jeb->list, &c->erase_complete_list); > > spin_unlock(&c->erase_completion_lock); > > mutex_unlock(&c->erase_free_sem); > > - /* Ensure that kupdated calls us again to mark them clean */ > > - jffs2_erase_pending_trigger(c); > > } > > Looks like BGT should be triggered from here in order to write the clean > marker. How so? JFFS2 is already running jffs2_erase_pending_blocks() and has just completed an erase, the next thing it will do is to mark it with a clean marker. To me it looks like belts and suspenders: kick it again just in case we missed something. Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-17 7:35 ` Joakim Tjernlund @ 2010-02-17 7:48 ` Artem Bityutskiy 2010-02-17 7:55 ` Joakim Tjernlund 0 siblings, 1 reply; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-17 7:48 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Wed, 2010-02-17 at 08:35 +0100, Joakim Tjernlund wrote: > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/17 08:18:23: > > > > On Tue, 2010-02-16 at 15:27 +0100, Joakim Tjernlund wrote: > > > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:59:49: > > > > > > > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > > > > Since erasing is done in GC now, trigger GC instead. > > > > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > > > > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > > > > > write_super() > > > > > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > > > --- > > > > > fs/jffs2/erase.c | 4 +--- > > > > > fs/jffs2/gc.c | 2 +- > > > > > fs/jffs2/nodemgmt.c | 4 ++-- > > > > > fs/jffs2/os-linux.h | 2 +- > > > > > fs/jffs2/scan.c | 2 +- > > > > > fs/jffs2/super.c | 1 - > > > > > fs/jffs2/wbuf.c | 8 ++++---- > > > > > 7 files changed, 10 insertions(+), 13 deletions(-) > > > > > > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > > > > index 1ca2559..fdf9418 100644 > > > > > --- a/fs/jffs2/erase.c > > > > > +++ b/fs/jffs2/erase.c > > > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > > > > *c, struct jffs2_eraseblo > > > > > list_move_tail(&jeb->list, &c->erase_complete_list); > > > > > spin_unlock(&c->erase_completion_lock); > > > > > mutex_unlock(&c->erase_free_sem); > > > > > - /* Ensure that kupdated calls us again to mark them clean */ > > > > > - jffs2_erase_pending_trigger(c); > > > > > } > > > > > > > > > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct > > > > jffs2_eraseblock *jeb, uint32_t bad_offset) > > > > > @@ -492,7 +490,7 @@ filebad: > > > > > > > > > > refile: > > > > > /* Stick it back on the list from whence it came and come back later */ > > > > > - jffs2_erase_pending_trigger(c); > > > > > + jffs2_garbage_collect_trigger(c); > > > > > > > > But then you make the code more confusing. Indeed, readability becomes > > > > worse. > > > > > > > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up > > > > the GC thread, just like 'jffs2_garbage_collect_trigger()'... > > > > > > Here we go then: > > > > > > From 96a4a9dc054f2dbd57e180e202f69c9645536e0d Mon Sep 17 00:00:00 2001 > > > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > Date: Tue, 16 Feb 2010 15:18:31 +0100 > > > Subject: [PATCH] jffs2: Make jffs2_erase_pending_trigger() initiate GC. > > > > > > Since erasing is now in the GC thread, erases should trigger > > > the GC task instead. > > > wbuf.c still wants to flush its buffer via write_super so > > > invent jffs2_dirty_trigger() and use that in wbuf. > > > Remove surplus call to jffs2_erase_pending_trigger() in erase.c > > > and remove jffs2_garbage_collect_trigger() from write_super as > > > of now write_super() should only commit dirty data to disk. > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > --- > > > fs/jffs2/erase.c | 2 -- > > > fs/jffs2/os-linux.h | 9 +++++++-- > > > fs/jffs2/super.c | 1 - > > > fs/jffs2/wbuf.c | 2 +- > > > 4 files changed, 8 insertions(+), 6 deletions(-) > > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > > index 1ca2559..5616658 100644 > > > --- a/fs/jffs2/erase.c > > > +++ b/fs/jffs2/erase.c > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > > *c, struct jffs2_eraseblo > > > list_move_tail(&jeb->list, &c->erase_complete_list); > > > spin_unlock(&c->erase_completion_lock); > > > mutex_unlock(&c->erase_free_sem); > > > - /* Ensure that kupdated calls us again to mark them clean */ > > > - jffs2_erase_pending_trigger(c); > > > } > > > > Looks like BGT should be triggered from here in order to write the clean > > marker. > > How so? JFFS2 is already running jffs2_erase_pending_blocks() and > has just completed an erase, the next thing it will do is to mark it with a clean > marker. To me it looks like belts and suspenders: kick it again just in case we missed > something. OK. I expect you will send the final version of your 2 patches, then I'll put them to my l2 tree, right? -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-17 7:48 ` Artem Bityutskiy @ 2010-02-17 7:55 ` Joakim Tjernlund 2010-02-17 8:07 ` Artem Bityutskiy 0 siblings, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-17 7:55 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/17 08:48:28: > Subject: Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ > > On Wed, 2010-02-17 at 08:35 +0100, Joakim Tjernlund wrote: > > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/17 08:18:23: > > > > > > On Tue, 2010-02-16 at 15:27 +0100, Joakim Tjernlund wrote: > > > > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:59:49: > > > > > > > > > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > > > > > Since erasing is done in GC now, trigger GC instead. > > > > > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and > > > > > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in > > > > > > write_super() > > > > > > > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > > > > --- > > > > > > fs/jffs2/erase.c | 4 +--- > > > > > > fs/jffs2/gc.c | 2 +- > > > > > > fs/jffs2/nodemgmt.c | 4 ++-- > > > > > > fs/jffs2/os-linux.h | 2 +- > > > > > > fs/jffs2/scan.c | 2 +- > > > > > > fs/jffs2/super.c | 1 - > > > > > > fs/jffs2/wbuf.c | 8 ++++---- > > > > > > 7 files changed, 10 insertions(+), 13 deletions(-) > > > > > > > > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > > > > > index 1ca2559..fdf9418 100644 > > > > > > --- a/fs/jffs2/erase.c > > > > > > +++ b/fs/jffs2/erase.c > > > > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > > > > > *c, struct jffs2_eraseblo > > > > > > list_move_tail(&jeb->list, &c->erase_complete_list); > > > > > > spin_unlock(&c->erase_completion_lock); > > > > > > mutex_unlock(&c->erase_free_sem); > > > > > > - /* Ensure that kupdated calls us again to mark them clean */ > > > > > > - jffs2_erase_pending_trigger(c); > > > > > > } > > > > > > > > > > > > static void jffs2_erase_failed(struct jffs2_sb_info *c, struct > > > > > jffs2_eraseblock *jeb, uint32_t bad_offset) > > > > > > @@ -492,7 +490,7 @@ filebad: > > > > > > > > > > > > refile: > > > > > > /* Stick it back on the list from whence it came and come back later */ > > > > > > - jffs2_erase_pending_trigger(c); > > > > > > + jffs2_garbage_collect_trigger(c); > > > > > > > > > > But then you make the code more confusing. Indeed, readability becomes > > > > > worse. > > > > > > > > > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up > > > > > the GC thread, just like 'jffs2_garbage_collect_trigger()'... > > > > > > > > Here we go then: > > > > > > > > From 96a4a9dc054f2dbd57e180e202f69c9645536e0d Mon Sep 17 00:00:00 2001 > > > > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > > Date: Tue, 16 Feb 2010 15:18:31 +0100 > > > > Subject: [PATCH] jffs2: Make jffs2_erase_pending_trigger() initiate GC. > > > > > > > > Since erasing is now in the GC thread, erases should trigger > > > > the GC task instead. > > > > wbuf.c still wants to flush its buffer via write_super so > > > > invent jffs2_dirty_trigger() and use that in wbuf. > > > > Remove surplus call to jffs2_erase_pending_trigger() in erase.c > > > > and remove jffs2_garbage_collect_trigger() from write_super as > > > > of now write_super() should only commit dirty data to disk. > > > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > > > --- > > > > fs/jffs2/erase.c | 2 -- > > > > fs/jffs2/os-linux.h | 9 +++++++-- > > > > fs/jffs2/super.c | 1 - > > > > fs/jffs2/wbuf.c | 2 +- > > > > 4 files changed, 8 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > > > index 1ca2559..5616658 100644 > > > > --- a/fs/jffs2/erase.c > > > > +++ b/fs/jffs2/erase.c > > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info > > > *c, struct jffs2_eraseblo > > > > list_move_tail(&jeb->list, &c->erase_complete_list); > > > > spin_unlock(&c->erase_completion_lock); > > > > mutex_unlock(&c->erase_free_sem); > > > > - /* Ensure that kupdated calls us again to mark them clean */ > > > > - jffs2_erase_pending_trigger(c); > > > > } > > > > > > Looks like BGT should be triggered from here in order to write the clean > > > marker. > > > > How so? JFFS2 is already running jffs2_erase_pending_blocks() and > > has just completed an erase, the next thing it will do is to mark it with a clean > > marker. To me it looks like belts and suspenders: kick it again just in casewe missed > > something. > > OK. I expect you will send the final version of your 2 patches, then > I'll put them to my l2 tree, right? OK, will do shortly. What is your l2 tree? I figured you would push this to the mtd tree. Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-17 7:55 ` Joakim Tjernlund @ 2010-02-17 8:07 ` Artem Bityutskiy 2010-02-17 8:19 ` Joakim Tjernlund 0 siblings, 1 reply; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-17 8:07 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Wed, 2010-02-17 at 08:55 +0100, Joakim Tjernlund wrote: > > OK. I expect you will send the final version of your 2 patches, then > > I'll put them to my l2 tree, right? > > OK, will do shortly. What is your l2 tree? It is just my own tree where I collect random mtd stuff, and then dwmw2 picks stuff from there to his tree. It is just some help I provide him. > I figured you would > push this to the mtd tree. No, I do not maintain mtd, and only David can push stuff there. My tree is only to help him. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ 2010-02-17 8:07 ` Artem Bityutskiy @ 2010-02-17 8:19 ` Joakim Tjernlund 0 siblings, 0 replies; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-17 8:19 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/17 09:07:46: > > On Wed, 2010-02-17 at 08:55 +0100, Joakim Tjernlund wrote: > > > OK. I expect you will send the final version of your 2 patches, then > > > I'll put them to my l2 tree, right? > > > > OK, will do shortly. What is your l2 tree? > > It is just my own tree where I collect random mtd stuff, and then dwmw2 > picks stuff from there to his tree. It is just some help I provide him. > > > I figured you would > > push this to the mtd tree. > > No, I do not maintain mtd, and only David can push stuff there. My tree > is only to help him. I see, seems David is busy these days as I have not seen any mails from him. David, are you following this? Anyhow, I just sent my two patches in its final form to the list. Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] jffs:2 Move erasing from write_super to GC. 2010-02-15 16:03 [PATCH 1/2] jffs:2 Move erasing from write_super to GC Joakim Tjernlund 2010-02-15 16:03 ` [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ Joakim Tjernlund @ 2010-02-16 8:57 ` Artem Bityutskiy 2010-02-16 9:18 ` Joakim Tjernlund 1 sibling, 1 reply; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-16 8:57 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > Erasing blocks is a form of GC and therefor it should live in the > GC task. By moving it there two problems will be solved: > 1) umounting will not hang until all pending blocks has > been erased. > 2) Erasing can be paused by sending a SIGSTOP to the GC thread which > allowes for time critical tasks work in peace. > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > --- > fs/jffs2/background.c | 1 + > fs/jffs2/erase.c | 5 +++++ > fs/jffs2/nodemgmt.c | 4 ++++ > fs/jffs2/super.c | 1 - > 4 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c > index 3ff50da..a8e0140 100644 > --- a/fs/jffs2/background.c > +++ b/fs/jffs2/background.c > @@ -146,6 +146,7 @@ static int jffs2_garbage_collect_thread(void *_c) > disallow_signal(SIGHUP); > > D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): pass\n")); > + jffs2_erase_pending_blocks(c, 0); > if (jffs2_garbage_collect_pass(c) == -ENOSPC) { > printk(KERN_NOTICE "No space for garbage collection. Aborting GC thread\n"); > goto die; > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > index b47679b..1ca2559 100644 > --- a/fs/jffs2/erase.c > +++ b/fs/jffs2/erase.c > @@ -114,6 +114,11 @@ void jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count) > while (!list_empty(&c->erase_complete_list) || > !list_empty(&c->erase_pending_list)) { > > + if (signal_pending(current)) { > + spin_unlock(&c->erase_completion_lock); > + mutex_unlock(&c->erase_free_sem); > + goto done; > + } I think this is not very good split of functionality. 'jffs2_erase_pending_blocks()' should be stupid, it should just erase as many EBs as it was asked to. It should have zero knowledge about signals. I think you should remove the signals checking from here and move that to the GC thread. Also, I think you should call it with count = 1, so that you would erase only one EB at one iteration. This way you will let the GC thread do other things with less latency as well. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] jffs:2 Move erasing from write_super to GC. 2010-02-16 8:57 ` [PATCH 1/2] jffs:2 Move erasing from write_super to GC Artem Bityutskiy @ 2010-02-16 9:18 ` Joakim Tjernlund 2010-02-16 9:45 ` Artem Bityutskiy 0 siblings, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-16 9:18 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 09:57:52: > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote: > > Erasing blocks is a form of GC and therefor it should live in the > > GC task. By moving it there two problems will be solved: > > 1) umounting will not hang until all pending blocks has > > been erased. > > 2) Erasing can be paused by sending a SIGSTOP to the GC thread which > > allowes for time critical tasks work in peace. > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > --- > > fs/jffs2/background.c | 1 + > > fs/jffs2/erase.c | 5 +++++ > > fs/jffs2/nodemgmt.c | 4 ++++ > > fs/jffs2/super.c | 1 - > > 4 files changed, 10 insertions(+), 1 deletions(-) > > > > diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c > > index 3ff50da..a8e0140 100644 > > --- a/fs/jffs2/background.c > > +++ b/fs/jffs2/background.c > > @@ -146,6 +146,7 @@ static int jffs2_garbage_collect_thread(void *_c) > > disallow_signal(SIGHUP); > > > > D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): pass\n")); > > + jffs2_erase_pending_blocks(c, 0); > > if (jffs2_garbage_collect_pass(c) == -ENOSPC) { > > printk(KERN_NOTICE "No space for garbage collection. Aborting GC thread\n"); > > goto die; > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > index b47679b..1ca2559 100644 > > --- a/fs/jffs2/erase.c > > +++ b/fs/jffs2/erase.c > > @@ -114,6 +114,11 @@ void jffs2_erase_pending_blocks(struct jffs2_sb_info > *c, int count) > > while (!list_empty(&c->erase_complete_list) || > > !list_empty(&c->erase_pending_list)) { > > > > + if (signal_pending(current)) { > > + spin_unlock(&c->erase_completion_lock); > > + mutex_unlock(&c->erase_free_sem); > > + goto done; > > + } > > I think this is not very good split of functionality. > 'jffs2_erase_pending_blocks()' should be stupid, it should just erase as > many EBs as it was asked to. It should have zero knowledge about > signals. hmm, you have a point there. > > I think you should remove the signals checking from here and move that > to the GC thread. > > Also, I think you should call it with count = 1, so that you would erase > only one EB at one iteration. This way you will let the GC thread do > other things with less latency as well. If we reduce the count to 1 one might just as well remove count from jffs2_erase_pending_blocks() and make it just erase one block on each invocation, not sure I like that as it will be more overhead when erasing many blocks. I figure erasing blocks is the most rewarding form of GC so there should be no need to reduce latency for other things, can you think of a reason? Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] jffs:2 Move erasing from write_super to GC. 2010-02-16 9:18 ` Joakim Tjernlund @ 2010-02-16 9:45 ` Artem Bityutskiy 2010-02-16 11:49 ` Joakim Tjernlund 0 siblings, 1 reply; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-16 9:45 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Tue, 2010-02-16 at 10:18 +0100, Joakim Tjernlund wrote: > > Also, I think you should call it with count = 1, so that you would erase > > only one EB at one iteration. This way you will let the GC thread do > > other things with less latency as well. > > If we reduce the count to 1 one might just as well remove count from > jffs2_erase_pending_blocks() and make it just erase one block on each invocation, > not sure I like that as it will be more overhead when erasing many blocks. > > I figure erasing blocks is the most rewarding form of GC so there should > be no need to reduce latency for other things, can you think of a reason? Hmm, yes, you are right. Please, discard my "erase only 1" suggestion. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] jffs:2 Move erasing from write_super to GC. 2010-02-16 9:45 ` Artem Bityutskiy @ 2010-02-16 11:49 ` Joakim Tjernlund 2010-02-16 11:59 ` Artem Bityutskiy 0 siblings, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2010-02-16 11:49 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/16 10:45:35: > > On Tue, 2010-02-16 at 10:18 +0100, Joakim Tjernlund wrote: > > > Also, I think you should call it with count = 1, so that you would erase > > > only one EB at one iteration. This way you will let the GC thread do > > > other things with less latency as well. > > > > If we reduce the count to 1 one might just as well remove count from > > jffs2_erase_pending_blocks() and make it just erase one block on each invocation, > > not sure I like that as it will be more overhead when erasing many blocks. > > > > I figure erasing blocks is the most rewarding form of GC so there should > > be no need to reduce latency for other things, can you think of a reason? > > Hmm, yes, you are right. Please, discard my "erase only 1" suggestion. OK. Both you and Kai-Uwe wants back jffs2_erase_pending_trigger() so I will redo this part. Jocke BTW, I see UBIFS is using CRC16 as well as CRC32. Is there a reason you are using CRC16 in UBIFS. Any particular reason for that? CRC16 is much slower than crc32 in linux since the CRC32 impl. was optimized by yours truly long time ago just because it was impacting mount time for JFFS2. Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] jffs:2 Move erasing from write_super to GC. 2010-02-16 11:49 ` Joakim Tjernlund @ 2010-02-16 11:59 ` Artem Bityutskiy 0 siblings, 0 replies; 16+ messages in thread From: Artem Bityutskiy @ 2010-02-16 11:59 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Tue, 2010-02-16 at 12:49 +0100, Joakim Tjernlund wrote: > BTW, I see UBIFS is using CRC16 as well as CRC32. Is there a reason > you are using CRC16 in UBIFS. Any particular reason for that? CRC16 is > much slower than crc32 in linux since the CRC32 impl. was optimized > by yours truly long time ago just because it was impacting mount time > for JFFS2. We use crc16 only for LPT (Leb Properties Tree). The tree is very optimized to be as small as possible, and we protect very short pieces of data, this is why crc16 was choosed. Everything else is protected with crc32. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-02-17 8:23 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-15 16:03 [PATCH 1/2] jffs:2 Move erasing from write_super to GC Joakim Tjernlund 2010-02-15 16:03 ` [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/ Joakim Tjernlund 2010-02-16 8:59 ` Artem Bityutskiy 2010-02-16 9:32 ` Joakim Tjernlund 2010-02-16 14:27 ` Joakim Tjernlund 2010-02-17 7:18 ` Artem Bityutskiy 2010-02-17 7:35 ` Joakim Tjernlund 2010-02-17 7:48 ` Artem Bityutskiy 2010-02-17 7:55 ` Joakim Tjernlund 2010-02-17 8:07 ` Artem Bityutskiy 2010-02-17 8:19 ` Joakim Tjernlund 2010-02-16 8:57 ` [PATCH 1/2] jffs:2 Move erasing from write_super to GC Artem Bityutskiy 2010-02-16 9:18 ` Joakim Tjernlund 2010-02-16 9:45 ` Artem Bityutskiy 2010-02-16 11:49 ` Joakim Tjernlund 2010-02-16 11:59 ` Artem Bityutskiy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox