On 2/29/12 1:22 AM, Lukas Czerner wrote:
> On Tue, 28 Feb 2012, Eric Sandeen wrote:
...
>>> + /*
>>> + * If the last inode is free, we can discard it as well.
>>> + */
>>> + if (inodes >= first_free)
>>> + e2fsck_discard_inodes(ctx, group, first_free,
>>> + inodes - first_free + 1);
>>
>> This is unrelated to the changelog
>>
>>> /*
>>> * If discard zeroes data and the group inode table
>>> * was not zeroed yet, set itable as zeroed
>>> */
>>> if ((ctx->options & E2F_OPT_DISCARD) &&
>>> - (io_channel_discard_zeroes_data(fs->io)) &&
>>> + !(ctx->options & E2F_OPT_NO) &&
>>
>> This is unrelated as well...
>
> So do you want me to separate those unrelated changes to a separate
> patch ? Those are pretty small changes..
They are, but they are completely separate fixes, and not described in the changelog.
Ideally I'd say they should be a separate testable commit, but at least it
should be mentioned in the changelog (and patch summary).
Sorry, compound commits with unrelated changes are my pet peeve; they make bisecting
harder, and make it harder to find fixes when looking back over git history.
-Eric
>>
>> Rather than continuing to check _OPT_NO here and in e2fsck_discard_blocks(), it
>> might be better (in a separate patch) ;) to simply turn off OPT_DISCARD right
>> after options processing if -n was specified, and not worry about it down here.
>
> Makes sense, I can change it.
>
> Thanks!
> -Lukas
>
>>
>>> + io_channel_discard_zeroes_data(fs->io) &&
>>> !(ext2fs_bg_flags_test(fs, group,
>>> - EXT2_BG_INODE_ZEROED))) {
>>> + EXT2_BG_INODE_ZEROED))) {
>>> ext2fs_bg_flags_set(fs, group,
>>> EXT2_BG_INODE_ZEROED);
>>> ext2fs_group_desc_csum_set(fs, group);
>>> }
>>>
>>> + first_free = fs->super->s_inodes_per_group + 1;
>>> + free_array[group] = group_free;
>>> + dir_array[group] = dirs_count;
>>> group ++;
>>> inodes = 0;
>>> skip_group = 0;
>>
>>
>
--
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
