Re: [PATCH 1/2] libext2fs: add metadata checksum and snapshot feature flags

[Ted Ts'o <tytso@mit.edu>]

On Thu, Sep 15, 2011 at 06:09:04PM -0600, Andreas Dilger wrote:
> 
> If you are reading the structure (not modifying it) and trying to verify
> the checksum, it would be necessary to read-lock the structure, zero
> the field, compute the checksum, reset the field, unlock, and then
> compare checksums.  Alternately, one would have to make a copy of the
> struct to zero out the field and compute the checksum on the copy.  Both
> are more complex than just doing the checksum on two separate chunks.

You have to read-lock the structure before calculating the checksum in
any case, since otherwise when someone else is modifying the
structure, before they have a chance to update the checksum, you'll
calculate the checksum and discover that it is incorrect.

In practice we would probably be calculating the checksum when the
inode if first read into memory, before it it is visible to the rest
of the system, so this shouldn't be an issue.  But if it is visible to
the rest of the system, even you put the checksum at the end, if
someone else can modify the data structure while you are calculating
it, the checksum will be wrong.

> No, because for group descriptors, the size is conditional on whether
> RO_COMPAT_GDT_CSUM is set, so it is just as easy to always compute the
> first 32-byte crc32 (excluding the bg_checksum field) and then only
> conditionally compute the second 32-byte checksum.  The same is true
> of the inode checksum and s_inode_size, if the checksum is at the last
> field of struct ext2_inode.

But wouldn't it be faster to zero out the two fields, and then
caluclate a single 64-byte checksum?  That way we avoid the setup
costs of the crc32, especially in the case of the crc32c-sby8-[lb]e
implementation.

						- Ted