diff mbox

generic/224: Increase filesystem instance size to 1.5 GiB

Message ID 1440945981-323-1-git-send-email-chandan@linux.vnet.ibm.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Chandan Rajendra Aug. 30, 2015, 2:46 p.m. UTC
For small filesystem instances (i.e. size <= 1 GiB), mkfs.btrfs fails when
"data block size" does not match with the "metadata block size" specified on
the mkfs.btrfs command line. This commit increases the size of filesystem
instance created so that the test can be executed on subpagesize-blocksize
Btrfs instances which have different values for data and metadata blocksizes.

Increasing the filesystem size had a negligible impact on the runtime of the
test as indicated by the following table:

|------------+--------------+----------------|
| Filesystem | 1GiB FS size | 1.5GiB FS size |
|------------+--------------+----------------|
| Btrfs      | 21secs       | 24secs         |
| Ext4       | 12secs       | 15secs         |
| Xfs        | 10secs       | 11secs         |
|------------+--------------+----------------|

Timing information reported in the above table were the worst-case times
observed when running the test on a spinning disk.

Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
---
 tests/generic/224 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Theodore Ts'o Aug. 31, 2015, 6:11 p.m. UTC | #1
On Sun, Aug 30, 2015 at 08:16:21PM +0530, Chandan Rajendra wrote:
> For small filesystem instances (i.e. size <= 1 GiB), mkfs.btrfs fails when
> "data block size" does not match with the "metadata block size" specified on
> the mkfs.btrfs command line. This commit increases the size of filesystem
> instance created so that the test can be executed on subpagesize-blocksize
> Btrfs instances which have different values for data and metadata blocksizes.

Stupid question --- why isn't this considered a bug in mkfs.btrfs?
Does btrfs simply not support file systems <= 1 GB?  So if someone has
a 1GB USB disk or SD card, what's the official advice from the btrfs
developers?  Use xfs or ext4?

							- Ted
--
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
Austin S. Hemmelgarn Aug. 31, 2015, 7:19 p.m. UTC | #2
On 2015-08-31 14:11, Theodore Ts'o wrote:
> On Sun, Aug 30, 2015 at 08:16:21PM +0530, Chandan Rajendra wrote:
>> For small filesystem instances (i.e. size <= 1 GiB), mkfs.btrfs fails when
>> "data block size" does not match with the "metadata block size" specified on
>> the mkfs.btrfs command line. This commit increases the size of filesystem
>> instance created so that the test can be executed on subpagesize-blocksize
>> Btrfs instances which have different values for data and metadata blocksizes.
>
> Stupid question --- why isn't this considered a bug in mkfs.btrfs?
> Does btrfs simply not support file systems <= 1 GB?  So if someone has
> a 1GB USB disk or SD card, what's the official advice from the btrfs
> developers?  Use xfs or ext4?
AFAIK, it shouldn't be failing that way, and should automatically switch 
to mixed mode allocation.  A 1G filesystem should work fine for BTRFS, 
but smaller ones will have higher chances of ENOSPC issues (inversely 
proportional to the size of the FS).  I would advise against using BTRFS 
on such a small disk (I avoid using it on anything smaller than 4G 
personally), but I'm not one of the developers, and the fact that I feel 
it isn't a good idea doesn't mean it shouldn't work.
Theodore Ts'o Aug. 31, 2015, 9:09 p.m. UTC | #3
On Mon, Aug 31, 2015 at 03:19:22PM -0400, Austin S Hemmelgarn wrote:
> AFAIK, it shouldn't be failing that way, and should automatically switch to
> mixed mode allocation.  A 1G filesystem should work fine for BTRFS, but
> smaller ones will have higher chances of ENOSPC issues (inversely
> proportional to the size of the FS).  I would advise against using BTRFS on
> such a small disk (I avoid using it on anything smaller than 4G personally),
> but I'm not one of the developers, and the fact that I feel it isn't a good
> idea doesn't mean it shouldn't work.

Instead of modifying generic/224, maybe it would be better to have a
way to specify a minimum file system size on a per-file system basis.
That way, if some file system does have a minimum size of say, 1G or
4G, it can be configured in one place, instead of needing to modify
every test that uses a small file system size, or forcing all file
systems to use a larger file systems just for the benefit of a single
file system?

Or maybe just fix mkfs.btrfs?

						- Ted
--
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
Chandan Rajendra Sept. 1, 2015, 12:19 a.m. UTC | #4
On Monday 31 Aug 2015 14:11:27 Theodore Ts'o wrote:
> On Sun, Aug 30, 2015 at 08:16:21PM +0530, Chandan Rajendra wrote:
> > For small filesystem instances (i.e. size <= 1 GiB), mkfs.btrfs fails when
> > "data block size" does not match with the "metadata block size" specified
> > on the mkfs.btrfs command line. This commit increases the size of
> > filesystem instance created so that the test can be executed on
> > subpagesize-blocksize Btrfs instances which have different values for
> > data and metadata blocksizes.
> Stupid question --- why isn't this considered a bug in mkfs.btrfs?
> Does btrfs simply not support file systems <= 1 GB?  So if someone has
> a 1GB USB disk or SD card, what's the official advice from the btrfs
> developers?  Use xfs or ext4?
> 
Ted, Btrfs does indeed support filesystem instances <= 1GiB. When creating
such instances, mixed block groups are created i.e. These block groups hold
both data and metadata. Hence the requirement of having matching "data block
size" and "metadata block size" for filesystems <= 1 GiB.

mkfs.btrfs when invoked on small filesystems by "not" specifying any block
sizes (i.e. mkfs.btrfs -f /dev/sda1) will automatically create filesystem
instance with "data block size" == "metadata block size". However in the
subpagesize-blocksize scenario, we need to specify both data and metadata
block size on the command line (For e.g. mkfs.btrfs -f -s 4096 -n 16384
/dev/sda1). In this case, Since the user is forcing the block sizes and it is
impossible to have mixed block groups with differing data and metadata block
sizes, mkfs.btrfs will fail.
Chandan Rajendra Sept. 1, 2015, 12:38 a.m. UTC | #5
On Tuesday 01 Sep 2015 05:49:14 Chandan Rajendra wrote:
> On Monday 31 Aug 2015 14:11:27 Theodore Ts'o wrote:
> > On Sun, Aug 30, 2015 at 08:16:21PM +0530, Chandan Rajendra wrote:
> > > For small filesystem instances (i.e. size <= 1 GiB), mkfs.btrfs fails
> > > when
> > > "data block size" does not match with the "metadata block size"
> > > specified
> > > on the mkfs.btrfs command line. This commit increases the size of
> > > filesystem instance created so that the test can be executed on
> > > subpagesize-blocksize Btrfs instances which have different values for
> > > data and metadata blocksizes.
> > 
> > Stupid question --- why isn't this considered a bug in mkfs.btrfs?
> > Does btrfs simply not support file systems <= 1 GB?  So if someone has
> > a 1GB USB disk or SD card, what's the official advice from the btrfs
> > developers?  Use xfs or ext4?
> 
> Ted, Btrfs does indeed support filesystem instances <= 1GiB. When creating
> such instances, mixed block groups are created i.e. These block groups hold
> both data and metadata. Hence the requirement of having matching "data block
> size" and "metadata block size" for filesystems <= 1 GiB.
> 
> mkfs.btrfs when invoked on small filesystems by "not" specifying any block
> sizes (i.e. mkfs.btrfs -f /dev/sda1) will automatically create filesystem
> instance with "data block size" == "metadata block size". However in the
> subpagesize-blocksize scenario, we need to specify both data and metadata
> block size on the command line (For e.g. mkfs.btrfs -f -s 4096 -n 16384
> /dev/sda1). In this case, Since the user is forcing the block sizes and it
> is impossible to have mixed block groups with differing data and metadata
> block sizes, mkfs.btrfs will fail.

Also, For small filesystems, "mkfs.btrfs -f /dev/sda1 -s 4096 -n 4096" works
fine since the command is invoked with the same size for both data and
metadata blocks.
Theodore Ts'o Sept. 1, 2015, 2:15 a.m. UTC | #6
On Tue, Sep 01, 2015 at 05:49:14AM +0530, Chandan Rajendra wrote:
> mkfs.btrfs when invoked on small filesystems by "not" specifying any block
> sizes (i.e. mkfs.btrfs -f /dev/sda1) will automatically create filesystem
> instance with "data block size" == "metadata block size". However in the
> subpagesize-blocksize scenario, we need to specify both data and metadata
> block size on the command line (For e.g. mkfs.btrfs -f -s 4096 -n 16384
> /dev/sda1). In this case, Since the user is forcing the block sizes and it is
> impossible to have mixed block groups with differing data and metadata block
> sizes, mkfs.btrfs will fail.

Ok, so the issue is that for this particular test configuration, btrfs
has a minimum file system size.  What about changing
_scratch_mkfs_sized so that if MIN_FS_SIZE is set, the file system
created will be at least MIN_FS_SIZE in size.

This way it sets the minimum file system size for all tests, not just
generic/224, and any test configuration, whether it be ext4, xfs, or
btrfs where the data and metadata block size are the same, don't have
to take extra time -- only the test configuration of btrfs with
data_block_size != metadata_block_size.

Cheers,

						- Ted
--
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
Chandan Rajendra Sept. 1, 2015, 2:33 a.m. UTC | #7
On Monday 31 Aug 2015 22:15:10 Theodore Ts'o wrote:
> On Tue, Sep 01, 2015 at 05:49:14AM +0530, Chandan Rajendra wrote:
> > mkfs.btrfs when invoked on small filesystems by "not" specifying any block
> > sizes (i.e. mkfs.btrfs -f /dev/sda1) will automatically create filesystem
> > instance with "data block size" == "metadata block size". However in the
> > subpagesize-blocksize scenario, we need to specify both data and metadata
> > block size on the command line (For e.g. mkfs.btrfs -f -s 4096 -n 16384
> > /dev/sda1). In this case, Since the user is forcing the block sizes and it
> > is impossible to have mixed block groups with differing data and metadata
> > block sizes, mkfs.btrfs will fail.
> 
> Ok, so the issue is that for this particular test configuration, btrfs
> has a minimum file system size.  What about changing
> _scratch_mkfs_sized so that if MIN_FS_SIZE is set, the file system
> created will be at least MIN_FS_SIZE in size.
> 
> This way it sets the minimum file system size for all tests, not just
> generic/224, and any test configuration, whether it be ext4, xfs, or
> btrfs where the data and metadata block size are the same, don't have
> to take extra time -- only the test configuration of btrfs with
> data_block_size != metadata_block_size.
> 
I agree with the approach you have suggested. I will write up a patch and send
it across the mailing list. Thanks Ted.
Qu Wenruo Sept. 11, 2015, 1:38 a.m. UTC | #8
Chandan Rajendra wrote on 2015/09/01 08:03 +0530:
> On Monday 31 Aug 2015 22:15:10 Theodore Ts'o wrote:
>> On Tue, Sep 01, 2015 at 05:49:14AM +0530, Chandan Rajendra wrote:
>>> mkfs.btrfs when invoked on small filesystems by "not" specifying any block
>>> sizes (i.e. mkfs.btrfs -f /dev/sda1) will automatically create filesystem
>>> instance with "data block size" == "metadata block size". However in the
>>> subpagesize-blocksize scenario, we need to specify both data and metadata
>>> block size on the command line (For e.g. mkfs.btrfs -f -s 4096 -n 16384
>>> /dev/sda1). In this case, Since the user is forcing the block sizes and it
>>> is impossible to have mixed block groups with differing data and metadata
>>> block sizes, mkfs.btrfs will fail.
>>
>> Ok, so the issue is that for this particular test configuration, btrfs
>> has a minimum file system size.  What about changing
>> _scratch_mkfs_sized so that if MIN_FS_SIZE is set, the file system
>> created will be at least MIN_FS_SIZE in size.
>>
>> This way it sets the minimum file system size for all tests, not just
>> generic/224, and any test configuration, whether it be ext4, xfs, or
>> btrfs where the data and metadata block size are the same, don't have
>> to take extra time -- only the test configuration of btrfs with
>> data_block_size != metadata_block_size.
>>
> I agree with the approach you have suggested. I will write up a patch and send
> it across the mailing list. Thanks Ted.
>
Hi Chandan,

Although the reply may be a little late, but I think it's still better 
to fix the bug in mkfs.btrfs.

The core problem is, why mkfs.btrfs is insisting on make the fs into 
mixed-bg.

Auto detection for small device and make it into mixed-bg should be a 
"OPTIONAL" feature, not a "mandatory" one.

So IMHO, the best method is to disable mixed-bg when nodesize/sectorsize 
is given and differs and user doesn't force mkfs to create mixed-bg.

Thanks,
Qu
--
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
Chandan Rajendra Sept. 22, 2015, 5:18 a.m. UTC | #9
On Friday 11 Sep 2015 09:38:43 Qu Wenruo wrote:

Qu, Sorry for the late response. My email client's filtering had broken and
hence I did not see your response until now.

> Hi Chandan,
> 
> Although the reply may be a little late, but I think it's still better
> to fix the bug in mkfs.btrfs.
> 
> The core problem is, why mkfs.btrfs is insisting on make the fs into
> mixed-bg.
> 
> Auto detection for small device and make it into mixed-bg should be a
> "OPTIONAL" feature, not a "mandatory" one.
> 
> So IMHO, the best method is to disable mixed-bg when nodesize/sectorsize
> is given and differs and user doesn't force mkfs to create mixed-bg.
> 

My understanding of the chunk allocator (i.e. __btrfs_alloc_chunk()) was
incomplete. Looking into the code more thoroughly I now realize that 'small 
filesystem detection and mixed-bg creation' can be made optional. Thanks for
the suggestion.
diff mbox

Patch

diff --git a/tests/generic/224 b/tests/generic/224
index 391d877..8fae187 100755
--- a/tests/generic/224
+++ b/tests/generic/224
@@ -53,8 +53,9 @@  _supported_os Linux
 
 _require_scratch
 
-# make a 1GB filesystem
-_scratch_mkfs_sized `expr 1024 \* 1024 \* 1024` > $seqres.full 2>&1
+# make a 1.5GB filesystem
+FS_SIZE=$(echo "1.5 $((1024 * 1024 * 1024))" | awk '{printf "%d", $1 * $2}')
+_scratch_mkfs_sized $FS_SIZE > $seqres.full 2>&1
 _scratch_mount >> $seqres.full 2>&1
 
 # set the reserved block pool to almost empty for XFS