Skip to contentSkip to conversation

btrfs-progs: prevent mkfs from aborting with small volume

PATCH Superseded Hidetoshi Seto 3 participants 7 messages Aug 23, 2013 – Aug 30, 2013 Btrfs file system

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover btrfs-progs: prevent mkfs from aborting with small volume

I found that mkfs.btrfs aborts when one of assigned volume is too small. Here are 2 patches to fix 2 independent problems.
Both are based on top of Chris's btrfs-progs.git:

git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git

Thanks,
H.Seto

Hidetoshi Seto (2):
btrfs-progs: treat reserved 1MB for superblock properly
btrfs-progs: exit if there is not enough free space for mkfs

 ctree.h   |    3 +++
 mkfs.c    |    4 ++++
 volumes.c |    7 ++++++-
 3 files changed, 13 insertions(+), 1 deletions(-)
#1 lore raw

PATCH 1/2 btrfs-progs: treat reserved 1MB for superblock properly

I found that mkfs.btrfs aborts when assigned multi volumes contain a small volume:

  # parted /dev/sdf p
  Model: LSI MegaRAID SAS RMB (scsi)
  Disk /dev/sdf: 72.8GB
  Sector size (logical/physical): 512B/512B
  Partition Table: msdos
  Number  Start   End     Size    Type     File system  Flags
   1      32.3kB  72.4GB  72.4GB  primary
   2      72.4GB  72.8GB  461MB   primary
  # ./mkfs.btrfs -f /dev/sdf1 /dev/sdf2
  :
  SMALL VOLUME: forcing mixed metadata/data groups
  adding device /dev/sdf2 id 2
  mkfs.btrfs: volumes.c:852: btrfs_alloc_chunk: Assertion `!(ret)' failed.
  Aborted (core dumped)

This failure of btrfs_alloc_chunk was caused by following steps:
1) since there is only small space in the small device, mkfs was going to allocate a chunk from free space as much as available. So mkfs called btrfs_alloc_chunk with
size = device->total_bytes - device->used_bytes.
2) To avoid overwriting superblock, btrfs_alloc_chunk starts taking chunks at an offset of 1MB. It means that the layout of a disk will be like:
[[1MB at begging for sb][allocated chunks]* ... free space ... ]
and you can see that the available free space for allocation is:
avail = device->total_bytes - device->used_bytes - 1MB.
3) Therefore there is only free space 1MB less than requested. damn.

So this fix let mkfs know how much spaces are really there.

---
 ctree.h   |    3 +++
 volumes.c |    7 ++++++-
 2 files changed, 9 insertions(+), 1 deletions(-)
ctree.h +3 -0
index 0b0d701..791bd14 100644
@@ -811,6 +811,9 @@ struct btrfs_csum_item {
 	u8 csum;
 } __attribute__ ((__packed__));
 
+/* we have reserved 1M for superblock at the begging of device */
+#define BTRFS_BLOCK_RESERVED_1M_FOR_SUPER	((u64)1024 * 1024)
+
 /* tag for the radix tree of block groups in ram */
 #define BTRFS_BLOCK_GROUP_DATA		(1ULL << 0)
 #define BTRFS_BLOCK_GROUP_SYSTEM	(1ULL << 1)
volumes.c +6 -1
index 0ff2283..bf6b2e1 100644
@@ -283,7 +283,7 @@ static int find_free_dev_extent(struct btrfs_trans_handle *trans,
 	/* we don't want to overwrite the superblock on the drive,
 	 * so we make sure to start at an offset of at least 1MB
 	 */
-	search_start = max((u64)1024 * 1024, search_start);
+	search_start = max(BTRFS_BLOCK_RESERVED_1M_FOR_SUPER, search_start);
 
 	if (root->fs_info->alloc_start + num_bytes <= device->total_bytes)
 		search_start = max(root->fs_info->alloc_start, search_start);
@@ -783,6 +783,11 @@ again:
 	while(index < num_stripes) {
 		device = list_entry(cur, struct btrfs_device, dev_list);
 		avail = device->total_bytes - device->bytes_used;
+		/* we have reserved 1M for superblock at the head of device */
+		if (avail > BTRFS_BLOCK_RESERVED_1M_FOR_SUPER)
+			avail -= BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
+		else
+			avail = 0;
 		cur = cur->next;
 		if (avail >= min_free) {
 			list_move_tail(&device->dev_list, &private_devs);
Signature · 1 line
1.7.1
#2 lore raw

PATCH 2/2 btrfs-progs: exit if there is not enough free space for mkfs

In reply to Hidetoshi Seto

Again, while playing mkfs with small volumes, I found that mkfs.btrfs aborts if there is really no spaces for a brand-new filesystem:

  # ./mkfs.btrfs -f /dev/sdf1 /dev/sdf2
  :
  SMALL VOLUME: forcing mixed metadata/data groups
  adding device /dev/sdf2 id 2
  mkfs.btrfs: mkfs.c:184: create_one_raid_group: Assertion `!(ret)' failed.
  Aborted (core dumped)

This fix let mkfs prints error message if it cannot make filesystem due to a lack of free spaces.

---
 mkfs.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
mkfs.c +4 -0
index b412b7e..3c0bc60 100644
@@ -181,6 +181,10 @@ static int create_one_raid_group(struct btrfs_trans_handle *trans,
 
 	ret = btrfs_alloc_chunk(trans, root->fs_info->extent_root,
 				&chunk_start, &chunk_size, type);
+	if (ret == -ENOSPC) {
+		fprintf(stderr, "not enough free space\n");
+		exit(1);
+	}
 	BUG_ON(ret);
 	ret = btrfs_make_block_group(trans, root->fs_info->extent_root, 0,
 				     type, BTRFS_FIRST_CHUNK_TREE_OBJECTID,
Signature · 1 line
1.7.1
#3 lore raw
In reply to Hidetoshi Seto
Quoted 9 lines · Hidetoshi Seto

I found that mkfs.btrfs aborts when one of assigned volume is too small. Here are 2 patches to fix 2 independent problems.
Both are based on top of Chris's btrfs-progs.git:

git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git

Thanks,
H.Seto

Thanks for looking into this - how small of a device did you test?

I tried a 2MB device w/ these 2 patches and still got:

[btrfs-progs]# truncate --size=2m testfile
[btrfs-progs]# ./mkfs.btrfs testfile

WARNING! - Btrfs v0.20-rc1-360-geeeb4e9 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using

SMALL VOLUME: forcing mixed metadata/data groups
mkfs.btrfs: volumes.c:857: btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)

which was at:

                ret = btrfs_alloc_dev_extent(trans, device,
                             info->chunk_root->root_key.objectid,
                             BTRFS_FIRST_CHUNK_TREE_OBJECTID, key.offset,
                             calc_size, &dev_offset);
                BUG_ON(ret);

:(

Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?

-Eric

Quoted 9 lines

Hidetoshi Seto (2):
btrfs-progs: treat reserved 1MB for superblock properly
btrfs-progs: exit if there is not enough free space for mkfs

 ctree.h   |    3 +++
 mkfs.c    |    4 ++++
 volumes.c |    7 ++++++-
 3 files changed, 13 insertions(+), 1 deletions(-)
#4 lore raw
Quoted 16 lines · Eric Sandeen

Thanks for looking into this - how small of a device did you test?

I tried a 2MB device w/ these 2 patches and still got:

[btrfs-progs]# truncate --size=2m testfile
[btrfs-progs]# ./mkfs.btrfs testfile

WARNING! - Btrfs v0.20-rc1-360-geeeb4e9 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using

SMALL VOLUME: forcing mixed metadata/data groups
mkfs.btrfs: volumes.c:857: btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)

which was at:

Quoted 7 lines
                ret = btrfs_alloc_dev_extent(trans, device,
                             info->chunk_root->root_key.objectid,
                             BTRFS_FIRST_CHUNK_TREE_OBJECTID, key.offset,
                             calc_size, &dev_offset);
                BUG_ON(ret);

:(

Wow...
It seems that this abort is different problem from the bug which my patches are going to fix. I'll try to make new patch to fix this problem.

Quoted 6 lines

Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?

-Eric

I'm not sure... According to the git-log, this 1M trick is in the following old commit by Chris:

commit a6de0bd778475504f42a142c83b8077993cbddfe
Author: Chris Mason <[email protected]>
Date: Thu Apr 3 16:35:48 2008 -0400

Add mirroring support across multiple drives

Thanks,
H.Seto

#5 lore raw
Quoted 37 lines · Hidetoshi Seto
Quoted 16 lines · Eric Sandeen

Thanks for looking into this - how small of a device did you test?

I tried a 2MB device w/ these 2 patches and still got:

[btrfs-progs]# truncate --size=2m testfile
[btrfs-progs]# ./mkfs.btrfs testfile

WARNING! - Btrfs v0.20-rc1-360-geeeb4e9 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using

SMALL VOLUME: forcing mixed metadata/data groups
mkfs.btrfs: volumes.c:857: btrfs_alloc_chunk: Assertion `!(ret)' failed. Aborted (core dumped)

which was at:

Quoted 7 lines
                ret = btrfs_alloc_dev_extent(trans, device,
                             info->chunk_root->root_key.objectid,
                             BTRFS_FIRST_CHUNK_TREE_OBJECTID, key.offset,
                             calc_size, &dev_offset);
                BUG_ON(ret);

:(

Wow...
It seems that this abort is different problem from the bug which my patches are going to fix. I'll try to make new patch to fix this problem.

Quoted 6 lines

Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?

-Eric

Quoted 8 lines

I'm not sure... According to the git-log, this 1M trick is in the following old commit by Chris:

commit a6de0bd778475504f42a142c83b8077993cbddfe
Author: Chris Mason <[email protected]>
Date: Thu Apr 3 16:35:48 2008 -0400

Add mirroring support across multiple drives

Yep I saw that too. Seemingly unrelated. :( Unless I'm missing something (which I probably am).

-Eric

Quoted 4 lines

Thanks,
H.Seto

#6 lore raw
Quoted 7 lines · Eric Sandeen
Quoted 7 lines
Quoted 3 lines

Also, I'm curious - I know the code existed before your patch 2/2, but I don't understand why it reserves 1MB for the first superblock when the first superblock is actually at 64k. Any idea?

I'm not sure... According to the git-log, this 1M trick is in the following old commit by Chris:

Quoted 8 lines
Quoted 5 lines

commit a6de0bd778475504f42a142c83b8077993cbddfe
Author: Chris Mason <[email protected]>
Date: Thu Apr 3 16:35:48 2008 -0400

Add mirroring support across multiple drives

Yep I saw that too. Seemingly unrelated. :( Unless I'm missing something (which I probably am).

IIRC the 1 MB of unused space (minus the first superblock) is there to avoid random overwrites from paritioners or somesuch. If the whole megabyte is overwritten including the superblock, the 1st copy lives at 64 MB which exists on practically every fs and the one at 64k can be retored from it.

david