diff mbox series

[RESEND] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values

Message ID 20240314093127.2100974-1-srivathsa.d.dara@oracle.com
State Superseded
Headers show
Series [RESEND] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values | expand

Commit Message

Srivathsa Dara March 14, 2024, 9:31 a.m. UTC 
According to the mke2fs man page, the supported cluster-size values
for an ext4 filesystem are 2048 to 256M bytes. However, this is not
the case.

When mkfs is run to create a filesystem with following specifications:
* 1k blocksize and cluster-size greater than 32M
* 2k blocksize and cluster-size greater than 64M
* 4k blocksize and cluster-size greater than 128M
mkfs fails with "Invalid argument passed to ext2 library while trying
to create journal" error. In general, when the cluster-size to blocksize
ratio is greater than 32k, mkfs fails with this error.

Went through the code and found out that the function
`ext2fs_new_range()` is the source of this error. This is because when
the cluster-size to blocksize ratio exceeds 32k, the length argument
to the function `ext2fs_new_range()` results in 0. Hence, the error.

This patch corrects the valid cluster-size values.
---
 misc/mke2fs.8.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Andreas Dilger March 15, 2024, 9:32 p.m. UTC | #1 
On Mar 14, 2024, at 3:31 AM, Srivathsa Dara <srivathsa.d.dara@oracle.com> wrote:
> 
> According to the mke2fs man page, the supported cluster-size values
> for an ext4 filesystem are 2048 to 256M bytes. However, this is not
> the case.
> 
> When mkfs is run to create a filesystem with following specifications:
> * 1k blocksize and cluster-size greater than 32M
> * 2k blocksize and cluster-size greater than 64M
> * 4k blocksize and cluster-size greater than 128M
> mkfs fails with "Invalid argument passed to ext2 library while trying
> to create journal" error. In general, when the cluster-size to blocksize
> ratio is greater than 32k, mkfs fails with this error.
> 
> Went through the code and found out that the function
> `ext2fs_new_range()` is the source of this error. This is because when
> the cluster-size to blocksize ratio exceeds 32k, the length argument
> to the function `ext2fs_new_range()` results in 0. Hence, the error.
> 
> This patch corrects the valid cluster-size values.
> ---
> misc/mke2fs.8.in | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/misc/mke2fs.8.in b/misc/mke2fs.8.in
> index e6bfc6d6..b5b02144 100644
> --- a/misc/mke2fs.8.in
> +++ b/misc/mke2fs.8.in
> @@ -230,9 +230,9 @@ test is used instead of a fast read-only test.
> .TP
> .B \-C " cluster-size"
> Specify the size of cluster in bytes for filesystems using the bigalloc
> -feature.  Valid cluster-size values are from 2048 to 256M bytes per
> -cluster.  This can only be specified if the bigalloc feature is
> -enabled.  (See the
> +feature.  Valid cluster-size values are from 2048 to 128M bytes per
> +cluster based on filesystem blocksize. This can only be specified if the
> +bigalloc feature is enabled.  (See the
> .B ext4 (5)


This is an improvement, but doesn't really explain the details of the
limits.  Instead of "based on filesystem blocksize." I think writing
"between 2-32768 times the filesystem blocksize." or similar would be
more clear and explain how the actual limits relate to the blocksize.

Cheers, Andreas
Srivathsa Dara March 19, 2024, 11:25 a.m. UTC | #2 
> 
> 
> -----Original Message-----
> From: Andreas Dilger <adilger@dilger.ca> 
> Sent: Saturday, March 16, 2024 3:02 AM
> To: Srivathsa Dara <srivathsa.d.dara@oracle.com>
> Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>; 
> 	Theodore Ts'o <tytso@mit.edu>; Rajesh Sivaramasubramaniom 
> 	<rajesh.sivaramasubramaniom@oracle.com>; 
> 	Junxiao Bi <junxiao.bi@oracle.com>
> Re: [RESEND PATCH] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values
> 
> On Mar 14, 2024, at 3:31 AM, Srivathsa Dara <srivathsa.d.dara@oracle.com> wrote:
> > 
> > According to the mke2fs man page, the supported cluster-size values 
> > for an ext4 filesystem are 2048 to 256M bytes. However, this is not 
> > the case.
> > 
> > When mkfs is run to create a filesystem with following specifications:
> > * 1k blocksize and cluster-size greater than 32M
> > * 2k blocksize and cluster-size greater than 64M
> > * 4k blocksize and cluster-size greater than 128M mkfs fails with 
> > "Invalid argument passed to ext2 library while trying to create 
> > journal" error. In general, when the cluster-size to blocksize ratio 
> > is greater than 32k, mkfs fails with this error.
> > 
> > Went through the code and found out that the function 
> > `ext2fs_new_range()` is the source of this error. This is because when 
> > the cluster-size to blocksize ratio exceeds 32k, the length argument 
> > to the function `ext2fs_new_range()` results in 0. Hence, the error.
> > 
> > This patch corrects the valid cluster-size values.
> > ---
> > misc/mke2fs.8.in | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/misc/mke2fs.8.in b/misc/mke2fs.8.in index 
> > e6bfc6d6..b5b02144 100644
> > --- a/misc/mke2fs.8.in
> > +++ b/misc/mke2fs.8.in
> > @@ -230,9 +230,9 @@ test is used instead of a fast read-only test.
> > .TP
> > .B \-C " cluster-size"
> > Specify the size of cluster in bytes for filesystems using the 
> > bigalloc -feature.  Valid cluster-size values are from 2048 to 256M 
> > bytes per -cluster.  This can only be specified if the bigalloc 
> > feature is -enabled.  (See the
> > +feature.  Valid cluster-size values are from 2048 to 128M bytes per 
> > +cluster based on filesystem blocksize. This can only be specified if 
> > +the bigalloc feature is enabled.  (See the
> > .B ext4 (5)
> 
> 
> This is an improvement, but doesn't really explain the details of the limits.
> Instead of "based on filesystem blocksize." I think writing "between 2-32768 
> times the filesystem blocksize." or similar would be more clear and explain 
> how the actual limits relate to the blocksize.

Hi, Andreas. Thank you for the comment. Here are the details:

The function ext2fs_new_range() is causing the error. This function gets 
called while creating the journal inode.
The purpose of ext2fs_new_range() is to return atleast requested amount of 
free memory.

Following is case, where the function returns successfully:

Breakpoint 5, ext2fs_new_range (fs=fs@entry=0x555555779070, 
flags=flags@entry=0, goal=131072, len=len@entry=32768,
    map=map@entry=0x0, pblk=pblk@entry=0x7fffffffd120, plen=0x7fffffffd128) 
at alloc.c:407
 
If we observe we can see that the argument length is positive value (32768), 
hence the function allocates atleast requested amount of memory and returns 
successfully.
 
Now, lets look at the function arguments, when it is returning error:
 
Breakpoint 5, ext2fs_new_range (fs=fs@entry=0x555555779070, 
flags=flags@entry=0,
    goal=262144, len=len@entry=0, map=map@entry=0x0, 
pblk=pblk@entry=0x7fffffffd140,
    plen=0x7fffffffd148) at alloc.c:407
 
The length argument is 0. Therefore, function is returning an error.
 
The value of len argument is calculate based on the values of blocksize, 
cluster_size.

Following are steps through which len value is calculated:

len = EXT_INIT_MAX_LEN & ~EXT2FS_CLUSTER_MASK(fs);

* EXT_INIT_MAX_LEN = 1 << 15
* EXT2FS_CLUSTER_MASK(fs) = ((1 << (fs)->cluster_ratio_bits) - 1)
* fs->cluster_ratio_bits = super->s_log_cluster_size - super->s_log_block_size;
* super->s_log_cluster_size = int_log2(cluster_size >> EXT2_MIN_CLUSTER_LOG_SIZE);
					where EXT2_MIN_CLUSTER_LOG_SIZE = 10
* super->s_log_block_size = int_log2(blocksize >> EXT2_MIN_BLOCK_LOG_SIZE)
				where EXT2_MIN_BLOCK_LOG_SIZE =  10

Following table which gives len value, for different combinations of 
blocksize and clustersize:

A = blocksize
B = clustersize
C = s_log_block_size
D = s_log_cluster_size
E = D - C
F = ((1<<E)-1)
len = (32768 & ~F)

len is passed as argument to ext2fs_new_range().

Failure cases:
------------------------------------------
A   | B   | C   | D   | E   | F     | len
------------------------------------------
1k    64m   0     16    16    65535    0     

1k   128m   0     17    17    131071   0

1k   256m   0     18    18    262143   0

2k   128m   1     17    16    65535    0

2k   256m   1     18    17    131071   0

4k   256m   2     18    16    65535    0

successful cases:

1k   32m    0     15    15    32767   32768

2k   64m    1     16    15    32767   32768

4k   128m   2     17    15    32767   32768

 
For successful cases, len is valid value (len>0), whereas for the
failed case length is zero. Hence, the error 'Invalid Argument'.

The conclusion is that mkfs.ext4 fails for all the cases, where
the ratio of cluster_size to blocksize is greaterthan or equal to 2^16.

> 
> Cheers, Andreas

Thanks, Srivathsa
Andreas Dilger March 22, 2024, 11:10 p.m. UTC | #3 
> On Mar 19, 2024, at 5:25 AM, Srivathsa Dara <srivathsa.d.dara@oracle.com> wrote:
> 
> On March 16, 2024 3:02 AM, Andreas Dilger <adilger@dilger.ca> wrote:
>> 
>> On Mar 14, 2024, at 3:31 AM, Srivathsa Dara <srivathsa.d.dara@oracle.com> wrote:
>>> 
>>> According to the mke2fs man page, the supported cluster-size values
>>> for an ext4 filesystem are 2048 to 256M bytes. However, this is not
>>> the case.
>>> 
>>> When mkfs is run to create a filesystem with following specifications:
>>> * 1k blocksize and cluster-size greater than 32M
>>> * 2k blocksize and cluster-size greater than 64M
>>> * 4k blocksize and cluster-size greater than 128M mkfs fails with
>>> "Invalid argument passed to ext2 library while trying to create
>>> journal" error. In general, when the cluster-size to blocksize ratio
>>> is greater than 32k, mkfs fails with this error.
>>> 
>>> Went through the code and found out that the function
>>> `ext2fs_new_range()` is the source of this error. This is because when
>>> the cluster-size to blocksize ratio exceeds 32k, the length argument
>>> to the function `ext2fs_new_range()` results in 0. Hence, the error.
>>> 
>>> This patch corrects the valid cluster-size values.
>>> ---
>>> misc/mke2fs.8.in | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/misc/mke2fs.8.in b/misc/mke2fs.8.in index
>>> e6bfc6d6..b5b02144 100644
>>> --- a/misc/mke2fs.8.in
>>> +++ b/misc/mke2fs.8.in
>>> @@ -230,9 +230,9 @@ test is used instead of a fast read-only test.
>>> .TP
>>> .B \-C " cluster-size"
>>> Specify the size of cluster in bytes for filesystems using the
>>> bigalloc -feature.  Valid cluster-size values are from 2048 to 256M
>>> bytes per -cluster.  This can only be specified if the bigalloc
>>> feature is -enabled.  (See the
>>> +feature.  Valid cluster-size values are from 2048 to 128M bytes per
>>> +cluster based on filesystem blocksize. This can only be specified if
>>> +the bigalloc feature is enabled.  (See the
>>> .B ext4 (5)
>> 
>> 
>> This is an improvement, but doesn't really explain the details of the limits.
>> Instead of "based on filesystem blocksize." I think writing "between 2-32768
>> times the filesystem blocksize." or similar would be more clear and explain
>> how the actual limits relate to the blocksize.
> 
> Hi, Andreas. Thank you for the comment. Here are the details:

Thank you for this added information, but I guess my comment wasn't very clear.
*I* understand the background here.  My email was directed at your change to
the *man page*, where users who *do not* know the details go for information.

> The function ext2fs_new_range() is causing the error. This function gets
> called while creating the journal inode.
> 
> Failure cases:
> ------------------------------------------
> A   | B   | C   | D   | E   | F     | len
> ------------------------------------------
> 1k    64m   0     16    16    65535    0
> 1k   128m   0     17    17    131071   0
> 1k   256m   0     18    18    262143   0
> 2k   128m   1     17    16    65535    0
> 2k   256m   1     18    17    131071   0
> 4k   256m   2     18    16    65535    0
> 
> successful cases:
> 
> 1k   32m    0     15    15    32767   32768
> 2k   64m    1     16    15    32767   32768
> 4k   128m   2     17    15    32767   32768

Basically, what I was asking is that your patch for the man page be updated to
include just a fraction of this information, specifically that it includes that
"the cluster size must be between 2-32768 times the filesystem blocksize" and
not just "based on the filesystem blocksize".

Cheers, Andreas
diff mbox series

Patch

diff --git a/misc/mke2fs.8.in b/misc/mke2fs.8.in
index e6bfc6d6..b5b02144 100644
--- a/misc/mke2fs.8.in
+++ b/misc/mke2fs.8.in
@@ -230,9 +230,9 @@  test is used instead of a fast read-only test.
 .TP
 .B \-C " cluster-size"
 Specify the size of cluster in bytes for filesystems using the bigalloc
-feature.  Valid cluster-size values are from 2048 to 256M bytes per
-cluster.  This can only be specified if the bigalloc feature is
-enabled.  (See the
+feature.  Valid cluster-size values are from 2048 to 128M bytes per 
+cluster based on filesystem blocksize. This can only be specified if the
+bigalloc feature is enabled.  (See the
 .B ext4 (5)
 man page for more details about bigalloc.)   The default cluster size if
 bigalloc is enabled is 16 times the block size.