On thu, 27 Sep 2012 19:56:24 +0300, Ilya Dryomov wrote:
>> the parameters are right. So besides the code cleanup, this patch also
>> add a check for the usage of the space balance, it is the only place that
>> we need add check to make sure the parameters of div_factor are right till
>> now. Besides that, the old kernel may hold the wrong usage value, so we
>> must rectify it.
>
> Cleaning up/unifying duplicated functions and changing the existing
> logic are two very different things. If you, in the course of writing
> this patch, became unhappy with the way balancing ioctl deals with
> "invalid" input, please send a separate patch.
>
> Before your patch, volumes.c had its own copy of div_factor_fine():
>
> static u64 div_factor_fine(u64 num, int factor)
> {
> if (factor <= 0)
> return 0;
> if (factor >= 100)
> return num;
>
> num *= factor;
> do_div(num, 100);
> return num;
> }
>
> which was called from chunk_usage_filter() on unvalidated user input.
> As far as the cleanup part of your patch goes, you've dropped
> factor <= 0 / factor >= 100 logic, merged volumes.c's copy with
> extent-tree.c's copy and renamed div_factor_fine() to div_factor(). To
> make chunk_usage_filter() happy again, it's enough to move the dropped
> logic directly to the call site:
>
> static int chunk_usage_filter(struct btrfs_fs_info *fs_info, u64 chunk_offset,
> struct btrfs_balance_args *bargs)
> {
> ...
>
> - user_thresh = div_factor_fine(cache->key.offset, bargs->usage);
> + if (bargs->usage == 0)
> + user_thresh = 0;
> + else if (bargs->usage >= 100)
> + user_thresh = cache->key.offset;
> + else
> + user_thresh = div_factor(cache->key.offset, bargs->usage);
>
> ...
> }
>
> So I would suggest you drop all hunks related to changing the way
> balancing ioctl works and make the above change to chunk_usage_filter()
> instead. Once again, if you are unhappy with usage filter argument
> handling, send a separate patch.
Fine.
(I forget the rule that one patch just do one thing)
Thanks
Miao
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
