On Fri, Jul 13, 2018 at 11:02:03PM +0200, Goffredo Baroncelli wrote:
> As general comment, good to hear that something is moving around raid5/6 +
> write hole and multiple mirroring.
> However I am guessing if this is time to simplify the RAID code. There are a
> lot of "if" which could be avoided using
> the values stored in the array "btrfs_raid_array[]".
I absolutely agree and had the same impression during implementing the
feature. For this patchset I did only a minimal prep work, the
suggestions you give below make sense to me.
Enhancing the table would make a lot of code go away and just use one
formula to calculate the results that are now opencoded. I'll be going
through the raid code so I'll get to the cleanups eventually.
> Below some comments:
> > @@ -5075,6 +5093,8 @@ static inline int btrfs_chunk_max_errors(struct
> > map_lookup *map)
> > BTRFS_BLOCK_GROUP_RAID5 |
> > BTRFS_BLOCK_GROUP_DUP)) {
> > max_errors = 1;
> > + } else if (map->type & BTRFS_BLOCK_GROUP_RAID1C3) {
> > + max_errors = 2;
> > } else if (map->type & BTRFS_BLOCK_GROUP_RAID6) {
> > max_errors = 2;
> > } else {
>
> Even in this case the ifs above could be replaced with something like:
>
> index = btrfs_bg_flags_to_raid_index(map->type)
> max_errors = btrfs_raid_array[index].ncopies-1;
There's .tolerated_failures that should equal ncopies - 1 in general,
but does not for DUP so the semantics of the function and caller needs
to be verified.
--
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
