On 2011-09-15, at 5:41 PM, Ted Ts'o wrote: > On Thu, Sep 15, 2011 at 05:34:41PM -0600, Andreas Dilger wrote: >> >> Darrick and I discussed zeroing the checksum fields, but then there is a >> race with other threads accessing the same structure. > > What race are you worried about? The moment you modify some part of > the data structure, the checksum is going to be wrong. This is true > whether you zero out the checksum field before you do the calculations > or not. 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. >> If we went to a crc32c LSB for filesystems with RO_COMPAT_CSUM it would >> be possible to change how it is computed. Since we have freedom to move >> the checksum field now, why have the added complexity to do zeroing of >> the field or two chunks? > > Why is zero'ing out the field complex? It's a single line of code.... > It's certainly easier than doing it in two chunks, and there will be > some data structures (the block group descriptors at the very least) > where zero'ing the checksum is definitely going to be the easier way > to go. 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. 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
