On 2011-09-15, at 5:11 PM, Ted Ts'o wrote:
> On Thu, Sep 15, 2011 at 05:09:13PM -0600, Andreas Dilger wrote:
>>
>> I thought it would be better to move s_checksum to be the last field in the
>> superblock to avoid multiple calls to the CRC function?
>
> Did you see my comment about just zero'ing the checksum field before
> running the CRC? We're going to have to do that for other data
> structures, such as the inode structure, and it's what we do with the
> block group descriptor checksum.
That isn't correct. The group descriptor checksum is computed in chunks:
__le16 ext4_group_desc_csum(struct ext4_sb_info *sbi, __u32 block_group,
struct ext4_group_desc *gdp)
{
int offset = offsetof(struct ext4_group_desc, bg_checksum);
__le32 le_group = cpu_to_le32(block_group);
crc = crc16(~0, sbi->s_es->s_uuid, sizeof(sbi->s_es->s_uuid));
crc = crc16(crc, (__u8 *)&le_group, sizeof(le_group));
crc = crc16(crc, (__u8 *)gdp, offset);
offset += sizeof(gdp->bg_checksum); /* skip checksum */
****HERE****
/* for checksum of struct ext4_group_desc do the rest...*/
if ((sbi->s_es->s_feature_incompat &
cpu_to_le32(EXT4_FEATURE_INCOMPAT_64BIT)) &&
offset < le16_to_cpu(sbi->s_es->s_desc_size))
crc = crc16(crc, (__u8 *)gdp + offset,
le16_to_cpu(sbi->s_es->s_desc_size) -
offset);
}
Darrick and I discussed zeroing the checksum fields, but then there is a
race with other threads accessing the same structure.
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?
Since we naturally have to break the checksum calculation for 128-byte
inodes and 32-byte descriptors, due to old versions of those structs,
there is little overhead in just skipping the field, and no races.
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
