public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: disk-io: Kill the BUG_ON()s in write_and_map_eb()
@ 2020-03-02  1:41 Qu Wenruo
  2020-03-02  3:35 ` Su Yue
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2020-03-02  1:41 UTC (permalink / raw)
  To: linux-btrfs

All callers of write_and_map_eb(), except btrfs-corrupt-block, have
handled error, but inside write_and_map_eb() itself, the only error handling
is BUG_ON().

This patch will kill all the BUG_ON()s inside write_and_map_eb(), and
enhance the the caller in btrfs-corrupt-block() to handle the error.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-corrupt-block.c       |  9 ++++++++-
 cmds/rescue-super-recover.c |  3 ++-
 disk-io.c                   | 26 +++++++++++++++++++++++---
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 95df871a7822..3c236e146176 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -771,8 +771,15 @@ static int corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
 		u64 bogus = generate_u64(orig);
 
 		btrfs_set_header_generation(eb, bogus);
-		write_and_map_eb(fs_info, eb);
+		ret = write_and_map_eb(fs_info, eb);
 		free_extent_buffer(eb);
+		if (ret < 0) {
+			errno = -ret;
+			fprintf(stderr,
+				"failed to write extent buffer at %llu: %m",
+				eb->start);
+			return ret;
+		}
 		break;
 		}
 	case BTRFS_METADATA_BLOCK_SHIFT_ITEMS:
diff --git a/cmds/rescue-super-recover.c b/cmds/rescue-super-recover.c
index 5d6bea836c8b..62f4f7754720 100644
--- a/cmds/rescue-super-recover.c
+++ b/cmds/rescue-super-recover.c
@@ -276,7 +276,8 @@ int btrfs_recover_superblocks(const char *dname,
 				  struct super_block_record, list);
 
 	root = open_ctree(record->device_name, record->bytenr,
-			  OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES);
+			  OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES |
+			  OPEN_CTREE_EXCLUSIVE);
 	if (!root) {
 		ret = 3;
 		goto no_recover;
diff --git a/disk-io.c b/disk-io.c
index e8a2e4afa93a..9ff62fcd54d1 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -487,20 +487,40 @@ int write_and_map_eb(struct btrfs_fs_info *fs_info, struct extent_buffer *eb)
 	length = eb->len;
 	ret = btrfs_map_block(fs_info, WRITE, eb->start, &length,
 			      &multi, 0, &raid_map);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to map bytenr %llu length %u: %m",
+			eb->start, eb->len);
+		goto out;
+	}
 
 	if (raid_map) {
 		ret = write_raid56_with_parity(fs_info, eb, multi,
 					       length, raid_map);
-		BUG_ON(ret);
+		if (ret < 0) {
+			errno = -ret;
+			error(
+		"failed to write raid56 stripe for bytenr %llu length %llu: %m",
+				eb->start, length);
+			goto out;
+		}
 	} else while (dev_nr < multi->num_stripes) {
-		BUG_ON(ret);
 		eb->fd = multi->stripes[dev_nr].dev->fd;
 		eb->dev_bytenr = multi->stripes[dev_nr].physical;
 		multi->stripes[dev_nr].dev->total_ios++;
 		dev_nr++;
 		ret = write_extent_to_disk(eb);
-		BUG_ON(ret);
+		if (ret < 0) {
+			errno = -ret;
+			error(
+"failed to write bytenr %llu length %u devid %llu dev_bytenr %llu: %m",
+				eb->start, eb->len,
+				multi->stripes[dev_nr].dev->devid,
+				eb->dev_bytenr);
+			goto out;
+		}
 	}
+out:
 	kfree(raid_map);
 	kfree(multi);
 	return 0;
-- 
2.25.1


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

* Re: [PATCH] btrfs: disk-io: Kill the BUG_ON()s in write_and_map_eb()
  2020-03-02  1:41 [PATCH] btrfs: disk-io: Kill the BUG_ON()s in write_and_map_eb() Qu Wenruo
@ 2020-03-02  3:35 ` Su Yue
  2020-03-02  4:45   ` Qu Wenruo
  0 siblings, 1 reply; 3+ messages in thread
From: Su Yue @ 2020-03-02  3:35 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs


On Mon 02 Mar 2020 at 09:41, Qu Wenruo <wqu@suse.com> wrote:

> All callers of write_and_map_eb(), except btrfs-corrupt-block,
> have handled error, but inside write_and_map_eb() itself, the
> only error handling is BUG_ON().
>
> This patch will kill all the BUG_ON()s inside
> write_and_map_eb(), and enhance the the caller in
> btrfs-corrupt-block() to handle the error.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com> ---
>  btrfs-corrupt-block.c       |  9 ++++++++-
>  cmds/rescue-super-recover.c |  3 ++-

So may the subject be prefixed with "btrfs-progs"?

>  disk-io.c                   | 26 +++++++++++++++++++++++--- 3
>  files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index
> 95df871a7822..3c236e146176 100644 --- a/btrfs-corrupt-block.c
> +++ b/btrfs-corrupt-block.c @@ -771,8 +771,15 @@ static int
> corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
>  		u64 bogus = generate_u64(orig);
>  btrfs_set_header_generation(eb, bogus);
> -		write_and_map_eb(fs_info, eb); +		ret =
> write_and_map_eb(fs_info, eb);
>  		free_extent_buffer(eb);
> +		if (ret < 0) { +			errno = -ret; +
> fprintf(stderr, +				"failed to write extent buffer at
> %llu: %m", +				eb->start); +			return ret; +
> }
>  		break; } case BTRFS_METADATA_BLOCK_SHIFT_ITEMS:
> diff --git a/cmds/rescue-super-recover.c
> b/cmds/rescue-super-recover.c index 5d6bea836c8b..62f4f7754720
> 100644 --- a/cmds/rescue-super-recover.c +++
> b/cmds/rescue-super-recover.c @@ -276,7 +276,8 @@ int
> btrfs_recover_superblocks(const char *dname,
>  				  struct super_block_record, list);  root =
>  open_ctree(record->device_name, record->bytenr,
> -			  OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES); +
> OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES | +
> OPEN_CTREE_EXCLUSIVE);

Any explanation about this change?

Thanks

>  	if (!root) {
>  		ret = 3;
>  		goto no_recover;
> diff --git a/disk-io.c b/disk-io.c
> index e8a2e4afa93a..9ff62fcd54d1 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -487,20 +487,40 @@ int write_and_map_eb(struct btrfs_fs_info *fs_info, struct extent_buffer *eb)
>  	length = eb->len;
>  	ret = btrfs_map_block(fs_info, WRITE, eb->start, &length,
>  			      &multi, 0, &raid_map);
> +	if (ret < 0) {
> +		errno = -ret;
> +		error("failed to map bytenr %llu length %u: %m",
> +			eb->start, eb->len);
> +		goto out;
> +	}
>
>  	if (raid_map) {
>  		ret = write_raid56_with_parity(fs_info, eb, multi,
>  					       length, raid_map);
> -		BUG_ON(ret);
> +		if (ret < 0) {
> +			errno = -ret;
> +			error(
> +		"failed to write raid56 stripe for bytenr %llu length %llu: %m",
> +				eb->start, length);
> +			goto out;
> +		}
>  	} else while (dev_nr < multi->num_stripes) {
> -		BUG_ON(ret);
>  		eb->fd = multi->stripes[dev_nr].dev->fd;
>  		eb->dev_bytenr = multi->stripes[dev_nr].physical;
>  		multi->stripes[dev_nr].dev->total_ios++;
>  		dev_nr++;
>  		ret = write_extent_to_disk(eb);
> -		BUG_ON(ret);
> +		if (ret < 0) {
> +			errno = -ret;
> +			error(
> +"failed to write bytenr %llu length %u devid %llu dev_bytenr %llu: %m",
> +				eb->start, eb->len,
> +				multi->stripes[dev_nr].dev->devid,
> +				eb->dev_bytenr);
> +			goto out;
> +		}
>  	}
> +out:
>  	kfree(raid_map);
>  	kfree(multi);
>  	return 0;


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

* Re: [PATCH] btrfs: disk-io: Kill the BUG_ON()s in write_and_map_eb()
  2020-03-02  3:35 ` Su Yue
@ 2020-03-02  4:45   ` Qu Wenruo
  0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2020-03-02  4:45 UTC (permalink / raw)
  To: Su Yue, Qu Wenruo; +Cc: linux-btrfs


[-- Attachment #1.1: Type: text/plain, Size: 4531 bytes --]



On 2020/3/2 上午11:35, Su Yue wrote:
> 
> On Mon 02 Mar 2020 at 09:41, Qu Wenruo <wqu@suse.com> wrote:
> 
>> All callers of write_and_map_eb(), except btrfs-corrupt-block,
>> have handled error, but inside write_and_map_eb() itself, the
>> only error handling is BUG_ON().
>>
>> This patch will kill all the BUG_ON()s inside
>> write_and_map_eb(), and enhance the the caller in
>> btrfs-corrupt-block() to handle the error.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com> ---
>>  btrfs-corrupt-block.c       |  9 ++++++++-
>>  cmds/rescue-super-recover.c |  3 ++-
> 
> So may the subject be prefixed with "btrfs-progs"?
> 
>>  disk-io.c                   | 26 +++++++++++++++++++++++--- 3
>>  files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index
>> 95df871a7822..3c236e146176 100644 --- a/btrfs-corrupt-block.c
>> +++ b/btrfs-corrupt-block.c @@ -771,8 +771,15 @@ static int
>> corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
>>          u64 bogus = generate_u64(orig);
>>  btrfs_set_header_generation(eb, bogus);
>> -        write_and_map_eb(fs_info, eb); +        ret =
>> write_and_map_eb(fs_info, eb);
>>          free_extent_buffer(eb);
>> +        if (ret < 0) { +            errno = -ret; +
>> fprintf(stderr, +                "failed to write extent buffer at
>> %llu: %m", +                eb->start); +            return ret; +
>> }
>>          break; } case BTRFS_METADATA_BLOCK_SHIFT_ITEMS:
>> diff --git a/cmds/rescue-super-recover.c
>> b/cmds/rescue-super-recover.c index 5d6bea836c8b..62f4f7754720
>> 100644 --- a/cmds/rescue-super-recover.c +++
>> b/cmds/rescue-super-recover.c @@ -276,7 +276,8 @@ int
>> btrfs_recover_superblocks(const char *dname,
>>                    struct super_block_record, list);  root =
>>  open_ctree(record->device_name, record->bytenr,
>> -              OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES); +
>> OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES | +
>> OPEN_CTREE_EXCLUSIVE);
> 
> Any explanation about this change?

My bad, unclean base.

I'll resend the patch.

Thanks,
Qu
> 
> Thanks
> 
>>      if (!root) {
>>          ret = 3;
>>          goto no_recover;
>> diff --git a/disk-io.c b/disk-io.c
>> index e8a2e4afa93a..9ff62fcd54d1 100644
>> --- a/disk-io.c
>> +++ b/disk-io.c
>> @@ -487,20 +487,40 @@ int write_and_map_eb(struct btrfs_fs_info
>> *fs_info, struct extent_buffer *eb)
>>      length = eb->len;
>>      ret = btrfs_map_block(fs_info, WRITE, eb->start, &length,
>>                    &multi, 0, &raid_map);
>> +    if (ret < 0) {
>> +        errno = -ret;
>> +        error("failed to map bytenr %llu length %u: %m",
>> +            eb->start, eb->len);
>> +        goto out;
>> +    }
>>
>>      if (raid_map) {
>>          ret = write_raid56_with_parity(fs_info, eb, multi,
>>                             length, raid_map);
>> -        BUG_ON(ret);
>> +        if (ret < 0) {
>> +            errno = -ret;
>> +            error(
>> +        "failed to write raid56 stripe for bytenr %llu length %llu: %m",
>> +                eb->start, length);
>> +            goto out;
>> +        }
>>      } else while (dev_nr < multi->num_stripes) {
>> -        BUG_ON(ret);
>>          eb->fd = multi->stripes[dev_nr].dev->fd;
>>          eb->dev_bytenr = multi->stripes[dev_nr].physical;
>>          multi->stripes[dev_nr].dev->total_ios++;
>>          dev_nr++;
>>          ret = write_extent_to_disk(eb);
>> -        BUG_ON(ret);
>> +        if (ret < 0) {
>> +            errno = -ret;
>> +            error(
>> +"failed to write bytenr %llu length %u devid %llu dev_bytenr %llu: %m",
>> +                eb->start, eb->len,
>> +                multi->stripes[dev_nr].dev->devid,
>> +                eb->dev_bytenr);
>> +            goto out;
>> +        }
>>      }
>> +out:
>>      kfree(raid_map);
>>      kfree(multi);
>>      return 0;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2020-03-02  4:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-02  1:41 [PATCH] btrfs: disk-io: Kill the BUG_ON()s in write_and_map_eb() Qu Wenruo
2020-03-02  3:35 ` Su Yue
2020-03-02  4:45   ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox