2017-12-30 11:14 GMT+03:00 Dmitrii Tcvetkov <[email protected]>: > On Sat, 30 Dec 2017 03:15:20 +0300 > Timofey Titovets <[email protected]> wrote: > >> 2017-12-29 22:14 GMT+03:00 Dmitrii Tcvetkov <[email protected]>: >> > On Fri, 29 Dec 2017 21:44:19 +0300 >> > Dmitrii Tcvetkov <[email protected]> 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 [email protected] >> > 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 [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes, my bad, copy-paste error, will be fixed in v3 Thanks -- Have a nice day, Timofey. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
