From: "Gang He" <ghe@suse.com> To: "Junxiao Bi" <junxiao.bi@oracle.com>, "Mark Fasheh" <MFasheh@suse.com>, <rgoldwyn@suse.de> Cc: <akpm@linux-foundation.org>, <ocfs2-devel@oss.oracle.com>, <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 4/4] ocfs2: check/fix inode block for online file check Date: Tue, 03 Nov 2015 01:15:13 -0700 [thread overview] Message-ID: <5638DD91020000F90001CC78@relay2.provo.novell.com> (raw) In-Reply-To: <56385E63.80808@oracle.com> Hello Junxiao, See my comments inline. >>> > 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. This online file check/fix feature is used to check/fix some light file meta block corruption, instead of turning a file system off and using fsck.ocfs2. e.g. meta ecc error, we really need not to use fsck.ocfs2. of course, this feature does not replace fsck.ocfs2 and touch some complicated meta block problems, if there is some potential problem in some areas, we can discuss them one by one. > 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)? Here, if the inode block was corrupted, the file system is not able to load it into the memory. ocfs2_filecheck_repair_inode_block() will able to load it into the memory, since it try to fix these light-level problem before loading. if the fix is OK, the changed meta-block can pass the block-validate function and load into the memory as a inode object. Since the file system is under a cluster environment, we have to use some existing function and code path to keep these block operation under a cluster lock. Thanks Gang > > Thanks, > Junxiao. > On 10/28/2015 02:26 PM, Gang He wrote: >> +static int ocfs2_filecheck_repair_inode_block(struct super_block *sb, >> + struct buffer_head *bh) >> +{ >> + int rc; >> + int changed = 0; >> + struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data; >> + >> + rc = ocfs2_filecheck_validate_inode_block(sb, bh); >> + /* Can't fix invalid inode block */ >> + if (!rc || rc == -OCFS2_FILECHECK_ERR_INVALIDINO) >> + return rc; >> + >> + trace_ocfs2_filecheck_repair_inode_block( >> + (unsigned long long)bh->b_blocknr); >> + >> + if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) || >> + ocfs2_is_soft_readonly(OCFS2_SB(sb))) { >> + mlog(ML_ERROR, >> + "Filecheck: try to repair dinode #%llu on readonly filesystem\n", >> + (unsigned long long)bh->b_blocknr); >> + return -OCFS2_FILECHECK_ERR_READONLY; >> + } >> + >> + if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) { >> + di->i_blkno = cpu_to_le64(bh->b_blocknr); >> + changed = 1; >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: i_blkno to %llu\n", >> + (unsigned long long)bh->b_blocknr, >> + (unsigned long long)le64_to_cpu(di->i_blkno)); >> + } >> + >> + if (!(di->i_flags & cpu_to_le32(OCFS2_VALID_FL))) { >> + di->i_flags |= cpu_to_le32(OCFS2_VALID_FL); >> + changed = 1; >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: OCFS2_VALID_FL is set\n", >> + (unsigned long long)bh->b_blocknr); >> + } >> + >> + if (le32_to_cpu(di->i_fs_generation) != >> + OCFS2_SB(sb)->fs_generation) { >> + di->i_fs_generation = cpu_to_le32(OCFS2_SB(sb)->fs_generation); >> + changed = 1; >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: fs_generation to %u\n", >> + (unsigned long long)bh->b_blocknr, >> + le32_to_cpu(di->i_fs_generation)); >> + } >> + >> + if (changed || >> + ocfs2_validate_meta_ecc(sb, bh->b_data, &di->i_check)) { >> + ocfs2_compute_meta_ecc(sb, bh->b_data, &di->i_check); >> + mark_buffer_dirty(bh); >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: compute meta ecc\n", >> + (unsigned long long)bh->b_blocknr); >> + } >> + >> + return 0; >> +}
WARNING: multiple messages have this Message-ID (diff)
From: Gang He <ghe@suse.com> To: Junxiao Bi <junxiao.bi@oracle.com>, Mark Fasheh <MFasheh@suse.com>, rgoldwyn@suse.de Cc: akpm@linux-foundation.org, ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org Subject: [Ocfs2-devel] [PATCH v2 4/4] ocfs2: check/fix inode block for online file check Date: Tue, 03 Nov 2015 01:15:13 -0700 [thread overview] Message-ID: <5638DD91020000F90001CC78@relay2.provo.novell.com> (raw) In-Reply-To: <56385E63.80808@oracle.com> Hello Junxiao, See my comments inline. >>> > 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. This online file check/fix feature is used to check/fix some light file meta block corruption, instead of turning a file system off and using fsck.ocfs2. e.g. meta ecc error, we really need not to use fsck.ocfs2. of course, this feature does not replace fsck.ocfs2 and touch some complicated meta block problems, if there is some potential problem in some areas, we can discuss them one by one. > 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)? Here, if the inode block was corrupted, the file system is not able to load it into the memory. ocfs2_filecheck_repair_inode_block() will able to load it into the memory, since it try to fix these light-level problem before loading. if the fix is OK, the changed meta-block can pass the block-validate function and load into the memory as a inode object. Since the file system is under a cluster environment, we have to use some existing function and code path to keep these block operation under a cluster lock. Thanks Gang > > Thanks, > Junxiao. > On 10/28/2015 02:26 PM, Gang He wrote: >> +static int ocfs2_filecheck_repair_inode_block(struct super_block *sb, >> + struct buffer_head *bh) >> +{ >> + int rc; >> + int changed = 0; >> + struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data; >> + >> + rc = ocfs2_filecheck_validate_inode_block(sb, bh); >> + /* Can't fix invalid inode block */ >> + if (!rc || rc == -OCFS2_FILECHECK_ERR_INVALIDINO) >> + return rc; >> + >> + trace_ocfs2_filecheck_repair_inode_block( >> + (unsigned long long)bh->b_blocknr); >> + >> + if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) || >> + ocfs2_is_soft_readonly(OCFS2_SB(sb))) { >> + mlog(ML_ERROR, >> + "Filecheck: try to repair dinode #%llu on readonly filesystem\n", >> + (unsigned long long)bh->b_blocknr); >> + return -OCFS2_FILECHECK_ERR_READONLY; >> + } >> + >> + if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) { >> + di->i_blkno = cpu_to_le64(bh->b_blocknr); >> + changed = 1; >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: i_blkno to %llu\n", >> + (unsigned long long)bh->b_blocknr, >> + (unsigned long long)le64_to_cpu(di->i_blkno)); >> + } >> + >> + if (!(di->i_flags & cpu_to_le32(OCFS2_VALID_FL))) { >> + di->i_flags |= cpu_to_le32(OCFS2_VALID_FL); >> + changed = 1; >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: OCFS2_VALID_FL is set\n", >> + (unsigned long long)bh->b_blocknr); >> + } >> + >> + if (le32_to_cpu(di->i_fs_generation) != >> + OCFS2_SB(sb)->fs_generation) { >> + di->i_fs_generation = cpu_to_le32(OCFS2_SB(sb)->fs_generation); >> + changed = 1; >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: fs_generation to %u\n", >> + (unsigned long long)bh->b_blocknr, >> + le32_to_cpu(di->i_fs_generation)); >> + } >> + >> + if (changed || >> + ocfs2_validate_meta_ecc(sb, bh->b_data, &di->i_check)) { >> + ocfs2_compute_meta_ecc(sb, bh->b_data, &di->i_check); >> + mark_buffer_dirty(bh); >> + mlog(ML_ERROR, >> + "Filecheck: reset dinode #%llu: compute meta ecc\n", >> + (unsigned long long)bh->b_blocknr); >> + } >> + >> + return 0; >> +}
next prev parent reply other threads:[~2015-11-03 8:15 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 [this message] 2015-11-03 8:15 ` 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 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=5638DD91020000F90001CC78@relay2.provo.novell.com \ --to=ghe@suse.com \ --cc=MFasheh@suse.com \ --cc=akpm@linux-foundation.org \ --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: linkBe 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.