Re: [PATCH] ext4: Change the handling of RAID stripe width

[Daniel Ehrenberg]

On Wed, Jul 6, 2011 at 2:18 PM, Eric Sandeen <sandeen@xxxxxxxxxx> wrote:
> On 7/6/11 3:47 PM, Dan Ehrenberg wrote:
>> Previously, the stripe width was blindly used for determining the size
>> of allocations. Now, the stripe width is used as a hint for the initial
>> mb_group_prealloc; if it is greater than 1, then we make sure that
>> mb_group_prealloc is some multiple of it, and otherwise it is ignored.
>> mb_group_prealloc is always usable to adjust the preallocation strategy,
>> not just when the stripe-width is 0 as before.
>>
>> Signed-off-by: Dan Ehrenberg <dehrenberg@xxxxxxxxxx>
>> ---
>>  fs/ext4/mballoc.c |   40 +++++++++++++++++++++++++++++-----------
>>  1 files changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 6ed859d..710c27f 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -127,13 +127,14 @@
>>   * based on file size. This can be found in ext4_mb_normalize_request. If
>>   * we are doing a group prealloc we try to normalize the request to
>>   * sbi->s_mb_group_prealloc. Default value of s_mb_group_prealloc is
>> - * 512 blocks. This can be tuned via
>> - * /sys/fs/ext4/<partition/mb_group_prealloc. The value is represented in
>> - * terms of number of blocks. If we have mounted the file system with -O
>> + * 512 blocks. If we have mounted the file system with -O
>>   * stripe=<value> option the group prealloc request is normalized to the
>> - * stripe value (sbi->s_stripe)
>> + * the smallest multiple of the stripe value (sbi->s_stripe) which is
>> + * greater than the default mb_group_prealloc. This can be tuned via
>> + * /sys/fs/ext4/<partition>/mb_group_prealloc. The value is represented in
>> + * terms of number of blocks.
>>   *
>> - * The regular allocator(using the buddy cache) supports few tunables.
>> + * The regular allocator (using the buddy cache) supports a few tunables.
>>   *
>>   * /sys/fs/ext4/<partition>/mb_min_to_scan
>>   * /sys/fs/ext4/<partition>/mb_max_to_scan
>> @@ -2471,7 +2472,26 @@ int ext4_mb_init(struct super_block *sb, int needs_recovery)
>>       sbi->s_mb_stats = MB_DEFAULT_STATS;
>>       sbi->s_mb_stream_request = MB_DEFAULT_STREAM_THRESHOLD;
>>       sbi->s_mb_order2_reqs = MB_DEFAULT_ORDER2_REQS;
>> +     /*
>> +      * If the stripe width is 1, this makes no sense and
>> +      * we set it to 0 to turn off stripe handling code.
>> +      */
>> +     if (sbi->s_stripe == 1)
>> +             sbi->s_stripe = 0;
>
> This strikes me as a weird band-aid-y place to fix this up.
>
> Wouldn't it be better suited for the option-parsing code, and/or
> in ext4_get_stripe_size()?  Why let a value of 1 get this far
> only to override it here?
>
> -Eric

The mount option parsing code wouldn't be a working place to do it,
since it can be specified on-disk what the stripe size is. But
ext4_get_stripe_size might be a good place to put it instead. I guess
there are two unrelated parts to this patch: handling where the stripe
width was set to 1, and handling where it's more than 1 but much less
than what it should be.

Dan

Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/