linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Preparatory to add the csum check in the scan context
@ 2018-03-20 23:33 Anand Jain
  2018-03-20 23:33 ` [PATCH v2 1/2] btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the type Anand Jain
  2018-03-20 23:33 ` [PATCH v2 2/2] btrfs: return required error from btrfs_check_super_csum Anand Jain
  0 siblings, 2 replies; 5+ messages in thread
From: Anand Jain @ 2018-03-20 23:33 UTC (permalink / raw)
  To: linux-btrfs

v1->v2:
  Merge 2/3 and 3/3
  Use -EUCLEAN for csum mismatch
  Move the error logging to the parent function
  Drop the struct btrfs_fsinfo as the argument for
        btrfs_check_super_csum()
  Do not make btrfs_check_super_csum() nonstatic here, it will be part
        of the patchset which will use the csum in the scan context

Anand Jain (2):
  btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the
    type
  btrfs: return required error from btrfs_check_super_csum

 fs/btrfs/disk-io.c | 57 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 27 deletions(-)

-- 
2.15.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the type
  2018-03-20 23:33 [PATCH v2 0/2] Preparatory to add the csum check in the scan context Anand Jain
@ 2018-03-20 23:33 ` Anand Jain
  2018-03-21  6:23   ` Nikolay Borisov
  2018-03-20 23:33 ` [PATCH v2 2/2] btrfs: return required error from btrfs_check_super_csum Anand Jain
  1 sibling, 1 reply; 5+ messages in thread
From: Anand Jain @ 2018-03-20 23:33 UTC (permalink / raw)
  To: linux-btrfs

%csum_type is being checked to check the number of csum types we support,
which is 1 as of now. Instead just check if the type matches with the
only type we support, that is BTRFS_CSUM_TYPE_CRC32. And further adds
cleanup.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1657d6aa4fa6..582ed6af3c50 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -399,32 +399,29 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
 	u16 csum_type = btrfs_super_csum_type(disk_sb);
-	int ret = 0;
-
-	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
-		u32 crc = ~(u32)0;
-		char result[sizeof(crc)];
-
-		/*
-		 * The super_block structure does not span the whole
-		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
-		 * is filled with zeros and is included in the checksum.
-		 */
-		crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
-				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-		btrfs_csum_final(crc, result);
-
-		if (memcmp(raw_disk_sb, result, sizeof(result)))
-			ret = 1;
-	}
+	u32 crc = ~(u32)0;
+	char result[sizeof(crc)];
 
-	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
+	/* We support csum type crc32 only as of now */
+	if (csum_type != BTRFS_CSUM_TYPE_CRC32) {
 		btrfs_err(fs_info, "unsupported checksum algorithm %u",
-				csum_type);
-		ret = 1;
+			  csum_type);
+		return 1;
 	}
 
-	return ret;
+	/*
+	 * The super_block structure does not span the whole
+	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+	 * is filled with zeros and is included in the checksum.
+	 */
+	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
+			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+	btrfs_csum_final(crc, result);
+
+	if (memcmp(raw_disk_sb, result, sizeof(result)))
+		return 1;
+
+	return 0;
 }
 
 /*
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] btrfs: return required error from btrfs_check_super_csum
  2018-03-20 23:33 [PATCH v2 0/2] Preparatory to add the csum check in the scan context Anand Jain
  2018-03-20 23:33 ` [PATCH v2 1/2] btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the type Anand Jain
@ 2018-03-20 23:33 ` Anand Jain
  2018-03-21  6:25   ` Nikolay Borisov
  1 sibling, 1 reply; 5+ messages in thread
From: Anand Jain @ 2018-03-20 23:33 UTC (permalink / raw)
  To: linux-btrfs

Return the required -EINVAL and -EUCLEAN from the function
btrfs_check_super_csum(). And more the error log into the
parent function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 582ed6af3c50..4c6de2743250 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -392,9 +392,10 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
 /*
  * Return 0 if the superblock checksum type matches the checksum value of that
  * algorithm. Pass the raw disk superblock data.
+ * Otherwise: -EINVAL  if csum type is not found
+ * 	      -EUCLEAN if csum does not match
  */
-static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
-				  char *raw_disk_sb)
+static int btrfs_check_super_csum(char *raw_disk_sb)
 {
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
@@ -403,11 +404,8 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	char result[sizeof(crc)];
 
 	/* We support csum type crc32 only as of now */
-	if (csum_type != BTRFS_CSUM_TYPE_CRC32) {
-		btrfs_err(fs_info, "unsupported checksum algorithm %u",
-			  csum_type);
-		return 1;
-	}
+	if (csum_type != BTRFS_CSUM_TYPE_CRC32)
+		return -EINVAL;
 
 	/*
 	 * The super_block structure does not span the whole
@@ -419,7 +417,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	btrfs_csum_final(crc, result);
 
 	if (memcmp(raw_disk_sb, result, sizeof(result)))
-		return 1;
+		return -EUCLEAN;
 
 	return 0;
 }
@@ -2571,9 +2569,17 @@ int open_ctree(struct super_block *sb,
 	 * We want to check superblock checksum, the type is stored inside.
 	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
 	 */
-	if (btrfs_check_super_csum(fs_info, bh->b_data)) {
-		btrfs_err(fs_info, "superblock checksum mismatch");
-		err = -EINVAL;
+	err = btrfs_check_super_csum(bh->b_data);
+	if (err) {
+		if (err == -EINVAL)
+			pr_err("BTRFS error (device %pg): unsupported checksum algorithm",
+				fs_devices->latest_bdev);
+		else if (err == -EUCLEAN)
+			pr_err("BTRFS error (device %pg): superblock checksum mismatch",
+				fs_devices->latest_bdev);
+		else
+			pr_err("BTRFS error (device %pg): checksum check failed %d",
+				fs_devices->latest_bdev, err);
 		brelse(bh);
 		goto fail_alloc;
 	}
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the type
  2018-03-20 23:33 ` [PATCH v2 1/2] btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the type Anand Jain
@ 2018-03-21  6:23   ` Nikolay Borisov
  0 siblings, 0 replies; 5+ messages in thread
From: Nikolay Borisov @ 2018-03-21  6:23 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On 21.03.2018 01:33, Anand Jain wrote:
> %csum_type is being checked to check the number of csum types we support,
> which is 1 as of now. Instead just check if the type matches with the
> only type we support, that is BTRFS_CSUM_TYPE_CRC32. And further adds
> cleanup.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/disk-io.c | 41 +++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 1657d6aa4fa6..582ed6af3c50 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -399,32 +399,29 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  	struct btrfs_super_block *disk_sb =
>  		(struct btrfs_super_block *)raw_disk_sb;
>  	u16 csum_type = btrfs_super_csum_type(disk_sb);
> -	int ret = 0;
> -
> -	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
> -		u32 crc = ~(u32)0;
> -		char result[sizeof(crc)];
> -
> -		/*
> -		 * The super_block structure does not span the whole
> -		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> -		 * is filled with zeros and is included in the checksum.
> -		 */
> -		crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> -				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> -		btrfs_csum_final(crc, result);
> -
> -		if (memcmp(raw_disk_sb, result, sizeof(result)))
> -			ret = 1;
> -	}
> +	u32 crc = ~(u32)0;
> +	char result[sizeof(crc)];
>  
> -	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> +	/* We support csum type crc32 only as of now */
> +	if (csum_type != BTRFS_CSUM_TYPE_CRC32) {
>  		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -				csum_type);
> -		ret = 1;
> +			  csum_type);
> +		return 1;
>  	}
>  
> -	return ret;
> +	/*
> +	 * The super_block structure does not span the whole
> +	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> +	 * is filled with zeros and is included in the checksum.
> +	 */
> +	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> +			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> +	btrfs_csum_final(crc, result);
> +
> +	if (memcmp(raw_disk_sb, result, sizeof(result)))
> +		return 1;
> +
> +	return 0;
>  }
>  
>  /*
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] btrfs: return required error from btrfs_check_super_csum
  2018-03-20 23:33 ` [PATCH v2 2/2] btrfs: return required error from btrfs_check_super_csum Anand Jain
@ 2018-03-21  6:25   ` Nikolay Borisov
  0 siblings, 0 replies; 5+ messages in thread
From: Nikolay Borisov @ 2018-03-21  6:25 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On 21.03.2018 01:33, Anand Jain wrote:
> Return the required -EINVAL and -EUCLEAN from the function
> btrfs_check_super_csum(). And more the error log into the
> parent function.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/disk-io.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 582ed6af3c50..4c6de2743250 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -392,9 +392,10 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
>  /*
>   * Return 0 if the superblock checksum type matches the checksum value of that
>   * algorithm. Pass the raw disk superblock data.
> + * Otherwise: -EINVAL  if csum type is not found
> + * 	      -EUCLEAN if csum does not match
>   */
> -static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
> -				  char *raw_disk_sb)
> +static int btrfs_check_super_csum(char *raw_disk_sb)
>  {
>  	struct btrfs_super_block *disk_sb =
>  		(struct btrfs_super_block *)raw_disk_sb;
> @@ -403,11 +404,8 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  	char result[sizeof(crc)];
>  
>  	/* We support csum type crc32 only as of now */
> -	if (csum_type != BTRFS_CSUM_TYPE_CRC32) {
> -		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -			  csum_type);
> -		return 1;
> -	}
> +	if (csum_type != BTRFS_CSUM_TYPE_CRC32)
> +		return -EINVAL;
>  
>  	/*
>  	 * The super_block structure does not span the whole
> @@ -419,7 +417,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  	btrfs_csum_final(crc, result);
>  
>  	if (memcmp(raw_disk_sb, result, sizeof(result)))
> -		return 1;
> +		return -EUCLEAN;
>  
>  	return 0;
>  }
> @@ -2571,9 +2569,17 @@ int open_ctree(struct super_block *sb,
>  	 * We want to check superblock checksum, the type is stored inside.
>  	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
>  	 */
> -	if (btrfs_check_super_csum(fs_info, bh->b_data)) {
> -		btrfs_err(fs_info, "superblock checksum mismatch");
> -		err = -EINVAL;
> +	err = btrfs_check_super_csum(bh->b_data);
> +	if (err) {
> +		if (err == -EINVAL)
> +			pr_err("BTRFS error (device %pg): unsupported checksum algorithm",
> +				fs_devices->latest_bdev);
> +		else if (err == -EUCLEAN)
> +			pr_err("BTRFS error (device %pg): superblock checksum mismatch",
> +				fs_devices->latest_bdev);
> +		else
> +			pr_err("BTRFS error (device %pg): checksum check failed %d",
> +				fs_devices->latest_bdev, err);

This is redundant, it's never going to be executed since only
EINVAL/EUCLEAN is returned.


>  		brelse(bh);
>  		goto fail_alloc;
>  	}
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-03-21  6:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-20 23:33 [PATCH v2 0/2] Preparatory to add the csum check in the scan context Anand Jain
2018-03-20 23:33 ` [PATCH v2 1/2] btrfs: cleanup btrfs_check_super_csum() to check the csum_type to the type Anand Jain
2018-03-21  6:23   ` Nikolay Borisov
2018-03-20 23:33 ` [PATCH v2 2/2] btrfs: return required error from btrfs_check_super_csum Anand Jain
2018-03-21  6:25   ` Nikolay Borisov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).