This two case fail on my server machine with GPT disklabe type, info as below: mount failed: -o logbsize=32k dmesg info as below: [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> --- common/log | 1 + tests/generic/054 | 1 + tests/generic/055 | 1 + 3 files changed, 3 insertions(+) diff --git a/common/log b/common/log index c7921f50..f82919a6 100644 --- a/common/log +++ b/common/log @@ -494,6 +494,7 @@ _log_version() _require_v2log() { + [ "$FSTYP" != xfs ] && return 0 # test out mkfs to see if it supports "-l version=2" export MKFS_OPTIONS="-l version=2" if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then diff --git a/tests/generic/054 b/tests/generic/054 index 95d40c3c..b4b74615 100755 --- a/tests/generic/054 +++ b/tests/generic/054 @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* _require_scratch _require_scratch_shutdown _require_logstate +_require_v2log echo "*** init FS" _scratch_unmount >/dev/null 2>&1 diff --git a/tests/generic/055 b/tests/generic/055 index 9715f27b..071c4eaf 100755 --- a/tests/generic/055 +++ b/tests/generic/055 @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* _require_scratch _require_scratch_shutdown _require_logstate +_require_v2log QUOTA_OPTION=`_get_quota_option` -- 2.18.0
From xfs manpage, version 2 also supports 16k log buf size for mount option and case passed(only generic/054,055 used this api) on my machine. So delete 32k and 64k with different sunit to be consistented with ext4 test num(10) and we can test all logbuf size. Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> --- common/log | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/log b/common/log index f82919a6..98e0d638 100644 --- a/common/log +++ b/common/log @@ -547,10 +547,10 @@ _xfs_log_config() { echo "# mkfs-opt mount-opt" echo "# ------------------------------" + echo " version=2 logbsize=16k" + echo " version=2,su=16384 logbsize=16k" echo " version=2 logbsize=32k" - echo " version=2,su=4096 logbsize=32k" echo " version=2,su=32768 logbsize=32k" - echo " version=2,su=32768 logbsize=64k" echo " version=2 logbsize=64k" echo " version=2,su=64k logbsize=64k" echo " version=2 logbsize=128k" -- 2.18.0
On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote: > This two case fail on my server machine with GPT disklabe type, > info as below: > mount failed: -o logbsize=32k > dmesg info as below: > [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size > > The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. Works fine here. What is sda7's storage and raid alignment configuration? --D > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> > --- > common/log | 1 + > tests/generic/054 | 1 + > tests/generic/055 | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/common/log b/common/log > index c7921f50..f82919a6 100644 > --- a/common/log > +++ b/common/log > @@ -494,6 +494,7 @@ _log_version() > > _require_v2log() > { > + [ "$FSTYP" != xfs ] && return 0 > # test out mkfs to see if it supports "-l version=2" > export MKFS_OPTIONS="-l version=2" > if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then > diff --git a/tests/generic/054 b/tests/generic/054 > index 95d40c3c..b4b74615 100755 > --- a/tests/generic/054 > +++ b/tests/generic/054 > @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* > _require_scratch > _require_scratch_shutdown > _require_logstate > +_require_v2log > > echo "*** init FS" > _scratch_unmount >/dev/null 2>&1 > diff --git a/tests/generic/055 b/tests/generic/055 > index 9715f27b..071c4eaf 100755 > --- a/tests/generic/055 > +++ b/tests/generic/055 > @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* > _require_scratch > _require_scratch_shutdown > _require_logstate > +_require_v2log > > QUOTA_OPTION=`_get_quota_option` > > -- > 2.18.0 > > >
on 2020/02/19 13:06, Darrick J. Wong wrote: > On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote: >> This two case fail on my server machine with GPT disklabe type, >> info as below: >> mount failed: -o logbsize=32k >> dmesg info as below: >> [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size >> >> The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. > > Works fine here. What is sda7's storage and raid alignment > configuration? /dev/sda is a SAS disk, raid aligment is 256k. When I mkfs.xfs -f /dev/sda7(20G), its lsunit value is 64blks(block size is 4k). Best Regards Yang Xu > > --D > >> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> >> --- >> common/log | 1 + >> tests/generic/054 | 1 + >> tests/generic/055 | 1 + >> 3 files changed, 3 insertions(+) >> >> diff --git a/common/log b/common/log >> index c7921f50..f82919a6 100644 >> --- a/common/log >> +++ b/common/log >> @@ -494,6 +494,7 @@ _log_version() >> >> _require_v2log() >> { >> + [ "$FSTYP" != xfs ] && return 0 >> # test out mkfs to see if it supports "-l version=2" >> export MKFS_OPTIONS="-l version=2" >> if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then >> diff --git a/tests/generic/054 b/tests/generic/054 >> index 95d40c3c..b4b74615 100755 >> --- a/tests/generic/054 >> +++ b/tests/generic/054 >> @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* >> _require_scratch >> _require_scratch_shutdown >> _require_logstate >> +_require_v2log >> >> echo "*** init FS" >> _scratch_unmount >/dev/null 2>&1 >> diff --git a/tests/generic/055 b/tests/generic/055 >> index 9715f27b..071c4eaf 100755 >> --- a/tests/generic/055 >> +++ b/tests/generic/055 >> @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* >> _require_scratch >> _require_scratch_shutdown >> _require_logstate >> +_require_v2log >> >> QUOTA_OPTION=`_get_quota_option` >> >> -- >> 2.18.0 >> >> >> > >
On Wed, Feb 19, 2020 at 04:48:07PM +0800, Yang Xu wrote: > > > on 2020/02/19 13:06, Darrick J. Wong wrote: > > On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote: > > > This two case fail on my server machine with GPT disklabe type, > > > info as below: > > > mount failed: -o logbsize=32k > > > dmesg info as below: > > > [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size > > > > > > The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. > > > > Works fine here. What is sda7's storage and raid alignment > > configuration? > /dev/sda is a SAS disk, raid aligment is 256k. When I mkfs.xfs -f > /dev/sda7(20G), its lsunit value is 64blks(block size is 4k). _require_v2log probably ought to be smarter about that. But, uh, seeing as generic/05[45] don't currently call _require_v2log, how did logbsize=32k end up in the mount options to fail those two tests? --D > Best Regards > Yang Xu > > > > --D > > > > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> > > > --- > > > common/log | 1 + > > > tests/generic/054 | 1 + > > > tests/generic/055 | 1 + > > > 3 files changed, 3 insertions(+) > > > > > > diff --git a/common/log b/common/log > > > index c7921f50..f82919a6 100644 > > > --- a/common/log > > > +++ b/common/log > > > @@ -494,6 +494,7 @@ _log_version() > > > _require_v2log() > > > { > > > + [ "$FSTYP" != xfs ] && return 0 > > > # test out mkfs to see if it supports "-l version=2" > > > export MKFS_OPTIONS="-l version=2" > > > if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then > > > diff --git a/tests/generic/054 b/tests/generic/054 > > > index 95d40c3c..b4b74615 100755 > > > --- a/tests/generic/054 > > > +++ b/tests/generic/054 > > > @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* > > > _require_scratch > > > _require_scratch_shutdown > > > _require_logstate > > > +_require_v2log > > > echo "*** init FS" > > > _scratch_unmount >/dev/null 2>&1 > > > diff --git a/tests/generic/055 b/tests/generic/055 > > > index 9715f27b..071c4eaf 100755 > > > --- a/tests/generic/055 > > > +++ b/tests/generic/055 > > > @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* > > > _require_scratch > > > _require_scratch_shutdown > > > _require_logstate > > > +_require_v2log > > > QUOTA_OPTION=`_get_quota_option` > > > -- > > > 2.18.0 > > > > > > > > > > > > > > >
on 2020/02/20 6:11, Darrick J. Wong wrote: > On Wed, Feb 19, 2020 at 04:48:07PM +0800, Yang Xu wrote: >> >> >> on 2020/02/19 13:06, Darrick J. Wong wrote: >>> On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote: >>>> This two case fail on my server machine with GPT disklabe type, >>>> info as below: >>>> mount failed: -o logbsize=32k >>>> dmesg info as below: >>>> [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size >>>> >>>> The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. >>> >>> Works fine here. What is sda7's storage and raid alignment >>> configuration? >> /dev/sda is a SAS disk, raid aligment is 256k. When I mkfs.xfs -f >> /dev/sda7(20G), its lsunit value is 64blks(block size is 4k). > > _require_v2log probably ought to be smarter about that. maybe _require_v2log to specify logbszie for mount options? > > But, uh, seeing as generic/05[45] don't currently call _require_v2log, > how did logbsize=32k end up in the mount options to fail those two > tests? > But generic/05[45] call _get_log_configs. It will use _xfs_log_config and _xfs_log_config calls logbszie=32k/64k/128k/256k for mount options. So xfs/xfs_super.c xfs_finish_flag will report EINVAL error if logbsize < logsunit. So on my machine this case failed without calling _require_v2log. Best Regards Yang Xu > --D > >> Best Regards >> Yang Xu >>> >>> --D >>> >>>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> >>>> --- >>>> common/log | 1 + >>>> tests/generic/054 | 1 + >>>> tests/generic/055 | 1 + >>>> 3 files changed, 3 insertions(+) >>>> >>>> diff --git a/common/log b/common/log >>>> index c7921f50..f82919a6 100644 >>>> --- a/common/log >>>> +++ b/common/log >>>> @@ -494,6 +494,7 @@ _log_version() >>>> _require_v2log() >>>> { >>>> + [ "$FSTYP" != xfs ] && return 0 >>>> # test out mkfs to see if it supports "-l version=2" >>>> export MKFS_OPTIONS="-l version=2" >>>> if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then >>>> diff --git a/tests/generic/054 b/tests/generic/054 >>>> index 95d40c3c..b4b74615 100755 >>>> --- a/tests/generic/054 >>>> +++ b/tests/generic/054 >>>> @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* >>>> _require_scratch >>>> _require_scratch_shutdown >>>> _require_logstate >>>> +_require_v2log >>>> echo "*** init FS" >>>> _scratch_unmount >/dev/null 2>&1 >>>> diff --git a/tests/generic/055 b/tests/generic/055 >>>> index 9715f27b..071c4eaf 100755 >>>> --- a/tests/generic/055 >>>> +++ b/tests/generic/055 >>>> @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* >>>> _require_scratch >>>> _require_scratch_shutdown >>>> _require_logstate >>>> +_require_v2log >>>> QUOTA_OPTION=`_get_quota_option` >>>> -- >>>> 2.18.0 >>>> >>>> >>>> >>> >>> >> >> > >
On Thu, Feb 20, 2020 at 10:01:30AM +0800, Yang Xu wrote: > > > on 2020/02/20 6:11, Darrick J. Wong wrote: > > On Wed, Feb 19, 2020 at 04:48:07PM +0800, Yang Xu wrote: > > > > > > > > > on 2020/02/19 13:06, Darrick J. Wong wrote: > > > > On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote: > > > > > This two case fail on my server machine with GPT disklabe type, > > > > > info as below: > > > > > mount failed: -o logbsize=32k > > > > > dmesg info as below: > > > > > [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size > > > > > > > > > > The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. > > > > > > > > Works fine here. What is sda7's storage and raid alignment > > > > configuration? > > > /dev/sda is a SAS disk, raid aligment is 256k. When I mkfs.xfs -f > > > /dev/sda7(20G), its lsunit value is 64blks(block size is 4k). > > > > _require_v2log probably ought to be smarter about that. > maybe _require_v2log to specify logbszie for mount options? > > > > But, uh, seeing as generic/05[45] don't currently call _require_v2log, > > how did logbsize=32k end up in the mount options to fail those two > > tests? > > > But generic/05[45] call _get_log_configs. It will use _xfs_log_config > and _xfs_log_config calls logbszie=32k/64k/128k/256k for mount options. > So xfs/xfs_super.c xfs_finish_flag will report EINVAL error if logbsize < > logsunit. So on my machine this case failed without calling _require_v2log. logbsize=32k is a perfectly valid option on a fs with a v2 log, provided the log sunit isn't larger than 32k. _require_v2log conveniently _notruns the test when mounting with logbsize=32k fails, but maybe the problem here is that generic/05[45] need to format the fs with -l su=0 so that we can exercise full control over the mount options? --D > Best Regards > Yang Xu > > --D > > > > > Best Regards > > > Yang Xu > > > > > > > > --D > > > > > > > > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> > > > > > --- > > > > > common/log | 1 + > > > > > tests/generic/054 | 1 + > > > > > tests/generic/055 | 1 + > > > > > 3 files changed, 3 insertions(+) > > > > > > > > > > diff --git a/common/log b/common/log > > > > > index c7921f50..f82919a6 100644 > > > > > --- a/common/log > > > > > +++ b/common/log > > > > > @@ -494,6 +494,7 @@ _log_version() > > > > > _require_v2log() > > > > > { > > > > > + [ "$FSTYP" != xfs ] && return 0 > > > > > # test out mkfs to see if it supports "-l version=2" > > > > > export MKFS_OPTIONS="-l version=2" > > > > > if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then > > > > > diff --git a/tests/generic/054 b/tests/generic/054 > > > > > index 95d40c3c..b4b74615 100755 > > > > > --- a/tests/generic/054 > > > > > +++ b/tests/generic/054 > > > > > @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* > > > > > _require_scratch > > > > > _require_scratch_shutdown > > > > > _require_logstate > > > > > +_require_v2log > > > > > echo "*** init FS" > > > > > _scratch_unmount >/dev/null 2>&1 > > > > > diff --git a/tests/generic/055 b/tests/generic/055 > > > > > index 9715f27b..071c4eaf 100755 > > > > > --- a/tests/generic/055 > > > > > +++ b/tests/generic/055 > > > > > @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* > > > > > _require_scratch > > > > > _require_scratch_shutdown > > > > > _require_logstate > > > > > +_require_v2log > > > > > QUOTA_OPTION=`_get_quota_option` > > > > > -- > > > > > 2.18.0 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
on 2020/02/20 10:17, Darrick J. Wong wrote: > On Thu, Feb 20, 2020 at 10:01:30AM +0800, Yang Xu wrote: >> >> >> on 2020/02/20 6:11, Darrick J. Wong wrote: >>> On Wed, Feb 19, 2020 at 04:48:07PM +0800, Yang Xu wrote: >>>> >>>> >>>> on 2020/02/19 13:06, Darrick J. Wong wrote: >>>>> On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote: >>>>>> This two case fail on my server machine with GPT disklabe type, >>>>>> info as below: >>>>>> mount failed: -o logbsize=32k >>>>>> dmesg info as below: >>>>>> [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size >>>>>> >>>>>> The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check. >>>>> >>>>> Works fine here. What is sda7's storage and raid alignment >>>>> configuration? >>>> /dev/sda is a SAS disk, raid aligment is 256k. When I mkfs.xfs -f >>>> /dev/sda7(20G), its lsunit value is 64blks(block size is 4k). >>> >>> _require_v2log probably ought to be smarter about that. >> maybe _require_v2log to specify logbszie for mount options? >>> >>> But, uh, seeing as generic/05[45] don't currently call _require_v2log, >>> how did logbsize=32k end up in the mount options to fail those two >>> tests? >>> >> But generic/05[45] call _get_log_configs. It will use _xfs_log_config >> and _xfs_log_config calls logbszie=32k/64k/128k/256k for mount options. >> So xfs/xfs_super.c xfs_finish_flag will report EINVAL error if logbsize < >> logsunit. So on my machine this case failed without calling _require_v2log. > > logbsize=32k is a perfectly valid option on a fs with a v2 log, provided > the log sunit isn't larger than 32k. _require_v2log conveniently > _notruns the test when mounting with logbsize=32k fails, but > maybe the problem here is that generic/05[45] need to format the fs with > -l su=0 so that we can exercise full control over the mount options? Good point, this way is more wise. -l su=0 will report EINVAL error. I change xfs_test_config by specifying lsu to meet mount options as below: diff --git a/common/log b/common/log index c7921f50..9b5a2f6d 100644 --- a/common/log +++ b/common/log @@ -546,15 +546,15 @@ _xfs_log_config() { echo "# mkfs-opt mount-opt" echo "# ------------------------------" - echo " version=2 logbsize=32k" + echo " version=2,su=4096 logbsize=16k" + echo " version=2,su=16k logbsize=16k" echo " version=2,su=4096 logbsize=32k" - echo " version=2,su=32768 logbsize=32k" - echo " version=2,su=32768 logbsize=64k" - echo " version=2 logbsize=64k" + echo " version=2,su=32k logbsize=32k" + echo " version=2,su=4096 logbsize=64k" echo " version=2,su=64k logbsize=64k" - echo " version=2 logbsize=128k" + echo " version=2,su=4096 logbsize=128k" echo " version=2,su=128k logbsize=128k" - echo " version=2 logbsize=256k" + echo " version=2,su=4096 logbsize=256k" echo " version=2,su=256k logbsize=256k" } Best Regards Yang Xu > > --D > >> Best Regards >> Yang Xu >>> --D >>> >>>> Best Regards >>>> Yang Xu >>>>> >>>>> --D >>>>> >>>>>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> >>>>>> --- >>>>>> common/log | 1 + >>>>>> tests/generic/054 | 1 + >>>>>> tests/generic/055 | 1 + >>>>>> 3 files changed, 3 insertions(+) >>>>>> >>>>>> diff --git a/common/log b/common/log >>>>>> index c7921f50..f82919a6 100644 >>>>>> --- a/common/log >>>>>> +++ b/common/log >>>>>> @@ -494,6 +494,7 @@ _log_version() >>>>>> _require_v2log() >>>>>> { >>>>>> + [ "$FSTYP" != xfs ] && return 0 >>>>>> # test out mkfs to see if it supports "-l version=2" >>>>>> export MKFS_OPTIONS="-l version=2" >>>>>> if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then >>>>>> diff --git a/tests/generic/054 b/tests/generic/054 >>>>>> index 95d40c3c..b4b74615 100755 >>>>>> --- a/tests/generic/054 >>>>>> +++ b/tests/generic/054 >>>>>> @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.* >>>>>> _require_scratch >>>>>> _require_scratch_shutdown >>>>>> _require_logstate >>>>>> +_require_v2log >>>>>> echo "*** init FS" >>>>>> _scratch_unmount >/dev/null 2>&1 >>>>>> diff --git a/tests/generic/055 b/tests/generic/055 >>>>>> index 9715f27b..071c4eaf 100755 >>>>>> --- a/tests/generic/055 >>>>>> +++ b/tests/generic/055 >>>>>> @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.* >>>>>> _require_scratch >>>>>> _require_scratch_shutdown >>>>>> _require_logstate >>>>>> +_require_v2log >>>>>> QUOTA_OPTION=`_get_quota_option` >>>>>> -- >>>>>> 2.18.0 >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>> >>> >> >> > >
Currently, if we don't specify -l sunit or -l su option, mkfs.xfs will get the stripe size from underlying device. It works file on most situations. But on some machine, the size of underlying device greater than logbsize of mount options, it will report error like "logbuf size must be greater than or equal to log stripe size". We can specify -l su=4096 to meet this requirement and case can still run normally. Also, from xfs manpage, version 2 also supports 16k log buf size for mount option and case passed(only generic/054,055 used this api) on my machine. So delete 32k and 64k with different sunit to be consistented with ext4 test num(10) and we can test all logbuf size. Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> --- common/log | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/log b/common/log index c7921f50..9b5a2f6d 100644 --- a/common/log +++ b/common/log @@ -546,15 +546,15 @@ _xfs_log_config() { echo "# mkfs-opt mount-opt" echo "# ------------------------------" - echo " version=2 logbsize=32k" + echo " version=2,su=4096 logbsize=16k" + echo " version=2,su=16k logbsize=16k" echo " version=2,su=4096 logbsize=32k" - echo " version=2,su=32768 logbsize=32k" - echo " version=2,su=32768 logbsize=64k" - echo " version=2 logbsize=64k" + echo " version=2,su=32k logbsize=32k" + echo " version=2,su=4096 logbsize=64k" echo " version=2,su=64k logbsize=64k" - echo " version=2 logbsize=128k" + echo " version=2,su=4096 logbsize=128k" echo " version=2,su=128k logbsize=128k" - echo " version=2 logbsize=256k" + echo " version=2,su=4096 logbsize=256k" echo " version=2,su=256k logbsize=256k" } -- 2.18.0
Hi Darrick, On Thu, Feb 20, 2020 at 01:38:01PM +0800, Yang Xu wrote: > Currently, if we don't specify -l sunit or -l su option, mkfs.xfs > will get the stripe size from underlying device. > > It works file on most situations. But on some machine, the size of > underlying device greater than logbsize of mount options, it will > report error like "logbuf size must be greater than or equal to log > stripe size". We can specify -l su=4096 to meet this requirement and > case can still run normally. > > Also, from xfs manpage, version 2 also supports 16k log buf size for > mount option and case passed(only generic/054,055 used this api) on > my machine. So delete 32k and 64k with different sunit to be consistented > with ext4 test num(10) and we can test all logbuf size. > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> Would you like to review this v2 patch again? I feel more confident if xfs maintainer could ack it :) Thanks, Eryu > --- > common/log | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/common/log b/common/log > index c7921f50..9b5a2f6d 100644 > --- a/common/log > +++ b/common/log > @@ -546,15 +546,15 @@ _xfs_log_config() > { > echo "# mkfs-opt mount-opt" > echo "# ------------------------------" > - echo " version=2 logbsize=32k" > + echo " version=2,su=4096 logbsize=16k" > + echo " version=2,su=16k logbsize=16k" > echo " version=2,su=4096 logbsize=32k" > - echo " version=2,su=32768 logbsize=32k" > - echo " version=2,su=32768 logbsize=64k" > - echo " version=2 logbsize=64k" > + echo " version=2,su=32k logbsize=32k" > + echo " version=2,su=4096 logbsize=64k" > echo " version=2,su=64k logbsize=64k" > - echo " version=2 logbsize=128k" > + echo " version=2,su=4096 logbsize=128k" > echo " version=2,su=128k logbsize=128k" > - echo " version=2 logbsize=256k" > + echo " version=2,su=4096 logbsize=256k" > echo " version=2,su=256k logbsize=256k" > } > > -- > 2.18.0 > > >
On Sun, Feb 23, 2020 at 10:22:10PM +0800, Eryu Guan wrote: > Hi Darrick, > > On Thu, Feb 20, 2020 at 01:38:01PM +0800, Yang Xu wrote: > > Currently, if we don't specify -l sunit or -l su option, mkfs.xfs > > will get the stripe size from underlying device. > > > > It works file on most situations. But on some machine, the size of > > underlying device greater than logbsize of mount options, it will > > report error like "logbuf size must be greater than or equal to log > > stripe size". We can specify -l su=4096 to meet this requirement and > > case can still run normally. > > > > Also, from xfs manpage, version 2 also supports 16k log buf size for > > mount option and case passed(only generic/054,055 used this api) on > > my machine. So delete 32k and 64k with different sunit to be consistented I don't understand why there's a need to be consistent, which means I don't understand why we'd "delete 32k and 64k with different sunit". ext4 tests should not be invoking _xfs_log_config() > > with ext4 test num(10) and we can test all logbuf size. Hm? ext4/010 is an inode bitmap fuzz test. > > > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> > > Would you like to review this v2 patch again? I feel more confident if > xfs maintainer could ack it :) > > Thanks, > Eryu > > > --- > > common/log | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/common/log b/common/log > > index c7921f50..9b5a2f6d 100644 > > --- a/common/log > > +++ b/common/log > > @@ -546,15 +546,15 @@ _xfs_log_config() > > { > > echo "# mkfs-opt mount-opt" > > echo "# ------------------------------" > > - echo " version=2 logbsize=32k" > > + echo " version=2,su=4096 logbsize=16k" The straight substitutions look fine to me, but then... > > + echo " version=2,su=16k logbsize=16k" ...I can't tell why we're adding this extra case here, or why this has to be here and not in a separate patch justifying the addition? > > echo " version=2,su=4096 logbsize=32k" > > - echo " version=2,su=32768 logbsize=32k" > > - echo " version=2,su=32768 logbsize=64k" ...I also don't think it's a good idea to reduce test coverage, and definitely not buried in something that sounds like a fix patch. --D > > - echo " version=2 logbsize=64k" > > + echo " version=2,su=32k logbsize=32k" > > + echo " version=2,su=4096 logbsize=64k" > > echo " version=2,su=64k logbsize=64k" > > - echo " version=2 logbsize=128k" > > + echo " version=2,su=4096 logbsize=128k" > > echo " version=2,su=128k logbsize=128k" > > - echo " version=2 logbsize=256k" > > + echo " version=2,su=4096 logbsize=256k" > > echo " version=2,su=256k logbsize=256k" > > } > > > > -- > > 2.18.0 > > > > > >
on 2020/02/24 0:37, Darrick J. Wong wrote: > On Sun, Feb 23, 2020 at 10:22:10PM +0800, Eryu Guan wrote: >> Hi Darrick, >> >> On Thu, Feb 20, 2020 at 01:38:01PM +0800, Yang Xu wrote: >>> Currently, if we don't specify -l sunit or -l su option, mkfs.xfs >>> will get the stripe size from underlying device. >>> >>> It works file on most situations. But on some machine, the size of >>> underlying device greater than logbsize of mount options, it will >>> report error like "logbuf size must be greater than or equal to log >>> stripe size". We can specify -l su=4096 to meet this requirement and >>> case can still run normally. >>> >>> Also, from xfs manpage, version 2 also supports 16k log buf size for >>> mount option and case passed(only generic/054,055 used this api) on >>> my machine. So delete 32k and 64k with different sunit to be consistented > > I don't understand why there's a need to be consistent, which means I > don't understand why we'd "delete 32k and 64k with different sunit". > ext4 tests should not be invoking _xfs_log_config() Sorry for the confused message. _get_log_configs{ xfs) _xfs_log_config ;; f2fs) _f2fs_log_config ;; ext4) _ext4_log_config } xfs ,f2fs, and ext4 all have test1~test10. So for a generic case, it should be consistent. And only generic/054 055 uses _get_log_configs api and no test case used _xfs_log_config api. I think we should keep this(test1~test10, 054.out only 10 times mkfs and mount output) > >>> with ext4 test num(10) and we can test all logbuf size. > > Hm? ext4/010 is an inode bitmap fuzz test. > No. I mean _ext4_log_config has test1~test10. Best Regards Yang Xu >>> >>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> >> >> Would you like to review this v2 patch again? I feel more confident if >> xfs maintainer could ack it :) >> >> Thanks, >> Eryu >> >>> --- >>> common/log | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/common/log b/common/log >>> index c7921f50..9b5a2f6d 100644 >>> --- a/common/log >>> +++ b/common/log >>> @@ -546,15 +546,15 @@ _xfs_log_config() >>> { >>> echo "# mkfs-opt mount-opt" >>> echo "# ------------------------------" >>> - echo " version=2 logbsize=32k" >>> + echo " version=2,su=4096 logbsize=16k" > > The straight substitutions look fine to me, but then... > >>> + echo " version=2,su=16k logbsize=16k" > > ...I can't tell why we're adding this extra case here, or why this > has to be here and not in a separate patch justifying the addition? > >>> echo " version=2,su=4096 logbsize=32k" >>> - echo " version=2,su=32768 logbsize=32k" >>> - echo " version=2,su=32768 logbsize=64k" > > ...I also don't think it's a good idea to reduce test coverage, and > definitely not buried in something that sounds like a fix patch. > > --D > >>> - echo " version=2 logbsize=64k" >>> + echo " version=2,su=32k logbsize=32k" >>> + echo " version=2,su=4096 logbsize=64k" >>> echo " version=2,su=64k logbsize=64k" >>> - echo " version=2 logbsize=128k" >>> + echo " version=2,su=4096 logbsize=128k" >>> echo " version=2,su=128k logbsize=128k" >>> - echo " version=2 logbsize=256k" >>> + echo " version=2,su=4096 logbsize=256k" >>> echo " version=2,su=256k logbsize=256k" >>> } >>> >>> -- >>> 2.18.0 >>> >>> >>> > >
on 2020/02/24 9:51, Yang Xu wrote: > > > on 2020/02/24 0:37, Darrick J. Wong wrote: >> On Sun, Feb 23, 2020 at 10:22:10PM +0800, Eryu Guan wrote: >>> Hi Darrick, >>> >>> On Thu, Feb 20, 2020 at 01:38:01PM +0800, Yang Xu wrote: >>>> Currently, if we don't specify -l sunit or -l su option, mkfs.xfs >>>> will get the stripe size from underlying device. >>>> >>>> It works file on most situations. But on some machine, the size of >>>> underlying device greater than logbsize of mount options, it will >>>> report error like "logbuf size must be greater than or equal to log >>>> stripe size". We can specify -l su=4096 to meet this requirement and >>>> case can still run normally. >>>> >>>> Also, from xfs manpage, version 2 also supports 16k log buf size for >>>> mount option and case passed(only generic/054,055 used this api) on >>>> my machine. So delete 32k and 64k with different sunit to be >>>> consistented >> >> I don't understand why there's a need to be consistent, which means I >> don't understand why we'd "delete 32k and 64k with different sunit". >> ext4 tests should not be invoking _xfs_log_config() > > Sorry for the confused message. > _get_log_configs{ > xfs) > _xfs_log_config > ;; > f2fs) > _f2fs_log_config > ;; > ext4) > _ext4_log_config > > } > xfs ,f2fs, and ext4 all have test1~test10. > So for a generic case, it should be consistent. > And only generic/054 055 uses _get_log_configs api and no test case > used _xfs_log_config api. I think we should keep this(test1~test10, > 054.out only 10 times mkfs and mount output) >> >>>> with ext4 test num(10) and we can test all logbuf size. >> >> Hm? ext4/010 is an inode bitmap fuzz test. >> > No. I mean _ext4_log_config has test1~test10. > > Best Regards > Yang Xu >>>> >>>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com> >>> >>> Would you like to review this v2 patch again? I feel more confident if >>> xfs maintainer could ack it :) >>> >>> Thanks, >>> Eryu >>> >>>> --- >>>> common/log | 12 ++++++------ >>>> 1 file changed, 6 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/common/log b/common/log >>>> index c7921f50..9b5a2f6d 100644 >>>> --- a/common/log >>>> +++ b/common/log >>>> @@ -546,15 +546,15 @@ _xfs_log_config() >>>> { >>>> echo "# mkfs-opt mount-opt" >>>> echo "# ------------------------------" >>>> - echo " version=2 logbsize=32k" >>>> + echo " version=2,su=4096 logbsize=16k" >> >> The straight substitutions look fine to me, but then... >> >>>> + echo " version=2,su=16k logbsize=16k" >> >> ...I can't tell why we're adding this extra case here, or why this >> has to be here and not in a separate patch justifying the addition? For 16k logbsize, I don't know much about it. it may make no sense? So at the beginning, xfstests doesn't plan to test it. If so, I will remove this adding. >> >>>> echo " version=2,su=4096 logbsize=32k" >>>> - echo " version=2,su=32768 logbsize=32k" >>>> - echo " version=2,su=32768 logbsize=64k" >> >> ...I also don't think it's a good idea to reduce test coverage, and >> definitely not buried in something that sounds like a fix patch. OK. how about this change , as below(only specifying su=4096 for old options without su, logbusize=32K has su=4096, so add su=16k for it): diff --git a/common/log b/common/log index c7921f50..991c8e8f 100644 --- a/common/log +++ b/common/log @@ -546,15 +546,15 @@ _xfs_log_config() { echo "# mkfs-opt mount-opt" echo "# ------------------------------" - echo " version=2 logbsize=32k" echo " version=2,su=4096 logbsize=32k" + echo " version=2,su=16384 logbsize=32k" echo " version=2,su=32768 logbsize=32k" - echo " version=2,su=32768 logbsize=64k" - echo " version=2 logbsize=64k" + echo " version=2,su=4096 logbsize=64k" + echo " version=2,su=32768 logbsize=64k" echo " version=2,su=64k logbsize=64k" - echo " version=2 logbsize=128k" + echo " version=2,su=4096 logbsize=128k" echo " version=2,su=128k logbsize=128k" - echo " version=2 logbsize=256k" + echo " version=2,su=4096 logbsize=256k" echo " version=2,su=256k logbsize=256k" Best Regards Yang Xu >> >> --D >> >>>> - echo " version=2 logbsize=64k" >>>> + echo " version=2,su=32k logbsize=32k" >>>> + echo " version=2,su=4096 logbsize=64k" >>>> echo " version=2,su=64k logbsize=64k" >>>> - echo " version=2 logbsize=128k" >>>> + echo " version=2,su=4096 logbsize=128k" >>>> echo " version=2,su=128k logbsize=128k" >>>> - echo " version=2 logbsize=256k" >>>> + echo " version=2,su=4096 logbsize=256k" >>>> echo " version=2,su=256k logbsize=256k" >>>> } >>>> -- >>>> 2.18.0 >>>> >>>> >>>> >> >> > >