On Sat, 30 Dec 2017 03:15:20 +0300
Timofey Titovets <nefelim4ag@xxxxxxxxx> wrote:
> 2017-12-29 22:14 GMT+03:00 Dmitrii Tcvetkov <demfloro@xxxxxxxxxxx>:
> > On Fri, 29 Dec 2017 21:44:19 +0300
> > Dmitrii Tcvetkov <demfloro@xxxxxxxxxxx> wrote:
> >> > +/**
> >> > + * guess_optimal - return guessed optimal mirror
> >> > + *
> >> > + * Optimal expected to be pid % num_stripes
> >> > + *
> >> > + * That's generaly ok for spread load
> >> > + * Add some balancer based on queue leght to device
> >> > + *
> >> > + * Basic ideas:
> >> > + * - Sequential read generate low amount of request
> >> > + * so if load of drives are equal, use pid % num_stripes
> >> > balancing
> >> > + * - For mixed rotate/non-rotate mirrors, pick non-rotate as
> >> > optimal
> >> > + * and repick if other dev have "significant" less queue
> >> > lenght
> >> > + * - Repick optimal if queue leght of other mirror are less
> >> > + */
> >> > +static int guess_optimal(struct map_lookup *map, int optimal)
> >> > +{
> >> > + int i;
> >> > + int round_down = 8;
> >> > + int num = map->num_stripes;
> >>
> >> num has to be initialized from map->sub_stripes if we're reading
> >> RAID10, otherwise there will be NULL pointer dereference
> >>
> >
> > Check can be like:
> > if (map->type & BTRFS_BLOCK_GROUP_RAID10)
> > num = map->sub_stripes;
> >
> >>@@ -5804,10 +5914,12 @@ static int __btrfs_map_block(struct
> >>btrfs_fs_info *fs_info,
> >> stripe_index += mirror_num - 1;
> >> else {
> >> int old_stripe_index = stripe_index;
> >>+ optimal = guess_optimal(map,
> >>+ current->pid %
> >>map->num_stripes);
> >> stripe_index = find_live_mirror(fs_info, map,
> >> stripe_index,
> >> map->sub_stripes,
> >> stripe_index +
> >>- current->pid %
> >>map->sub_stripes,
> >>+ optimal,
> >> dev_replace_is_ongoing);
> >> mirror_num = stripe_index - old_stripe_index
> >> + 1; }
> >>--
> >>2.15.1
> >
> > Also here calculation should be with map->sub_stripes too.
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Why you think we need such check?
> I.e. guess_optimal always called for find_live_mirror()
> Both in same context, like that:
>
> if (map->type & BTRFS_BLOCK_GROUP_RAID10) {
> u32 factor = map->num_stripes / map->sub_stripes;
>
> stripe_nr = div_u64_rem(stripe_nr, factor, &stripe_index);
> stripe_index *= map->sub_stripes;
>
> if (need_full_stripe(op))
> num_stripes = map->sub_stripes;
> else if (mirror_num)
> stripe_index += mirror_num - 1;
> else {
> int old_stripe_index = stripe_index;
> stripe_index = find_live_mirror(fs_info, map,
> stripe_index,
> map->sub_stripes, stripe_index +
> current->pid % map->sub_stripes,
> dev_replace_is_ongoing);
> mirror_num = stripe_index - old_stripe_index + 1;
> }
>
> That useless to check that internally
My bad, so only need to call
guess_optimal(map, current->pid % map->sub_stripes)
in RAID10 branch.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
