2024-03-14 09:32:40

by Srivathsa Dara

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

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)
man page for more details about bigalloc.) The default cluster size if
bigalloc is enabled is 16 times the block size.
--
2.31.1



2024-03-15 21:32:26

by Andreas Dilger

[permalink] [raw]
Subject: Re: [RESEND PATCH] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values

On Mar 14, 2024, at 3:31 AM, Srivathsa Dara <[email protected]> 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






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP

2024-03-19 11:25:36

by Srivathsa Dara

[permalink] [raw]
Subject: RE: [External] : Re: [RESEND PATCH] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values

>
>
> -----Original Message-----
> From: Andreas Dilger <[email protected]>
> Sent: Saturday, March 16, 2024 3:02 AM
> To: Srivathsa Dara <[email protected]>
> Cc: Ext4 Developers List <[email protected]>;
> Theodore Ts'o <[email protected]>; Rajesh Sivaramasubramaniom
> <[email protected]>;
> Junxiao Bi <[email protected]>
> Re: [RESEND PATCH] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values
>
> On Mar 14, 2024, at 3:31 AM, Srivathsa Dara <[email protected]> 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

2024-03-22 23:09:38

by Andreas Dilger

[permalink] [raw]
Subject: Re: [External] : Re: [RESEND PATCH] e2fsprogs: misc/mke2fs.8.in: Correct valid cluster-size values


> On Mar 19, 2024, at 5:25 AM, Srivathsa Dara <[email protected]> wrote:
>
> On March 16, 2024 3:02 AM, Andreas Dilger <[email protected]> wrote:
>>
>> On Mar 14, 2024, at 3:31 AM, Srivathsa Dara <[email protected]> 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






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP