| Message ID | 20250106140104.91880-1-glass.su@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | generic/620: add '-J block64' mkfs option for ocfs2 | expand |
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.
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
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 > >
> 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
> 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
> 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. >
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.
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
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(+)