[PATCH] xfs/119: fix too small log size error

[PATCH] xfs/119: fix too small log size error

[Zorro Lang]

xfs/119 fails on 4k hard sector size device if reflink (with/without
rmapbt) is enabled:

  # mkfs.xfs -f -m reflink=1,rmapbt=0 -l version=2,size=2560b,su=64k /dev/sdf
  log size 2560 blocks too small, minimum size is 2688 blocks
  # mkfs.xfs -f -m reflink=1,rmapbt=1 -l version=2,size=2560b,su=64k /dev/sdf
  log size 2560 blocks too small, minimum size is 4368 blocks

So remove log size parameter and run mkfs.xfs again if it fails.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

I saw Dave changed the log size once in commit:
  d0a3cc5a xfs/104: log size too small for 4k sector drives

But it's still not enough after we have reflink feature now. I don't
know why this case need "-l version=2,size=2560b,su=64k",
if someone learns about this case, please tell me how to change this
case properly. If the log size parameter is not necessary, I'd like
to remove it.

Thanks,
Zorro

 tests/xfs/119 | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/xfs/119 b/tests/xfs/119
index bf7f1ca8..d507aacf 100755
--- a/tests/xfs/119
+++ b/tests/xfs/119
@@ -40,7 +40,14 @@ sync
 
 export MKFS_OPTIONS="-l version=2,size=2560b,su=64k" 
 export MOUNT_OPTIONS="-o logbsize=64k"
-_scratch_mkfs_xfs >/dev/null
+_scratch_mkfs_xfs >/dev/null 2>&1
+# The specified log size maybe too small if new features (e.g: reflink
+# or/and rmapbt) is enabled. So if mkfs fails, don't specify log size
+# and try to mkfs again.
+if [ $? -ne 0 ]; then
+	export MKFS_OPTIONS="-l version=2,su=64k"
+	_scratch_mkfs_xfs >/dev/null
+fi
 
 _scratch_mount
 
-- 
2.14.4

Re: [PATCH] xfs/119: fix too small log size error

[Eryu Guan]

On Wed, Sep 12, 2018 at 07:04:04PM +0800, Zorro Lang wrote:
> xfs/119 fails on 4k hard sector size device if reflink (with/without
> rmapbt) is enabled:
> 
>   # mkfs.xfs -f -m reflink=1,rmapbt=0 -l version=2,size=2560b,su=64k /dev/sdf
>   log size 2560 blocks too small, minimum size is 2688 blocks
>   # mkfs.xfs -f -m reflink=1,rmapbt=1 -l version=2,size=2560b,su=64k /dev/sdf
>   log size 2560 blocks too small, minimum size is 4368 blocks
> 
> So remove log size parameter and run mkfs.xfs again if it fails.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> I saw Dave changed the log size once in commit:
>   d0a3cc5a xfs/104: log size too small for 4k sector drives
> 
> But it's still not enough after we have reflink feature now. I don't
> know why this case need "-l version=2,size=2560b,su=64k",
> if someone learns about this case, please tell me how to change this
> case properly. If the log size parameter is not necessary, I'd like
> to remove it.

I guess we still need the log size param, as the test is testing
"freeze/thaws on a small log fs", please see commit 3ebdf78c16cb ("Test
out pv#942130 where the unmount rec is not ungranting log space. Merge
of master-melb:xfs-cmds:23759a by kenmcd."), but we don't have much
background information from that ancient commit log..

Thanks,
Eryu

> 
> Thanks,
> Zorro
> 
>  tests/xfs/119 | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/119 b/tests/xfs/119
> index bf7f1ca8..d507aacf 100755
> --- a/tests/xfs/119
> +++ b/tests/xfs/119
> @@ -40,7 +40,14 @@ sync
>  
>  export MKFS_OPTIONS="-l version=2,size=2560b,su=64k" 
>  export MOUNT_OPTIONS="-o logbsize=64k"
> -_scratch_mkfs_xfs >/dev/null
> +_scratch_mkfs_xfs >/dev/null 2>&1
> +# The specified log size maybe too small if new features (e.g: reflink
> +# or/and rmapbt) is enabled. So if mkfs fails, don't specify log size
> +# and try to mkfs again.
> +if [ $? -ne 0 ]; then
> +	export MKFS_OPTIONS="-l version=2,su=64k"
> +	_scratch_mkfs_xfs >/dev/null
> +fi
>  
>  _scratch_mount
>  
> -- 
> 2.14.4
> 

Re: [PATCH] xfs/119: fix too small log size error

[Zorro Lang]

On Wed, Sep 12, 2018 at 09:15:09PM +0800, Eryu Guan wrote:
> On Wed, Sep 12, 2018 at 07:04:04PM +0800, Zorro Lang wrote:
> > xfs/119 fails on 4k hard sector size device if reflink (with/without
> > rmapbt) is enabled:
> > 
> >   # mkfs.xfs -f -m reflink=1,rmapbt=0 -l version=2,size=2560b,su=64k /dev/sdf
> >   log size 2560 blocks too small, minimum size is 2688 blocks
> >   # mkfs.xfs -f -m reflink=1,rmapbt=1 -l version=2,size=2560b,su=64k /dev/sdf
> >   log size 2560 blocks too small, minimum size is 4368 blocks
> > 
> > So remove log size parameter and run mkfs.xfs again if it fails.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> > 
> > I saw Dave changed the log size once in commit:
> >   d0a3cc5a xfs/104: log size too small for 4k sector drives
> > 
> > But it's still not enough after we have reflink feature now. I don't
> > know why this case need "-l version=2,size=2560b,su=64k",
> > if someone learns about this case, please tell me how to change this
> > case properly. If the log size parameter is not necessary, I'd like
> > to remove it.
> 
> I guess we still need the log size param, as the test is testing
> "freeze/thaws on a small log fs", please see commit 3ebdf78c16cb ("Test
> out pv#942130 where the unmount rec is not ungranting log space. Merge
> of master-melb:xfs-cmds:23759a by kenmcd."), but we don't have much
> background information from that ancient commit log..

Yeah, I can't understand the pv#942130 meaning, and can't find it by Google.
Does anyone know the background, then tell me if it must use the minimum
log size?

And I find xfs_mkfs always trys to use the smallest log size which it can
use by using libxfs_log_calc_minimum_size() (but the real calculator is
more complicated)...

Thanks,
Zorro

> 
> Thanks,
> Eryu
> 
> > 
> > Thanks,
> > Zorro
> > 
> >  tests/xfs/119 | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/xfs/119 b/tests/xfs/119
> > index bf7f1ca8..d507aacf 100755
> > --- a/tests/xfs/119
> > +++ b/tests/xfs/119
> > @@ -40,7 +40,14 @@ sync
> >  
> >  export MKFS_OPTIONS="-l version=2,size=2560b,su=64k" 
> >  export MOUNT_OPTIONS="-o logbsize=64k"
> > -_scratch_mkfs_xfs >/dev/null
> > +_scratch_mkfs_xfs >/dev/null 2>&1
> > +# The specified log size maybe too small if new features (e.g: reflink
> > +# or/and rmapbt) is enabled. So if mkfs fails, don't specify log size
> > +# and try to mkfs again.
> > +if [ $? -ne 0 ]; then
> > +	export MKFS_OPTIONS="-l version=2,su=64k"
> > +	_scratch_mkfs_xfs >/dev/null
> > +fi
> >  
> >  _scratch_mount
> >  
> > -- 
> > 2.14.4
> > 

Re: [PATCH] xfs/119: fix too small log size error

[Dave Chinner]

On Wed, Sep 12, 2018 at 11:25:08PM +0800, Zorro Lang wrote:
> On Wed, Sep 12, 2018 at 09:15:09PM +0800, Eryu Guan wrote:
> > On Wed, Sep 12, 2018 at 07:04:04PM +0800, Zorro Lang wrote:
> > > xfs/119 fails on 4k hard sector size device if reflink (with/without
> > > rmapbt) is enabled:
> > > 
> > >   # mkfs.xfs -f -m reflink=1,rmapbt=0 -l version=2,size=2560b,su=64k /dev/sdf
> > >   log size 2560 blocks too small, minimum size is 2688 blocks
> > >   # mkfs.xfs -f -m reflink=1,rmapbt=1 -l version=2,size=2560b,su=64k /dev/sdf
> > >   log size 2560 blocks too small, minimum size is 4368 blocks
> > > 
> > > So remove log size parameter and run mkfs.xfs again if it fails.
> > > 
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > ---
> > > 
> > > I saw Dave changed the log size once in commit:
> > >   d0a3cc5a xfs/104: log size too small for 4k sector drives
> > > 
> > > But it's still not enough after we have reflink feature now. I don't
> > > know why this case need "-l version=2,size=2560b,su=64k",
> > > if someone learns about this case, please tell me how to change this
> > > case properly. If the log size parameter is not necessary, I'd like
> > > to remove it.
> > 
> > I guess we still need the log size param, as the test is testing
> > "freeze/thaws on a small log fs", please see commit 3ebdf78c16cb ("Test
> > out pv#942130 where the unmount rec is not ungranting log space. Merge
> > of master-melb:xfs-cmds:23759a by kenmcd."), but we don't have much
> > background information from that ancient commit log..
> 
> Yeah, I can't understand the pv#942130 meaning, and can't find it by Google.
> Does anyone know the background, then tell me if it must use the minimum
> log size?

s/pv/bz/

i.e. PV was the SGI internal bug tracking system. You'll never find
it in google.

The issue with that test was exercising was a log space leak caused
by unmount records not correctly reserving/releasing log space.
Freezzing the filesystem writes an unmount record, and hence when
filesystems were repeatedly frozen (e.g. for snapshots or backups)
they'd eventually leak enough log space to hang.

With a 10MB log, 100 iterations of the freeze/unfreeze loop leaked
enough log space that the test would hang. If you're going to
increase the log size, you need to increase the number of iterations
appropriately.

(The amount of almost entirely useless crap that has stuck inside my
head over the years never ceases to amaze me!)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com