From patchwork Sun Jan 16 17:48:24 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Artem Bityutskiy X-Patchwork-Id: 79092 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from canuck.infradead.org (canuck.infradead.org [134.117.69.58]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E82F0B6EED for ; Mon, 17 Jan 2011 04:51:44 +1100 (EST) Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1PeWiR-0000Dt-Dw; Sun, 16 Jan 2011 17:48:35 +0000 Received: from mail-fx0-f49.google.com ([209.85.161.49]) by canuck.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1PeWiN-0000DY-MB for linux-mtd@lists.infradead.org; Sun, 16 Jan 2011 17:48:32 +0000 Received: by fxm19 with SMTP id 19so5004008fxm.36 for ; Sun, 16 Jan 2011 09:48:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:subject:from:reply-to:to:cc:in-reply-to :references:content-type:date:message-id:mime-version:x-mailer :content-transfer-encoding; bh=FSfbT67QapZfMOvn7HjOx+9NhYEHrKO8wUXP+3l1vIg=; b=CkE3TqnTWIv6S7Zms1cXipf7POr7jnSb6G2fsi7Ps6WOpv/V+4kRqCey4+SuY3UkyQ wPqP2j6CnFEIrjqwfXIG/LCbHfoZ1LMa3q8ZAlFoBV7KRLtNnp+L+goGL6Uc9T2CJPsS rl8Aby8UFJIXpEja52aZjCMaCjq3byk2sDQik= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:reply-to:to:cc:in-reply-to:references:content-type :date:message-id:mime-version:x-mailer:content-transfer-encoding; b=K0CAzklpHSROLbq880n1LiOvTA0BhTwFSQIJRLbuRrlsSklh5mV0zX/QdDKDa+VxbC ldV9XsDdoQ5EW6fV0a23gUJwNdlL7ESr0urMpU++bAdFqP2qk8DvsEQ6AMwNsJHlbc15 3tka1zz9EgPO7wrwYn85rp9XeiJ9e59FA5dEc= Received: by 10.223.86.130 with SMTP id s2mr3641071fal.66.1295200109901; Sun, 16 Jan 2011 09:48:29 -0800 (PST) Received: from [192.168.255.4] (a91-152-69-107.elisa-laajakaista.fi [91.152.69.107]) by mx.google.com with ESMTPS id n1sm1298881fam.16.2011.01.16.09.48.25 (version=SSLv3 cipher=RC4-MD5); Sun, 16 Jan 2011 09:48:26 -0800 (PST) Subject: Re: ubifs: sync() causes writes even if nothing is changed From: Artem Bityutskiy To: "Hans J. Koch" , "Adrian.Hunter" In-Reply-To: <20101013163005.GB1889@silverbox.local> References: <20101013163005.GB1889@silverbox.local> Date: Sun, 16 Jan 2011 19:48:24 +0200 Message-ID: <1295200104.2470.5.camel@koala> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 (2.32.1-1.fc14) X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110116_124831_935025_4293DA23 X-CRM114-Status: GOOD ( 23.82 ) X-Spam-Score: 1.4 (+) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (1.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.161.49 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is freemail (dedekind1[at]gmail.com) 2.2 FREEMAIL_ENVFROM_END_DIGIT Envelope-from freemail username ends in digit (dedekind1[at]gmail.com) -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: Sebastian Andrzej Siewior , linux-mtd@lists.infradead.org, Adrian Hunter X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: dedekind1@gmail.com 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 Wed, 2010-10-13 at 18:30 +0200, Hans J. Koch wrote: > Running this command: > > # while true ; do sync; sleep 1; done > > causes two eraseblocks being erased every second, although there > are no writes to the ubifs filesystem. I hacked some printks into > my NAND driver that print page_address and column for each erase. > With that, I get this output every second: > > ... > [ 63.701765] erase p=0x0000ae40 c=0xffffffff > [ 63.706534] erase p=0xffffffff c=0xffffffff > [ 63.725492] erase p=0x0000ae80 c=0xffffffff > [ 63.730260] erase p=0xffffffff c=0xffffffff > ... > > From a quick glance at the ubifs code, this might come out of the > garbage collector that is triggered on every sync() and writes > something even if nothing has changed. With nandsim I only can see one erase, but this is anyway suboptimal. The below patch should fix the issue, please, test if you can. I've also pushed it to ubifs-2.6.git. From dca0fe61489805e0eb4ada7c6922856ca91eae52 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 16 Jan 2011 19:22:02 +0200 Subject: [PATCH] UBIFS: do not start the commit if there is nothing to commit 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 are not dirty znodes (hence, nothing to commit). Reported-by: Hans J. Koch Signed-off-by: Artem Bityutskiy --- fs/ubifs/commit.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-) diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c index 02429d8..a963d96 100644 --- a/fs/ubifs/commit.c +++ b/fs/ubifs/commit.c @@ -70,6 +70,21 @@ static int do_commit(struct ubifs_info *c) goto out_up; } + /* + * Every file-system change changes the TNC, and makes the root znode + * dirty. So if the root znode is clean we can just return immediately + * because there must be nothing to commit. Note, se do not have to + * lock @c->tnc_mutex because we have @c->commit_sem in write mode, + * which guarantees that no one else can access TNC functions + * concurrently. + */ + if (!c->zroot.znode || !test_bit(DIRTY_ZNODE, &c->zroot.znode->flags)) { + ubifs_assert(atomic_long_read(&c->dirty_zn_cnt) == 0); + err = 0; + up_write(&c->commit_sem); + 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 +177,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: