linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Dilger <adilger.kernel@dilger.ca>
To: djwong@us.ibm.com
Cc: Amir Goldstein <amir73il@gmail.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Andreas Dilger <aedilger@gmail.com>,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	"Theodore Ts'o" <tytso@mit.edu>, Mingming Cao <cmm@us.ibm.com>
Subject: Re: [PATCH 0/2] Add inode checksum support to ext4
Date: Fri, 29 Jul 2011 03:55:30 -0600	[thread overview]
Message-ID: <4B7BD8E2-A1F3-4788-9753-20DC78620C5D@dilger.ca> (raw)
In-Reply-To: <20110728185730.GJ20655@tux1.beaverton.ibm.com>

On 2011-07-28, at 12:57 PM, Darrick J. Wong wrote:
>>> On Wed, Jul 27, 2011 at 03:16:12AM -0600, Andreas Dilger wrote:
>>>> 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.
>>> 
>>> Does anybody have any objection to using crc32c (which we can hardware
>>> accelerate on new Intel boxen) over crc16?  I think it'll be pretty easy
>>> to use some of the reserved space in the group descriptor to store
>>> checksums of the block and inode bitmaps.
>> 
>> On LSF Ted told me i can use 32bit from the group descriptor for exclude
>> bitmap block and that inode and block bitmap checksum would use 16bit each .
> 
> I know;  as far as I know there's still 96 bits of free space in the group
> descriptor, which could be used to crc32 all three bitmaps.  But then that
> leaves the descriptors with no room for further expansion, unless we decide to
> expand them sort of like what was done for inodes.  Do we have a strategy for
> handling continued expansion of metadata objects in ext4?

One reason to stick to crc16 for the block checksums is that this would still
fit inside the ext3_group_desc space, so upgraded filesystems could enable
checksums.

>>> Adding tails to the extent tree blocks seems a bit trickier than that,
>>> but not a big deal, though I guess I'll have to reshuffle
>>> the extent tree to free up space at the end of the block.

I don't think reshuffling is necessary.  It should be OK to add it to blocks
that fit, and skip blocks that don't have enough space.

>>> I was also wondering what people think of adding checksums to directory
>> files?
>>> I think that it's possible to put a checksum in each directory block --
>> for
>>> blocks containing a linear array of actual directory entries, we could
>> zero out
>>> the space past the end of the array and put a checksum at the very end of
>> the
>>> block.  For the dx_node/dx_root blocks, we could probably use the space
>>> occupied by the last dx_entry to store the checksum.  Obviously, we'd have
>> to
>>> move whatever's at the end of the block elsewhere, but then, we have to do
>> that
>>> for the extent tree too.  Basically, the last 4 bytes become the checksum
>> after
>>> whatever's occupying the space is relocated. :)
>>> 
>>> It looks like there's sufficient unused space in ext4_xattr_header to add
>> a
>>> checksum.
>>> 
>>> Also -- should I create separate rocompat feature flags for each metadata
>>> object that I add checksums to?  Or just have one flag that covers them
>> all?
>>> 
>>> Ok, enough crazy ideas for now...
>>> 
>>> --D
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


  reply	other threads:[~2011-07-29  9:55 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
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]         ` <CAOQ4uxiOpwX2-Nfh9wJ7wSmAnbj9bh1+d9C95-N5D-8saRr6ww@mail.gmail.com>
2011-07-28 18:57           ` Darrick J. Wong
2011-07-29  9:55             ` Andreas Dilger [this message]
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=4B7BD8E2-A1F3-4788-9753-20DC78620C5D@dilger.ca \
    --to=adilger.kernel@dilger.ca \
    --cc=aedilger@gmail.com \
    --cc=amir73il@gmail.com \
    --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;
as well as URLs for NNTP newsgroup(s).