* [PATCH 0/2] Move erasing to the GC thread. @ 2010-02-17 8:16 Joakim Tjernlund 2010-02-17 8:16 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Joakim Tjernlund 0 siblings, 1 reply; 7+ messages in thread From: Joakim Tjernlund @ 2010-02-17 8:16 UTC (permalink / raw) To: linux-mtd; +Cc: Joakim Tjernlund Current JFFS2 erases blocks from write_super() which is problematic as a reboot while erasing will make the umount hang until all blocks has been erased. These patches address this problem by moving the erasing to the GC task instead, leaving write_super() to only handle dirty data. Joakim Tjernlund (2): jffs2: Move erasing from write_super to GC. jffs2: Make jffs2_erase_pending_trigger() initiate GC. fs/jffs2/background.c | 1 + fs/jffs2/erase.c | 7 +++++-- fs/jffs2/nodemgmt.c | 4 ++++ fs/jffs2/os-linux.h | 9 +++++++-- fs/jffs2/super.c | 2 -- fs/jffs2/wbuf.c | 2 +- 6 files changed, 18 insertions(+), 7 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] jffs2: Move erasing from write_super to GC. 2010-02-17 8:16 [PATCH 0/2] Move erasing to the GC thread Joakim Tjernlund @ 2010-02-17 8:16 ` Joakim Tjernlund 2010-02-17 8:16 ` [PATCH 2/2] jffs2: Make jffs2_erase_pending_trigger() initiate GC Joakim Tjernlund 2010-02-18 11:06 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Artem Bityutskiy 0 siblings, 2 replies; 7+ messages in thread From: Joakim Tjernlund @ 2010-02-17 8:16 UTC (permalink / raw) To: linux-mtd; +Cc: Joakim Tjernlund Erasing blocks is a form of GC and therefore it should live in the GC task. By moving it there two problems will be solved: 1) unmounting 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 to 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] 7+ messages in thread
* [PATCH 2/2] jffs2: Make jffs2_erase_pending_trigger() initiate GC. 2010-02-17 8:16 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Joakim Tjernlund @ 2010-02-17 8:16 ` Joakim Tjernlund 2010-02-18 11:06 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Artem Bityutskiy 1 sibling, 0 replies; 7+ messages in thread From: Joakim Tjernlund @ 2010-02-17 8:16 UTC (permalink / raw) To: linux-mtd; +Cc: Joakim Tjernlund 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] 7+ messages in thread
* Re: [PATCH 1/2] jffs2: Move erasing from write_super to GC. 2010-02-17 8:16 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Joakim Tjernlund 2010-02-17 8:16 ` [PATCH 2/2] jffs2: Make jffs2_erase_pending_trigger() initiate GC Joakim Tjernlund @ 2010-02-18 11:06 ` Artem Bityutskiy 2010-02-18 12:17 ` Joakim Tjernlund 1 sibling, 1 reply; 7+ messages in thread From: Artem Bityutskiy @ 2010-02-18 11:06 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Wed, 2010-02-17 at 09:16 +0100, Joakim Tjernlund wrote: > 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; > + } Err, I thought you would remove signal checking from this function? It really does not belong here. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] jffs2: Move erasing from write_super to GC. 2010-02-18 11:06 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Artem Bityutskiy @ 2010-02-18 12:17 ` Joakim Tjernlund 2010-02-18 12:22 ` Artem Bityutskiy 0 siblings, 1 reply; 7+ messages in thread From: Joakim Tjernlund @ 2010-02-18 12:17 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/18 12:06:06: > > On Wed, 2010-02-17 at 09:16 +0100, Joakim Tjernlund wrote: > > 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; > > + } > > Err, I thought you would remove signal checking from this function? It > really does not belong here. Slight misunderstanding, it needs to be there otherwise my patches won't do any good. Removing signal from this function basically means that we should remove count and only erase one block per invocation. Basically the while !empty loop needs to move into the GC task instead. This is a somewhat bigger surgery but doable. Jocke ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] jffs2: Move erasing from write_super to GC. 2010-02-18 12:17 ` Joakim Tjernlund @ 2010-02-18 12:22 ` Artem Bityutskiy 2010-02-18 13:12 ` Joakim Tjernlund 0 siblings, 1 reply; 7+ messages in thread From: Artem Bityutskiy @ 2010-02-18 12:22 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: linux-mtd On Thu, 2010-02-18 at 13:17 +0100, Joakim Tjernlund wrote: > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/18 12:06:06: > > > > On Wed, 2010-02-17 at 09:16 +0100, Joakim Tjernlund wrote: > > > 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; > > > + } > > > > Err, I thought you would remove signal checking from this function? It > > really does not belong here. > > Slight misunderstanding, it needs to be there otherwise my patches won't do any good. > Removing signal from this function basically means that we should remove count > and only erase one block per invocation. Basically the while !empty loop needs to move > into the GC task instead. This is a somewhat bigger surgery but doable. Right. You can move the signal check to the BGT, and do 1-3 erases at a time by passing cnt = 1-3. I think it is cleaner. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] jffs2: Move erasing from write_super to GC. 2010-02-18 12:22 ` Artem Bityutskiy @ 2010-02-18 13:12 ` Joakim Tjernlund 0 siblings, 0 replies; 7+ messages in thread From: Joakim Tjernlund @ 2010-02-18 13:12 UTC (permalink / raw) To: dedekind1, David Woodhouse; +Cc: linux-mtd Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/18 13:22:44: > > On Thu, 2010-02-18 at 13:17 +0100, Joakim Tjernlund wrote: > > Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/02/18 12:06:06: > > > > > > On Wed, 2010-02-17 at 09:16 +0100, Joakim Tjernlund wrote: > > > > 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; > > > > + } > > > > > > Err, I thought you would remove signal checking from this function? It > > > really does not belong here. > > > > Slight misunderstanding, it needs to be there otherwise my patches won't do any good. > > Removing signal from this function basically means that we should remove count > > and only erase one block per invocation. Basically the while !empty loop needs to move > > into the GC task instead. This is a somewhat bigger surgery but doable. > > Right. You can move the signal check to the BGT, and do 1-3 erases at a > time by passing cnt = 1-3. I think it is cleaner. Doing this warrants a return value from jffs2_erase_pending_blocks() as testing for empty lists will be clumsy in both GC and erase.c All in all this will bring some more churn in the code so I really don't won't to do all that for nothing, that is, David are you happy with this too? Question, is it really necessary to take erase_free_sem lock before testing for list_empty? Jocke ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-02-18 13:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-17 8:16 [PATCH 0/2] Move erasing to the GC thread Joakim Tjernlund 2010-02-17 8:16 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Joakim Tjernlund 2010-02-17 8:16 ` [PATCH 2/2] jffs2: Make jffs2_erase_pending_trigger() initiate GC Joakim Tjernlund 2010-02-18 11:06 ` [PATCH 1/2] jffs2: Move erasing from write_super to GC Artem Bityutskiy 2010-02-18 12:17 ` Joakim Tjernlund 2010-02-18 12:22 ` Artem Bityutskiy 2010-02-18 13:12 ` Joakim Tjernlund
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox