On Thu, Sep 22, 2011 at 05:12:32AM +0300, Amir Goldstein wrote: > On Wed, Sep 21, 2011 at 11:40 PM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > > On Wed, Sep 21, 2011 at 11:03:57AM +0300, Amir Goldstein wrote: > >> On Tue, Sep 20, 2011 at 9:56 PM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > >> > On Thu, Sep 15, 2011 at 06:50:51PM -0400, Theodore Ts'o wrote: > > <snip> > >> >> diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h > >> >> index 4fec5db..1c86cb2 100644 > >> >> --- a/lib/ext2fs/ext2_fs.h > >> >> +++ b/lib/ext2fs/ext2_fs.h > >> >> @@ -142,7 +142,9 @@ struct ext2_group_desc > >> >> __u16 bg_free_inodes_count; /* Free inodes count */ > >> >> __u16 bg_used_dirs_count; /* Directories count */ > >> >> __u16 bg_flags; > >> >> - __u32 bg_reserved[2]; > >> >> + __u32 bg_exclude_bitmap_lo; /* Exclude bitmap for snapshots */ > >> >> + __u16 bg_block_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */ > >> >> + __u16 bg_inode_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */ > >> >> __u16 bg_itable_unused; /* Unused inodes count */ > >> >> __u16 bg_checksum; /* crc16(s_uuid+grouo_num+group_desc)*/ > >> >> }; > >> >> @@ -159,7 +161,9 @@ struct ext4_group_desc > >> >> __u16 bg_free_inodes_count; /* Free inodes count */ > >> >> __u16 bg_used_dirs_count; /* Directories count */ > >> >> __u16 bg_flags; /* EXT4_BG_flags (INODE_UNINIT, etc) */ > >> >> - __u32 bg_reserved[2]; /* Likely block/inode bitmap checksum */ > >> >> + __u32 bg_exclude_bitmap_lo; /* Exclude bitmap for snapshots */ > >> >> + __u16 bg_block_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */ > >> >> + __u16 bg_inode_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */ > >> >> __u16 bg_itable_unused; /* Unused inodes count */ > >> >> __u16 bg_checksum; /* crc16(sb_uuid+group+desc) */ > >> >> __u32 bg_block_bitmap_hi; /* Blocks bitmap block MSB */ > >> >> @@ -169,7 +173,10 @@ struct ext4_group_desc > >> >> __u16 bg_free_inodes_count_hi;/* Free inodes count MSB */ > >> >> __u16 bg_used_dirs_count_hi; /* Directories count MSB */ > >> >> __u16 bg_itable_unused_hi; /* Unused inodes count MSB */ > >> >> - __u32 bg_reserved2[3]; > >> >> + __u32 bg_exclude_bitmap_hi; /* Exclude bitmap block MSB */ > >> >> + __u16 bg_block_bitmap_csum_hi;/* crc32c(s_uuid+grp_num+bitmap) MSB */ > >> >> + __u16 bg_inode_bitmap_csum_hi;/* crc32c(s_uuid+grp_num+bitmap) MSB */ > >> >> + __u32 bg_reserved; > >> > > >> > Should we attach a checksum to the exclude bitmap? That would, unfortunately, > >> > put the 64-byte block group pretty close to full, unless we're ok with making > >> > the bg even bigger... > >> > >> Good point. > >> My initial approach (which I probably expressed somewhere) was that when > >> exclude_bitmap feature is set, the block bitmap checksum should cover both > >> block bitmap and exclude bitmap. > >> The reason being that exclude bitmap adds little entropy over block bitmap: > >> - it should always be a sub set of block bitmap bits > >> - clearing exclude bit is done together with clearing the block used bit > >> - the only case of modifying exclude bitmap only is when a used block becomes > >> excluded (a.k.a moved to snapshot) > >> > >> so how do you feel about compressing 2 blocks into 16bits of checksum :-/? > > > > Hrm.... is it generally the case that both block and exclude bitmaps are loaded > > at the same time? I don't think it'd be difficult to checksum both blocks > > unless it's common that one of the two are not in memory. > > > > Hrm indeed. no. exclude bitmap is currently not loaded always together with > block bitmap, so unless this is changed, we'll have to think of something else. Or... does it make sense to preload the exclude bitmap at the same time as the block bitmap? If I'm reading you correctly, (block_bitmap | exclude_bitmap) yields a bitmap of all blocks that are not available, correct? So in the case that exclude bitmaps are enabled, you'd need both to be in memory anyway. > However that brings me to wonder: > block/inode/exclude bitmap seldom change more than a handful of bits/words > at a time. > Does it make sense to diff update the CRC of the bitmaps instead of > recomputing it? Yes, that would be a good (later) optimization at least to consider. --D -- 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
