All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Fasheh <mfasheh@suse.de>
To: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Gang He <ghe@suse.com>,
	rgoldwyn@suse.de, linux-kernel@vger.kernel.org,
	ocfs2-devel@oss.oracle.com
Subject: Re: [Ocfs2-devel] [PATCH v2 4/4] ocfs2: check/fix inode block for online file check
Date: Tue, 24 Nov 2015 14:16:04 -0800	[thread overview]
Message-ID: <20151124221604.GX15575@wotan.suse.de> (raw)
In-Reply-To: <56385E63.80808@oracle.com>

Hi Junxiao,

On Tue, Nov 03, 2015 at 03:12:35PM +0800, Junxiao Bi wrote:
> Hi Gang,
> 
> This is not like a right patch.
> First, online file check only checks inode's block number, valid flag,
> fs generation value, and meta ecc. I never see a real corruption
> happened only on this field, if these fields are corrupted, that means
> something bad may happen on other place. So fix this field may not help
> and even cause corruption more hard.

I agree that these are rather uncommon, we might even consider removing the
VALID_FL fixup. I definitely don't think we're ready for anything more
complicated than this though either. We kind of have to start somewhere too.


> Second, the repair way is wrong. In
> ocfs2_filecheck_repair_inode_block(), if these fields in disk don't
> match the ones in memory, the ones in memory are used to update the disk
> fields. The question is how do you know these field in memory are
> right(they may be the real corrupted ones)?

Your second point (and the last part of your 1st point) makes a good
argument for why this shouldn't happen automatically. Some of these
corruptions might require a human to look at the log and decide what to do.
Especially as you point out, where we might not know where the source of the
corruption is. And if the human can't figure it out, then it's probably time
to unmount and fsck.

Thanks,
	--Mark

--
Mark Fasheh

WARNING: multiple messages have this Message-ID (diff)
From: Mark Fasheh <mfasheh@suse.de>
To: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Gang He <ghe@suse.com>,
	rgoldwyn@suse.de, linux-kernel@vger.kernel.org,
	ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [PATCH v2 4/4] ocfs2: check/fix inode block for online file check
Date: Tue, 24 Nov 2015 14:16:04 -0800	[thread overview]
Message-ID: <20151124221604.GX15575@wotan.suse.de> (raw)
In-Reply-To: <56385E63.80808@oracle.com>

Hi Junxiao,

On Tue, Nov 03, 2015 at 03:12:35PM +0800, Junxiao Bi wrote:
> Hi Gang,
> 
> This is not like a right patch.
> First, online file check only checks inode's block number, valid flag,
> fs generation value, and meta ecc. I never see a real corruption
> happened only on this field, if these fields are corrupted, that means
> something bad may happen on other place. So fix this field may not help
> and even cause corruption more hard.

I agree that these are rather uncommon, we might even consider removing the
VALID_FL fixup. I definitely don't think we're ready for anything more
complicated than this though either. We kind of have to start somewhere too.


> Second, the repair way is wrong. In
> ocfs2_filecheck_repair_inode_block(), if these fields in disk don't
> match the ones in memory, the ones in memory are used to update the disk
> fields. The question is how do you know these field in memory are
> right(they may be the real corrupted ones)?

Your second point (and the last part of your 1st point) makes a good
argument for why this shouldn't happen automatically. Some of these
corruptions might require a human to look at the log and decide what to do.
Especially as you point out, where we might not know where the source of the
corruption is. And if the human can't figure it out, then it's probably time
to unmount and fsck.

Thanks,
	--Mark

--
Mark Fasheh

  parent reply	other threads:[~2015-11-24 22:16 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-28  6:25 [PATCH v2 0/4] Add online file check feature Gang He
2015-10-28  6:25 ` [Ocfs2-devel] " Gang He
2015-10-28  6:25 ` [PATCH v2 1/4] ocfs2: export ocfs2_kset for online file check Gang He
2015-10-28  6:25   ` [Ocfs2-devel] " Gang He
2015-11-24 21:47   ` Mark Fasheh
2015-11-24 21:47     ` [Ocfs2-devel] " Mark Fasheh
2015-10-28  6:25 ` [PATCH v2 2/4] ocfs2: sysfile interfaces " Gang He
2015-10-28  6:25   ` [Ocfs2-devel] " Gang He
2015-11-03  7:20   ` Junxiao Bi
2015-11-03  7:20     ` [Ocfs2-devel] " Junxiao Bi
2015-11-03  7:54     ` Gang He
2015-11-03  7:54       ` [Ocfs2-devel] " Gang He
2015-11-03  8:20       ` Junxiao Bi
2015-11-03  8:20         ` [Ocfs2-devel] " Junxiao Bi
2015-11-03  8:30         ` Gang He
2015-11-03  8:30           ` [Ocfs2-devel] " Gang He
2015-11-24 21:46         ` Mark Fasheh
2015-11-24 21:46           ` [Ocfs2-devel] " Mark Fasheh
2015-11-24 21:55           ` Srinivas Eeda
2015-11-24 21:55             ` Srinivas Eeda
2015-11-25  3:29           ` Gang He
2015-11-25  3:29             ` [Ocfs2-devel] " Gang He
2015-11-25  4:43             ` Junxiao Bi
2015-11-25  4:43               ` [Ocfs2-devel] " Junxiao Bi
2015-11-25  5:11               ` Gang He
2015-11-25  5:11                 ` [Ocfs2-devel] " Gang He
2015-12-18 22:37             ` Mark Fasheh
2015-12-18 22:37               ` [Ocfs2-devel] " Mark Fasheh
2015-11-25  4:33           ` Junxiao Bi
2015-11-25  4:33             ` [Ocfs2-devel] " Junxiao Bi
2015-11-24 21:52   ` Mark Fasheh
2015-11-24 21:52     ` Mark Fasheh
2015-10-28  6:26 ` [PATCH v2 3/4] ocfs2: create/remove sysfile " Gang He
2015-10-28  6:26   ` [Ocfs2-devel] " Gang He
2015-11-24 21:53   ` Mark Fasheh
2015-11-24 21:53     ` [Ocfs2-devel] " Mark Fasheh
2015-10-28  6:26 ` [PATCH v2 4/4] ocfs2: check/fix inode block " Gang He
2015-10-28  6:26   ` [Ocfs2-devel] " Gang He
2015-11-03  7:12   ` Junxiao Bi
2015-11-03  7:12     ` [Ocfs2-devel] " Junxiao Bi
2015-11-03  8:15     ` Gang He
2015-11-03  8:15       ` [Ocfs2-devel] " Gang He
2015-11-03  8:29       ` Junxiao Bi
2015-11-03  8:29         ` [Ocfs2-devel] " Junxiao Bi
2015-11-03  8:47         ` Gang He
2015-11-03  8:47           ` [Ocfs2-devel] " Gang He
2015-11-03  9:01           ` Junxiao Bi
2015-11-03  9:01             ` [Ocfs2-devel] " Junxiao Bi
2015-11-03  9:25             ` Gang He
2015-11-03  9:25               ` [Ocfs2-devel] " Gang He
2015-11-24 22:16     ` Mark Fasheh [this message]
2015-11-24 22:16       ` Mark Fasheh
2015-11-25  4:11       ` Junxiao Bi
2015-11-25  4:11         ` Junxiao Bi
2015-11-25  5:04         ` Gang He
2015-11-25  5:04           ` Gang He
2015-11-25  5:44           ` Junxiao Bi
2015-11-25  5:44             ` Junxiao Bi
2015-10-28 16:34 ` [Ocfs2-devel] [PATCH v2 0/4] Add online file check feature Srinivas Eeda
2015-10-28 16:34   ` Srinivas Eeda
2015-10-29  4:44   ` Gang He
2015-10-29  4:44     ` Gang He
2015-10-29  7:46     ` Srinivas Eeda
2015-10-29  7:46       ` Srinivas Eeda
2015-10-29  8:26       ` Gang He
2015-10-29  8:26         ` Gang He
2015-12-02 18:20 ` Pavel Machek
2015-12-02 18:20   ` [Ocfs2-devel] " Pavel Machek
2015-12-03  2:05   ` Gang He
2015-12-03  2:05     ` [Ocfs2-devel] " Gang He
2015-12-03  5:17     ` Greg KH
2015-12-03  5:17       ` [Ocfs2-devel] " Greg KH
2015-12-04  8:36       ` Gang He
2015-12-04  8:36         ` [Ocfs2-devel] " Gang He
2015-12-04  9:20         ` Pavel Machek
2015-12-04  9:20           ` [Ocfs2-devel] " Pavel Machek
2015-12-04 16:40         ` Greg KH
2015-12-04 16:40           ` [Ocfs2-devel] " Greg KH
2015-12-07  3:33           ` Gang He
2015-12-07  3:33             ` [Ocfs2-devel] " Gang He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151124221604.GX15575@wotan.suse.de \
    --to=mfasheh@suse.de \
    --cc=ghe@suse.com \
    --cc=junxiao.bi@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=rgoldwyn@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.