[v4,1/6] btrfs: Introduce a function to check if all chunks a OK for degraded rw mount
diff mbox

Message ID 20170628054335.18806-2-quwenruo@cn.fujitsu.com
State New
Headers show

Commit Message

Qu Wenruo June 28, 2017, 5:43 a.m. UTC
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 <quwenruo@cn.fujitsu.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h |  1 +
 2 files changed, 59 insertions(+)

Comments

Nikolay Borisov July 14, 2017, 7:44 a.m. UTC | #1
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 <quwenruo@cn.fujitsu.com>
> Signed-off-by: Anand Jain <anand.jain@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.

>  int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
>  {
>  	struct btrfs_root *root = fs_info->chunk_root;
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 6f45fd60d15a..a5897c7a7e86 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -543,4 +543,5 @@ struct list_head *btrfs_get_fs_uuids(void);
>  void btrfs_set_fs_info_ptr(struct btrfs_fs_info *fs_info);
>  void btrfs_reset_fs_info_ptr(struct btrfs_fs_info *fs_info);
>  
> +bool btrfs_check_rw_degradable(struct btrfs_fs_info *fs_info);
>  #endif
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qu Wenruo July 14, 2017, 8:19 a.m. UTC | #2
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 <quwenruo@cn.fujitsu.com>
>> Signed-off-by: Anand Jain <anand.jain@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.

Thanks,
Qu

> 
>>   int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
>>   {
>>   	struct btrfs_root *root = fs_info->chunk_root;
>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>> index 6f45fd60d15a..a5897c7a7e86 100644
>> --- a/fs/btrfs/volumes.h
>> +++ b/fs/btrfs/volumes.h
>> @@ -543,4 +543,5 @@ struct list_head *btrfs_get_fs_uuids(void);
>>   void btrfs_set_fs_info_ptr(struct btrfs_fs_info *fs_info);
>>   void btrfs_reset_fs_info_ptr(struct btrfs_fs_info *fs_info);
>>   
>> +bool btrfs_check_rw_degradable(struct btrfs_fs_info *fs_info);
>>   #endif
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba July 18, 2017, 4:29 p.m. UTC | #3
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 <quwenruo@cn.fujitsu.com>
> >> Signed-off-by: Anand Jain <anand.jain@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

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;
+}
+
 int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_root *root = fs_info->chunk_root;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 6f45fd60d15a..a5897c7a7e86 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -543,4 +543,5 @@  struct list_head *btrfs_get_fs_uuids(void);
 void btrfs_set_fs_info_ptr(struct btrfs_fs_info *fs_info);
 void btrfs_reset_fs_info_ptr(struct btrfs_fs_info *fs_info);
 
+bool btrfs_check_rw_degradable(struct btrfs_fs_info *fs_info);
 #endif