[PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Su Yue]

mkfs.ocfs2 is using 32bit journal as default.
For 16T size device support, '-J block64' should be used.

Signed-off-by: Su Yue <glass.su@suse.com>
---
 tests/generic/620 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/generic/620 b/tests/generic/620
index 3f1ce45a55fd..60e5a2cacdda 100755
--- a/tests/generic/620
+++ b/tests/generic/620
@@ -41,6 +41,9 @@ sectors=$((2*1024*1024*1024*17))
 chunk_size=128
 
 _dmhugedisk_init $sectors $chunk_size
+
+[ "$FSTYP" = "ocfs2"  ] && MKFS_OPTIONS="$MKFS_OPTIONS -J block64"
+
 _mkfs_dev $DMHUGEDISK_DEV
 _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
 testfile=$SCRATCH_MNT/testfile-$seq
-- 
2.47.1

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Christoph Hellwig]

On Mon, Jan 06, 2025 at 10:01:04PM +0800, Su Yue wrote:
> mkfs.ocfs2 is using 32bit journal as default.
> For 16T size device support, '-J block64' should be used.

Hard coding this into a specific test is weird.  I would expect
mkfs to pick the right one, but maybe the maintainers have a good
reason for not doing that?

But if not you probably want to force it in your config file or
we need a workaround in _try_mkfs_dev.

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Glass Su]

> On Jan 7, 2025, at 00:30, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Mon, Jan 06, 2025 at 10:01:04PM +0800, Su Yue wrote:
>> mkfs.ocfs2 is using 32bit journal as default.
>> For 16T size device support, '-J block64' should be used.
> 
> Hard coding this into a specific test is weird.  I would expect
> mkfs to pick the right one, but maybe the maintainers have a good
> reason for not doing that?
> 
Sounds more reasonable.  64bit journal support of ocfs2 was added in
2008, 17 gears agao...

— 
Su
> But if not you probably want to force it in your config file or
> we need a workaround in _try_mkfs_dev.
> 

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[David Disseldorp]

Hi,

On Mon,  6 Jan 2025 22:01:04 +0800, Su Yue wrote:

> mkfs.ocfs2 is using 32bit journal as default.
> For 16T size device support, '-J block64' should be used.
> 
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
>  tests/generic/620 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/generic/620 b/tests/generic/620
> index 3f1ce45a55fd..60e5a2cacdda 100755
> --- a/tests/generic/620
> +++ b/tests/generic/620
> @@ -41,6 +41,9 @@ sectors=$((2*1024*1024*1024*17))
>  chunk_size=128
>  
>  _dmhugedisk_init $sectors $chunk_size
> +
> +[ "$FSTYP" = "ocfs2"  ] && MKFS_OPTIONS="$MKFS_OPTIONS -J block64"

Makes sense, although as Christoph mentioned, a less test-specific
approach might be a better. E.g. _require_scratch_16T_support could skip
if block64 isn't configured, similar to ext4 (assuming tunefs.ocfs2
provides a way to query for it, which I don't see at first glance).

Cheers, David

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Glass Su]

> On Jan 7, 2025, at 00:39, David Disseldorp <ddiss@suse.de> wrote:
> 
> Hi,
> 
> On Mon,  6 Jan 2025 22:01:04 +0800, Su Yue wrote:
> 
>> mkfs.ocfs2 is using 32bit journal as default.
>> For 16T size device support, '-J block64' should be used.
>> 
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> tests/generic/620 | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/tests/generic/620 b/tests/generic/620
>> index 3f1ce45a55fd..60e5a2cacdda 100755
>> --- a/tests/generic/620
>> +++ b/tests/generic/620
>> @@ -41,6 +41,9 @@ sectors=$((2*1024*1024*1024*17))
>> chunk_size=128
>> 
>> _dmhugedisk_init $sectors $chunk_size
>> +
>> +[ "$FSTYP" = "ocfs2"  ] && MKFS_OPTIONS="$MKFS_OPTIONS -J block64"
> 
> Makes sense, although as Christoph mentioned, a less test-specific
> approach might be a better. E.g. _require_scratch_16T_support could skip

My brain is clearer after one night sleep. As Christoph said,  I think it’s problem of mkfs.ocfs2.
Let me drop this patch. Thanks for  your review.

— 
Su
> if block64 isn't configured, similar to ext4 (assuming tunefs.ocfs2
> provides a way to query for it, which I don't see at first glance).
> 
> Cheers, David

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Darrick J. Wong]

On Mon, Jan 06, 2025 at 10:01:04PM +0800, Su Yue wrote:
> mkfs.ocfs2 is using 32bit journal as default.
> For 16T size device support, '-J block64' should be used.
> 
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
>  tests/generic/620 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/generic/620 b/tests/generic/620
> index 3f1ce45a55fd..60e5a2cacdda 100755
> --- a/tests/generic/620
> +++ b/tests/generic/620
> @@ -41,6 +41,9 @@ sectors=$((2*1024*1024*1024*17))
>  chunk_size=128
>  
>  _dmhugedisk_init $sectors $chunk_size
> +
> +[ "$FSTYP" = "ocfs2"  ] && MKFS_OPTIONS="$MKFS_OPTIONS -J block64"

Won't mkfs.ocfs2 turn on block64 on a > 16T volume without prompting?

--D

> +
>  _mkfs_dev $DMHUGEDISK_DEV
>  _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
>  testfile=$SCRATCH_MNT/testfile-$seq
> -- 
> 2.47.1
> 
> 

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Glass Su]

> On Jan 7, 2025, at 10:12, Darrick J. Wong <djwong@kernel.org> wrote:
> 
> On Mon, Jan 06, 2025 at 10:01:04PM +0800, Su Yue wrote:
>> mkfs.ocfs2 is using 32bit journal as default.
>> For 16T size device support, '-J block64' should be used.
>> 
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> tests/generic/620 | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/tests/generic/620 b/tests/generic/620
>> index 3f1ce45a55fd..60e5a2cacdda 100755
>> --- a/tests/generic/620
>> +++ b/tests/generic/620
>> @@ -41,6 +41,9 @@ sectors=$((2*1024*1024*1024*17))
>> chunk_size=128
>> 
>> _dmhugedisk_init $sectors $chunk_size
>> +
>> +[ "$FSTYP" = "ocfs2"  ] && MKFS_OPTIONS="$MKFS_OPTIONS -J block64"
> 
> Won't mkfs.ocfs2 turn on block64 on a > 16T volume without prompting?
> 
It won’t. Only error will be printed:

ERROR: jbd can only store block numbers in 32 bits. /dev/mapper/huge-test.620 can hold 4563402752 blocks
which overflows this limit. If you have a new enough Ocfs2 with JBD2 support, you can try formatting with the "-Jblock64" option to turn on support for this size block device.

It’s 2025 now. Maybe it’s time to make mkfs.ocfs2  turn on block64 if device size > 16T.

Maintainers may know some stories.  Joseph?

— 
Su

> --D
> 
>> +
>> _mkfs_dev $DMHUGEDISK_DEV
>> _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
>> testfile=$SCRATCH_MNT/testfile-$seq
>> -- 
>> 2.47.1

Re: [PATCH] generic/620: add '-J block64' mkfs option for ocfs2

[Joseph Qi]

On 2025/1/7 10:42, Glass Su wrote:
> 
> 
>> On Jan 7, 2025, at 10:12, Darrick J. Wong <djwong@kernel.org> wrote:
>>
>> On Mon, Jan 06, 2025 at 10:01:04PM +0800, Su Yue wrote:
>>> mkfs.ocfs2 is using 32bit journal as default.
>>> For 16T size device support, '-J block64' should be used.
>>>
>>> Signed-off-by: Su Yue <glass.su@suse.com>
>>> ---
>>> tests/generic/620 | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tests/generic/620 b/tests/generic/620
>>> index 3f1ce45a55fd..60e5a2cacdda 100755
>>> --- a/tests/generic/620
>>> +++ b/tests/generic/620
>>> @@ -41,6 +41,9 @@ sectors=$((2*1024*1024*1024*17))
>>> chunk_size=128
>>>
>>> _dmhugedisk_init $sectors $chunk_size
>>> +
>>> +[ "$FSTYP" = "ocfs2"  ] && MKFS_OPTIONS="$MKFS_OPTIONS -J block64"
>>
>> Won't mkfs.ocfs2 turn on block64 on a > 16T volume without prompting?
>>
> It won’t. Only error will be printed:
> 
> ERROR: jbd can only store block numbers in 32 bits. /dev/mapper/huge-test.620 can hold 4563402752 blocks
> which overflows this limit. If you have a new enough Ocfs2 with JBD2 support, you can try formatting with the "-Jblock64" option to turn on support for this size block device.
> 
> It’s 2025 now. Maybe it’s time to make mkfs.ocfs2  turn on block64 if device size > 16T.
> 
> Maintainers may know some stories.  Joseph?
> 

AFAIK, standard 32-bit journal is the default journal format for ocfs2
since the beginning.