On 2011-02-09, at 06:01, Lukas Czerner wrote:
>> @@ -1200,6 +1200,9 @@ struct ext4_sb_info {
>> struct ext4_li_request *s_li_request;
>> /* Wait multiplier for lazy initialization thread */
>> unsigned int s_li_wait_mult;
>> +
>> + /* record the last minlen when FITRIM is called. */
>> + u64 trim_minlen;
>
> Maybe this is a bit nitpicking, but it is not very clear what is the
> "trim_minlen" for. I would rather use something like
> "last_trim_minblks", or similar.
And for good measure, this should use the "s_" prefix to indicate it is in the superblock.
>> +#define EXT4_GROUP_INFO_NEED_INIT_BIT 0
>> +#define EXT4_GROUP_INFO_HAS_BEEN_TRIMMED 1
It would also be good if the names of these fields were consistent, like
+#define EXT4_GROUP_INFO_NEED_INIT_BIT 0
+#define EXT4_GROUP_INFO_WAS_TRIMMED_BIT 1
>> @@ -1563,6 +1563,9 @@ static int mb_mark_used(struct ext4_buddy *e4b, struct ext4_free_extent *ex)
>> mb_set_bits(EXT4_MB_BITMAP(e4b), ex->fe_start, len0);
>> mb_check_buddy(e4b);
>>
>> + if (!ret)
>> + EXT4_SB(sb)->trim_minlen = minlen;
>> +
>
> Did you even tried to compile it ? "minlen" is not defined in
> mb_mark_used(), also it does make sense to set trim_minlen there. Did
> not you meant it to be in ext4_trim_fs ?
I was wondering exactly the same thing, but I haven't had time to look at the code.
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
