On Fri, Jul 14, 2017 at 04:19:07PM +0800, Qu Wenruo wrote: > On 2017年07月14日 15:44, Nikolay Borisov wrote: > > On 28.06.2017 08:43, Qu Wenruo wrote: > >> Introduce a new function, btrfs_check_rw_degradable(), to check if all > >> chunks in btrfs is OK for degraded rw mount. > >> > >> It provides the new basis for accurate btrfs mount/remount and even > >> runtime degraded mount check other than old one-size-fit-all method. > >> > >> Signed-off-by: Qu Wenruo <quwen...@cn.fujitsu.com> > >> Signed-off-by: Anand Jain <anand.j...@oracle.com> > >> --- > >> fs/btrfs/volumes.c | 58 > >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> fs/btrfs/volumes.h | 1 + > >> 2 files changed, 59 insertions(+) > >> > >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > >> index c95f018d4a1e..7a72fbdb8262 100644 > >> --- a/fs/btrfs/volumes.c > >> +++ b/fs/btrfs/volumes.c > >> @@ -6817,6 +6817,64 @@ int btrfs_read_sys_array(struct btrfs_fs_info > >> *fs_info) > >> return -EIO; > >> } > >> > >> +/* > >> + * Check if all chunks in the fs is OK for read-write degraded mount > >> + * > >> + * Return true if all chunks meet the minimal RW mount requirement. > >> + * Return false if any chunk doesn't meet the minimal RW mount > >> requirement. > >> + */ > >> +bool btrfs_check_rw_degradable(struct btrfs_fs_info *fs_info) > >> +{ > >> + struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree; > >> + struct extent_map *em; > >> + u64 next_start = 0; > >> + bool ret = true; > >> + > >> + read_lock(&map_tree->map_tree.lock); > >> + em = lookup_extent_mapping(&map_tree->map_tree, 0, (u64)-1); > >> + read_unlock(&map_tree->map_tree.lock); > >> + /* No chunk at all? Return false anyway */ > >> + if (!em) { > >> + ret = false; > >> + goto out; > >> + } > >> + while (em) { > >> + struct map_lookup *map; > >> + int missing = 0; > >> + int max_tolerated; > >> + int i; > >> + > >> + map = em->map_lookup; > >> + max_tolerated = > >> + btrfs_get_num_tolerated_disk_barrier_failures( > >> + map->type); > >> + for (i = 0; i < map->num_stripes; i++) { > >> + struct btrfs_device *dev = map->stripes[i].dev; > >> + > >> + if (!dev || !dev->bdev || dev->missing || > >> + dev->last_flush_error) > >> + missing++; > >> + } > >> + if (missing > max_tolerated) { > >> + ret = false; > >> + btrfs_warn(fs_info, > >> + "chunk %llu missing %d devices, max tolerance is %d for writeble mount", > >> + em->start, missing, max_tolerated); > >> + free_extent_map(em); > >> + goto out; > >> + } > >> + next_start = extent_map_end(em); > >> + free_extent_map(em); > >> + > >> + read_lock(&map_tree->map_tree.lock); > >> + em = lookup_extent_mapping(&map_tree->map_tree, next_start, > >> + (u64)(-1) - next_start); > >> + read_unlock(&map_tree->map_tree.lock); > >> + } > >> +out: > >> + return ret; > >> +} > >> + > > > > Nit but I think in this function it would be best to directly return > > true/false based on context rather than having the superfluous goto. > > Right, the goto out is not necessary as it's original design to handle > extent map unlock. > But the final patch uses the current method to free extent map. > > Indeed just returning true and false will be better, but goto out also > seems fine to me.
Yeah, it conforms to the pattern of single return point, though this usually is best in functions with multiple jumps sources and some non-trivial cleanup code. -- 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