| Message ID | 1295335817.2470.31.camel@koala |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2011-01-18 at 09:30 +0200, Artem Bityutskiy wrote: > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > > This patch fixes suboptimal UBIFS 'sync_fs()' implementation which causes > flash I/O even if the file-system is synchronized. E.g., a 'printk()' > in the MTD erasure function (e.g., 'nand_erase_nand()') can show that > for every 'sync' shell command UBIFS erases at least one eraseblock. > > So '$ while true; do sync; done' will cause huge amount of flash I/O. > > The reason for this is that UBIFS commits in 'sync_fs()', and starts the > commit even if there is nothing to commit, e.g., it anyway changes the > log. This patch adds a check in the 'do_commit()' UBIFS functions which > prevents the commit if there is nothing to commit. > > Reported-by: Hans J. Koch <hjk@linutronix.de> > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> Adrian, I've also pushed these 3 patches to ubifs-2.6.git / master. Could you please validate them?
Hi,
With these patches, there is no longer an erase for _every_
sync. However, there is still one erase that occurs when ubifs is
mounted. The following script will cause an erase with each sync.
while true; do
mount -t ubifs ubi0_2 /tmp/mnt
sync
umount /tmp/mnt
done
John Ogness
On Tue, 2011-01-18 at 13:29 +0100, John Ogness wrote: > Hi, > > With these patches, there is no longer an erase for _every_ > sync. However, there is still one erase that occurs when ubifs is > mounted. The following script will cause an erase with each sync. > > while true; do > mount -t ubifs ubi0_2 /tmp/mnt > sync > umount /tmp/mnt > done Hi, thanks, I need to look why there are still erases after mount. Probably this is actually part of mounting, not part of the first sync. But otherwise it sounds like you are satisfied, right? But I also have to do power cut tests emulation before really pushing this forward.
On 2011-01-21, Artem Bityutskiy wrote: > I need to look why there are still erases after mount. Probably > this is actually part of mounting, not part of the first sync. But > otherwise it sounds like you are satisfied, right? Yes, it is definately a big improvement. Thanks! > But I also have to do power cut tests emulation before really > pushing this forward. OK. BTW: The return value description for the function nothing_to_commit() is backwards. It says that it returns 1 if there is something to commit, but actually it returns 1 if there is nothing to commit. John Ogness
On Fri, 2011-01-21 at 12:28 +0100, John Ogness wrote: > On 2011-01-21, Artem Bityutskiy wrote: > > I need to look why there are still erases after mount. Probably > > this is actually part of mounting, not part of the first sync. But > > otherwise it sounds like you are satisfied, right? > > Yes, it is definately a big improvement. Thanks! > > > But I also have to do power cut tests emulation before really > > pushing this forward. The power cut simulation test is running. > BTW: The return value description for the function nothing_to_commit() > is backwards. It says that it returns 1 if there is something to > commit, but actually it returns 1 if there is nothing to commit. Amended, thanks. Also added: Tested-by: John Ogness <john.ogness@linutronix.de> Thanks.
diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c index 02429d8..fdd5112 100644 --- a/fs/ubifs/commit.c +++ b/fs/ubifs/commit.c @@ -48,6 +48,52 @@ #include <linux/slab.h> #include "ubifs.h" +/* + * nothing_to_commit - check if there is nothing to commit. + * @c: UBIFS file-system description object + * + * This is a helper function which checks if there is anything to commit. It is + * used as an optimization to avoid starting the commit if it is not really + * necessary. Indeed, the commit operation always assumes flash I/O (e.g., + * writing the commit start node to the log), and it is better to avoid doing + * this unnecessarily. E.g., 'ubifs_sync_fs()' runs the commit, but if there is + * nothing to commit, it is more optimal to avoid any flash I/O. + * + * This function returns %1 if there is something to commit and %0 otherwise. + */ +static int nothing_to_commit(struct ubifs_info *c) +{ + /* + * During mounting or remounting from R/O mode to R/W mode we may + * commit for various recovery-related reasons. + */ + if (c->mounting || c->remounting_rw) + return 0; + + /* + * If the root TNC node is dirty, we definitely have something to + * commit. + */ + if (c->zroot.znode && test_bit(DIRTY_ZNODE, &c->zroot.znode->flags)) + return 0; + + /* + * Even though the TNC is clean, the LPT tree may have dirty nodes. For + * example, this may happen if the budgeting subsystem invoked GC to + * make some free space, and the GC found an LEB with only dirty and + * free space. In this case GC would just change the lprops of this + * LEB (by turning all space into free space) and unmap it. + */ + if (c->nroot && test_bit(DIRTY_CNODE, &c->nroot->flags)) + return 0; + + ubifs_assert(atomic_long_read(&c->dirty_zn_cnt) == 0); + ubifs_assert(c->dirty_pn_cnt == 0); + ubifs_assert(c->dirty_nn_cnt == 0); + + return 1; +} + /** * do_commit - commit the journal. * @c: UBIFS file-system description object @@ -70,6 +116,12 @@ static int do_commit(struct ubifs_info *c) goto out_up; } + if (nothing_to_commit(c)) { + up_write(&c->commit_sem); + err = 0; + goto out_cancel; + } + /* Sync all write buffers (necessary for recovery) */ for (i = 0; i < c->jhead_cnt; i++) { err = ubifs_wbuf_sync(&c->jheads[i].wbuf); @@ -162,12 +214,12 @@ static int do_commit(struct ubifs_info *c) if (err) goto out; +out_cancel: spin_lock(&c->cs_lock); c->cmt_state = COMMIT_RESTING; wake_up(&c->cmt_wq); dbg_cmt("commit end"); spin_unlock(&c->cs_lock); - return 0; out_up: