mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andreas Dilger <aedilger@gmail.com>
To: "djwong@us.ibm.com" <djwong@us.ibm.com>
Cc: Mingming Cao <cmm@us.ibm.com>, "Theodore Ts'o" <tytso@mit.edu>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/2] Add inode checksum support to ext4
Date: Wed, 27 Jul 2011 03:16:12 -0600	[thread overview]
Message-ID: <EF1F2330-D8C4-427A-ABB9-D1E607D91A3D@gmail.com> (raw)
In-Reply-To: <20110727082730.GG20655@tux1.beaverton.ibm.com>

On 2011-07-27, at 2:27 AM, "Darrick J. Wong" <djwong@us.ibm.com> wrote:
> On Fri, Apr 08, 2011 at 12:27:48PM -0700, Mingming Cao wrote:
>> Beyond checksumming the inode itself, it
>> would be more useful to checksum the extent indexing blocks, as the ext3
>> corruption actually happen at the indirect block.  
>> 
>> The idea is to reduce the eh_max (the max # of extents stored in this
>> block) to save some space to store the checksums in the block, 
>> 
>> /*
>> * Each block (leaves and indexes), even inode-stored has header.
>> */
>> struct ext4_extent_header {
>>        __le16  eh_magic;       /* probably will support different
>> formats */
>>        __le16  eh_entries;     /* number of valid entries */
>>        __le16  eh_max;         /* capacity of store in entries */
>>        __le16  eh_depth;       /* has tree real underlying blocks? */
>>        __le32  eh_generation;  /* generation of the tree */
> 
> Does anyone use eh_generation?  Linux 3.0 shows no users and it didn't look like
> the snapshot patches do either.  If nobody intends to start using this field,
> (part of) it could become eh_checksum

We use eh_generation in Lustre to detect if the extent tree is being changed while it is unlocked.

In the past, the discussion about adding checksums to the index and extent blocks was about using an ext4_extent_tail that contained not only the checksum of the block, but also a back-pointer to the inode/generation of the inode using this block.

That would allow e2fsck to verify that it is using the correct index/extent blocks and not pointing to a stale block that belonged to some other inode.

Since the header and index/extent entries are always 3 *__u32 in size, the extent tail can always be 4 * __u32 in size yet only consume a single slot in the block. There of course is no reason to put an extent tail inside the inode itself.

Cheers, Andreas

  reply	other threads:[~2011-07-27  9:15 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-06 22:44 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
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 [this message]
2011-07-28 16:56       ` Darrick J. Wong
     [not found]         ` <CAOQ4uxiOpwX2-Nfh9wJ7wSmAnbj9bh1+d9C95-N5D-8saRr6ww@mail.gmail.com>
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:
  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=EF1F2330-D8C4-427A-ABB9-D1E607D91A3D@gmail.com \
    --to=aedilger@gmail.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=cmm@us.ibm.com \
    --cc=djwong@us.ibm.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®