From patchwork Tue May 18 18:59:12 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 52914 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id BB36CB6EEB for ; Wed, 19 May 2010 05:01:25 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1OES0d-0007AS-VP; Tue, 18 May 2010 18:59:20 +0000 Received: from casper.infradead.org ([2001:770:15f::2]) by bombadil.infradead.org with esmtps (Exim 4.69 #1 (Red Hat Linux)) id 1OES0b-00079k-1I for linux-mtd@bombadil.infradead.org; Tue, 18 May 2010 18:59:17 +0000 Received: from macbook.infradead.org ([2001:8b0:10b:1:216:eaff:fe05:bbb8]) by casper.infradead.org with esmtpsa (Exim 4.69 #1 (Red Hat Linux)) id 1OES0X-0002Mv-W9; Tue, 18 May 2010 18:59:15 +0000 Subject: Re: [PATCH] jffs2: Move erasing from write_super to GC. From: David Woodhouse To: Joakim Tjernlund In-Reply-To: <1274207844.6930.7754.camel@macbook.infradead.org> References: <1269079399-27087-1-git-send-email-Joakim.Tjernlund@transmode.se> <1273771018.12840.7077.camel@macbook.infradead.org> <1273833304.9999.1994.camel@macbook.infradead.org> <1274207844.6930.7754.camel@macbook.infradead.org> Date: Tue, 18 May 2010 19:59:12 +0100 Message-ID: <1274209152.6930.7802.camel@macbook.infradead.org> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 (2.28.3-1.fc12) X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org See http://www.infradead.org/rpr.html Cc: linux-mtd@lists.infradead.org X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Tue, 2010-05-18 at 19:37 +0100, David Woodhouse wrote: > > Hrm, and now everything which calls jffs2_erase_pending_trigger() > needs > _not_ to be holding c->erase_completion_lock, or it'll deadlock... Playing with this incremental patch now... I also moved the erasing into jffs2_garbage_collect_pass() itself rather than having both callers of jffs2_garbage_collect_pass() do it for themselves, under different circumstances. I think the one in jffs2_reserve_space() was buggy anyway. diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c index 6cc014c..8c9ffdc 100644 --- a/fs/jffs2/background.c +++ b/fs/jffs2/background.c @@ -21,12 +21,11 @@ static int jffs2_garbage_collect_thread(void *); -void jffs2_garbage_collect_trigger(struct jffs2_sb_info *c) +void jffs2_erase_pending_trigger(struct jffs2_sb_info *c) { - spin_lock(&c->erase_completion_lock); + BUG_ON(spin_trylock(&c->erase_completion_lock)); if (c->gc_task && jffs2_thread_should_wake(c)) send_sig(SIGHUP, c->gc_task, 1); - spin_unlock(&c->erase_completion_lock); } /* This must only ever be called when no GC thread is currently running */ @@ -146,9 +145,8 @@ static int jffs2_garbage_collect_thread(void *_c) disallow_signal(SIGHUP); D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): pass\n")); - if (jffs2_erase_pending_blocks(c, 1)) - /* Nothing more to do ATM */; - else if (jffs2_garbage_collect_pass(c) == -ENOSPC) { + + 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 2a6cc6c..e8638f8 100644 --- a/fs/jffs2/erase.c +++ b/fs/jffs2/erase.c @@ -168,8 +168,11 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblo mutex_lock(&c->erase_free_sem); spin_lock(&c->erase_completion_lock); list_move_tail(&jeb->list, &c->erase_complete_list); + /* Wake the GC thread to mark them clean */ + jffs2_erase_pending_trigger(c); spin_unlock(&c->erase_completion_lock); mutex_unlock(&c->erase_free_sem); + wake_up(&c->erase_wait); } static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb, uint32_t bad_offset) @@ -488,9 +491,9 @@ filebad: refile: /* Stick it back on the list from whence it came and come back later */ - jffs2_erase_pending_trigger(c); mutex_lock(&c->erase_free_sem); spin_lock(&c->erase_completion_lock); + jffs2_erase_pending_trigger(c); list_move(&jeb->list, &c->erase_complete_list); spin_unlock(&c->erase_completion_lock); mutex_unlock(&c->erase_free_sem); diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c index 3b6f2fa..1ea4a84 100644 --- a/fs/jffs2/gc.c +++ b/fs/jffs2/gc.c @@ -214,6 +214,19 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c) return ret; } + /* If there are any blocks which need erasing, erase them now */ + if (!list_empty(&c->erase_complete_list) || + !list_empty(&c->erase_pending_list)) { + spin_unlock(&c->erase_completion_lock); + D1(printk(KERN_DEBUG "jffs2_garbage_collect_pass() erasing pending blocks\n")); + if (jffs2_erase_pending_blocks(c, 1)) { + mutex_unlock(&c->alloc_sem); + return 0; + } + D1(printk(KERN_DEBUG "No progress from erasing blocks; doing GC anyway\n")); + spin_lock(&c->erase_completion_lock); + } + /* First, work out which block we're garbage-collecting */ jeb = c->gcblock; @@ -222,7 +235,7 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c) if (!jeb) { /* Couldn't find a free block. But maybe we can just erase one and make 'progress'? */ - if (!list_empty(&c->erase_pending_list)) { + if (c->nr_erasing_blocks) { spin_unlock(&c->erase_completion_lock); mutex_unlock(&c->alloc_sem); return -EAGAIN; diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c index 4779301..8ec436a 100644 --- a/fs/jffs2/nodemgmt.c +++ b/fs/jffs2/nodemgmt.c @@ -115,9 +115,21 @@ int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize, spin_unlock(&c->erase_completion_lock); ret = jffs2_garbage_collect_pass(c); - - if (ret == -EAGAIN) - jffs2_erase_pending_blocks(c, 1); + if (ret == -EAGAIN) { + spin_lock(&c->erase_completion_lock); + if (c->nr_erasing_blocks && + list_empty(&c->erase_pending_list) && + list_empty(&c->erase_complete_list)) { + DECLARE_WAITQUEUE(wait, current); + set_current_state(TASK_UNINTERRUPTIBLE); + add_wait_queue(&c->erase_wait, &wait); + D1(printk(KERN_DEBUG "%s waiting for erase to complete\n", __func__)); + spin_unlock(&c->erase_completion_lock); + + schedule(); + } else + spin_unlock(&c->erase_completion_lock); + } else if (ret) return ret; @@ -469,7 +481,9 @@ struct jffs2_raw_node_ref *jffs2_add_physical_node_ref(struct jffs2_sb_info *c, void jffs2_complete_reservation(struct jffs2_sb_info *c) { D1(printk(KERN_DEBUG "jffs2_complete_reservation()\n")); - jffs2_garbage_collect_trigger(c); + spin_lock(&c->erase_completion_lock); + jffs2_erase_pending_trigger(c); + spin_unlock(&c->erase_completion_lock); mutex_unlock(&c->alloc_sem); } diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h index 5d26362..64ac455 100644 --- a/fs/jffs2/os-linux.h +++ b/fs/jffs2/os-linux.h @@ -148,13 +148,7 @@ static inline void jffs2_dirty_trigger(struct jffs2_sb_info *c) /* background.c */ 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); -} +void jffs2_erase_pending_trigger(struct jffs2_sb_info *c); /* dir.c */ extern const struct file_operations jffs2_dir_operations; diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c index 696686c..1312552 100644 --- a/fs/jffs2/scan.c +++ b/fs/jffs2/scan.c @@ -260,7 +260,9 @@ int jffs2_scan_medium(struct jffs2_sb_info *c) ret = -EIO; goto out; } + spin_lock(&c->erase_completion_lock); jffs2_erase_pending_trigger(c); + spin_unlock(&c->erase_completion_lock); } ret = 0; out: