From patchwork Wed Feb 24 16:01:27 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Monakhov X-Patchwork-Id: 46136 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3C4C0B7CEF for ; Thu, 25 Feb 2010 03:01:37 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757132Ab0BXQBe (ORCPT ); Wed, 24 Feb 2010 11:01:34 -0500 Received: from mail-bw0-f209.google.com ([209.85.218.209]:49570 "EHLO mail-bw0-f209.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756703Ab0BXQBc (ORCPT ); Wed, 24 Feb 2010 11:01:32 -0500 Received: by bwz1 with SMTP id 1so2034462bwz.21 for ; Wed, 24 Feb 2010 08:01:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:from:to:cc:subject :references:date:in-reply-to:message-id:user-agent:mime-version :content-type; bh=K2L9WpgMVwXhJDuz4l5WKCdFMTS0zSauYg++2u2jXdQ=; b=xBWAhKkQtLmQDoyWns3wb+5G+I47UkHIVNm19tnasut/blHBK2F5vF6H4npAh+sr6k 3af/MfuX5BxBrfTTAW/83k2WettmJGNO6rJOKd3CwrYADMjybuQdflD663mccoTovKbQ EtYyatOLYYJ47kMj5ET8j92IrZ0/cJSLGqvfg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-type; b=vrVuEGO5AEstd583VWyDLsAxKjJ0IBmeQB1TZ0tjUN6S5HHb5UYWDBWU7HyxqFz4y3 boKU7un6FTUx/NoK1p6LDg+H+GTdHJgHgtCoRbadMyJrPMYHrCe7jf6Ase99vi+JnwCt J9wFZzw9FFddkqqVIWiv1sKn8N7YX6QbgXu+4= Received: by 10.204.6.203 with SMTP id a11mr7986bka.33.1267027290957; Wed, 24 Feb 2010 08:01:30 -0800 (PST) Received: from dmon-lp (swsoft-msk-nat.sw.ru [195.214.232.10]) by mx.google.com with ESMTPS id 13sm2762440bwz.11.2010.02.24.08.01.28 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 24 Feb 2010 08:01:29 -0800 (PST) From: Dmitry Monakhov To: Jan Kara Cc: Camille Moncelier , "linux-fsdevel\@vger.kernel.org" , ext4 development Subject: Re: [ext3] Changes to block device after an ext3 mount point has been remounted readonly References: <9F53CAF8-B6B4-40EB-89FA-CD6779D17DBE@sun.com> <20100222223252.GA13882@atrey.karlin.mff.cuni.cz> <20100222230552.GB13882@atrey.karlin.mff.cuni.cz> <16F918FB-F45D-478E-9358-550BB39E277E@sun.com> <20100223135531.GA7699@atrey.karlin.mff.cuni.cz> Date: Wed, 24 Feb 2010 19:01:27 +0300 In-Reply-To: <20100223135531.GA7699@atrey.karlin.mff.cuni.cz> (Jan Kara's message of "Tue, 23 Feb 2010 16:55:31 +0300") Message-ID: <877hq2tyg8.fsf@openvz.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) MIME-Version: 1.0 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Jan Kara writes: >> The fact is that I've been able to reproduce the problem on LVM block >> devices, and sd* block devices so it's definitely not a loop device >> specific problem. >> >> By the way, I tried several other things other than "echo s >> >/proc/sysrq_trigger" I tried multiple sync followed with a one minute >> "sleep", >> >> "echo 3 >/proc/sys/vm/drop_caches" seems to lower the chances of "hash >> changes" but doesn't stops them. > Strange. When I use sync(1) in your script and use /dev/sda5 instead of a > /dev/loop0, I cannot reproduce the problem (was running the script for > something like an hour). Theoretically some pages may exist after rw=>ro remount because of generic race between write/sync, And they will be written in by writepage if page already has buffers. This not happen in ext4 because. Each time it try to perform writepages it try to start_journal and this result in EROFS. The race bug will be closed some day but new one may appear again. Let's be honest and change ext3 writepage like follows: - check ROFS flag inside write page - dump writepage's errors. From a7cadf8017626cd80fcd8ea5a0e4deff4f63e02e Mon Sep 17 00:00:00 2001 From: Dmitry Monakhov Date: Wed, 24 Feb 2010 18:17:58 +0300 Subject: [PATCH] ext3: add sanity checks to writeback There is theoretical possibility to perform writepage on RO superblock. Add explicit check for what case. In fact writepage may fail by a number of reasons. This is really rare case but sill may result in data loss. At least we have to dump a error message. Signed-off-by: Dmitry Monakhov --- fs/ext3/inode.c | 40 +++++++++++++++++++++++++++++++++++----- 1 files changed, 35 insertions(+), 5 deletions(-) diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c index 455e6e6..cf0e3aa 100644 --- a/fs/ext3/inode.c +++ b/fs/ext3/inode.c @@ -1536,6 +1536,11 @@ static int ext3_ordered_writepage(struct page *page, if (ext3_journal_current_handle()) goto out_fail; + if (inode->i_sb->s_flags & MS_RDONLY) { + err = -EROFS; + goto out_fail; + } + if (!page_has_buffers(page)) { create_empty_buffers(page, inode->i_sb->s_blocksize, (1 << BH_Dirty)|(1 << BH_Uptodate)); @@ -1546,7 +1551,8 @@ static int ext3_ordered_writepage(struct page *page, NULL, buffer_unmapped)) { /* Provide NULL get_block() to catch bugs if buffers * weren't really mapped */ - return block_write_full_page(page, NULL, wbc); + ret = block_write_full_page(page, NULL, wbc); + goto out; } } handle = ext3_journal_start(inode, ext3_writepage_trans_blocks(inode)); @@ -1584,12 +1590,17 @@ static int ext3_ordered_writepage(struct page *page, err = ext3_journal_stop(handle); if (!ret) ret = err; +out: + if (ret) + ext3_msg(inode->i_sb, KERN_CRIT, "%s: failed " + "%ld pages, ino %lu; err %d\n", __func__, + wbc->nr_to_write, inode->i_ino, ret); return ret; out_fail: redirty_page_for_writepage(wbc, page); unlock_page(page); - return ret; + goto out; } static int ext3_writeback_writepage(struct page *page, @@ -1603,12 +1614,18 @@ static int ext3_writeback_writepage(struct page *page, if (ext3_journal_current_handle()) goto out_fail; + if (inode->i_sb->s_flags & MS_RDONLY) { + err = -EROFS; + goto out_fail; + } + if (page_has_buffers(page)) { if (!walk_page_buffers(NULL, page_buffers(page), 0, PAGE_CACHE_SIZE, NULL, buffer_unmapped)) { /* Provide NULL get_block() to catch bugs if buffers * weren't really mapped */ - return block_write_full_page(page, NULL, wbc); + ret = block_write_full_page(page, NULL, wbc); + goto out; } } @@ -1626,12 +1643,17 @@ static int ext3_writeback_writepage(struct page *page, err = ext3_journal_stop(handle); if (!ret) ret = err; +out: + if (ret) + ext3_msg(inode->i_sb, KERN_CRIT, "%s: failed " + "%ld pages, ino %lu; err %d\n", __func__, + wbc->nr_to_write, inode->i_ino, ret); return ret; out_fail: redirty_page_for_writepage(wbc, page); unlock_page(page); - return ret; + goto out; } static int ext3_journalled_writepage(struct page *page, @@ -1645,6 +1667,11 @@ static int ext3_journalled_writepage(struct page *page, if (ext3_journal_current_handle()) goto no_write; + if (inode->i_sb->s_flags & MS_RDONLY) { + err = -EROFS; + goto no_write; + } + handle = ext3_journal_start(inode, ext3_writepage_trans_blocks(inode)); if (IS_ERR(handle)) { ret = PTR_ERR(handle); @@ -1684,8 +1711,11 @@ static int ext3_journalled_writepage(struct page *page, if (!ret) ret = err; out: + if (ret) + ext3_msg(inode->i_sb, KERN_CRIT, "%s: failed " + "%ld pages, ino %lu; err %d\n", __func__, + wbc->nr_to_write, inode->i_ino, ret); return ret; - no_write: redirty_page_for_writepage(wbc, page); out_unlock: