On 2011-09-15, at 7:06 PM, Ted Ts'o wrote: > 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. Sure, but if the structure is read-locked it shouldn't be modified... > 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. True, but at the same time is there a reason _not_ to put the checksum at the end? For the superblock in particular it seems easy to do and simplifies the code either way. >> 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. Looking at Darrick's performance tests for the checksums, at 32 bytes the performance is 75%/95% (crc32-sby8-le/crc32-intel) of peak, and 91%/97% of peak for 128 bytes, so the overhead of computing the checksum in two parts is probably not significant. Cheers, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
