LKML Archive on
help / color / mirror / Atom feed
From: Andreas Dilger <>
Cc: "Theodore Ts'o" <>,
	linux-ext4 <>,
	linux-kernel <>
Subject: Re: [PATCH 2/2] e2fsprogs: Add support for toggling, verifying, and fixing inode checksums
Date: Fri, 8 Apr 2011 17:13:13 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 2011-04-08, at 1:25 PM, Darrick J. Wong wrote:
> On Fri, Apr 08, 2011 at 03:14:04AM -0600, Andreas Dilger wrote:
>> Do you have an e2fsck testcase for this code, to show that it detects/fixes
>> inodes with data corruption, and to fix the checksums after the ROCOMPAT flag
>> is set the first time?
> Not yet; I suspected that some clarification of exactly that issue was needed.
> It looks to me that in general the checksum will be zero for the "flag is
> enabled but no checksum has yet been provided" case, and nonzero in the "inode
> is corrupt" case.  So if e2fsck sees zero it'd first ask to correct the
> checksum, and if it sees nonzero it'll first ask to clear the inode.  If the
> user answers no to the first question, e2fsck can then propose the second
> option.

Seems reasonable, though it is possible that the inode checksums can also become invalid due to changing the filesystem UUID.  This should probably be handled by tune2fs when the UUID is changed, with an extra prompt if INODE_CSUM is enabled to indicate that the conversion may take a long time.

Looking at the checksum algorithm you used, the inode checksum does not change if the inode is relocated due to resize (i.e. it uses the inode number and not the underlying block number).  This is convenient, and does not impact the correctness in any way - if the wrong block is read/written then the inode number used in the checksum will not match either.

>> With the "ibadness" patch in our tree, the bad checksum should be a
>> significant factor in marking the inode as garbage, but possibly not enough
>> to have it thrown out if there are no other errors in the inode.
> Or e2fsck could use that heuristic; which tree is the ibadness patch in?
> Google shows a patch from 2008, but no recent discussion.

There is a relatively up-to-date version at;a=blob_plain;f=patches/e2fsprogs-ibadness-counter.patch;hb=8dd11ed9bdf0914d57d78d0c387bd21f747c1d29

> Something along the lines of: if the inode is not very bad, ask first to fix
> the checksum and second to clear the inode; if the inode seems bad, ask first
> to clear it and second to fix the checksum.

Yes, that is what I was thinking.  The real question is why the checksum would be bad in the case of no other "badness"?  If it is due to the UUID, that should be handled when the UUID is changed, and if it is due to a misplaced write (i.e. bad inode number) then it will help us to distinguish between the "real" inode and the misplaced "bad" inode.

>>> @@ -890,6 +890,11 @@ static struct e2fsck_problem problem_table[] = {
>>> 	     "(size %Is, lblk %r)\n"),
>>> +	/* Fast symlink has EXTENTS_FL set */
>>> +	  N_("inode %i checksum invalid.  "),
>> The comment for each problem should exactly mirror the text that is printed.
>> In this case, you haven't used the abbreviations "@i" and "@n", which would
>> normally make it much harder to search for this error string in the code, but
>> also simplifies the translation of the message.
> Oops, comment blooper that was a thinko on my part.  What would the @n be for?

@i is "inode", @n is "invalid", per e2fsck/message.c.

Cheers, Andreas

  reply	other threads:[~2011-04-08 23:13 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-06 22:44 [PATCH 0/2] Add inode checksum support to ext4 Darrick J. Wong
2011-04-06 22:45 ` [PATCH 1/2] ext4: Calculate and verify inode checksums Darrick J. Wong
2011-04-07  0:52   ` Sunil Mushran
2011-04-07 16:40     ` Darrick J. Wong
2011-04-07 17:10       ` Sunil Mushran
2011-04-08 18:50         ` Joel Becker
2011-04-08 19:30           ` Darrick J. Wong
2011-04-08  8:58   ` Andreas Dilger
2011-04-08 19:12     ` Darrick J. Wong
2011-04-08 22:49       ` Andreas Dilger
2011-04-06 22:47 ` [PATCH 2/2] e2fsprogs: Add support for toggling, verifying, and fixing " Darrick J. Wong
2011-04-08  9:14   ` Andreas Dilger
2011-04-08 19:25     ` Darrick J. Wong
2011-04-08 23:13       ` Andreas Dilger [this message]
2011-04-12  2:05         ` Darrick J. Wong
2011-04-08 19:27 ` [PATCH 0/2] Add inode checksum support to ext4 Mingming Cao
2011-04-08 20:17   ` Joel Becker
2011-07-27  8:27   ` Darrick J. Wong
2011-07-27  9:16     ` Andreas Dilger
2011-07-28 16:56       ` Darrick J. Wong
     [not found]         ` <>
2011-07-28 18:57           ` Darrick J. Wong
2011-07-29  9:55             ` Andreas Dilger
2011-07-28 22:07         ` Joel Becker
2011-07-29  9:48           ` Andreas Dilger
2011-07-29 13:19             ` Joel Becker
2011-07-30  7:25               ` Coly Li
2011-07-31  7:08                 ` Joel Becker
2011-07-31 23:52                   ` Coly Li
2011-08-01  4:57                     ` Joel Becker
2011-08-01  5:04                       ` Joel Becker
2011-08-01  7:16                         ` Coly Li
2011-04-20 17:40 ` Andi Kleen
2011-04-20 22:54   ` Darrick J. Wong
2011-04-21  0:25     ` Andreas Dilger

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:

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

  git send-email \ \ \ \ \ \ \
    --subject='Re: [PATCH 2/2] e2fsprogs: Add support for toggling, verifying, and fixing inode checksums' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).