All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] misc large filesystem fixes
@ 2018-08-29 21:43 Eric Sandeen
  2018-08-30  3:19 ` Eryu Guan
  2018-09-25 19:12 ` [PATCH V2] " Eric Sandeen
  0 siblings, 2 replies; 8+ messages in thread
From: Eric Sandeen @ 2018-08-29 21:43 UTC (permalink / raw)
  To: fstests

There are a few tests which fail on large filesytems because
we run into mkfs limits.

xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
than 2T this will fail.  Check the device size and restrict it
to just under 2T so that a 2-AG mkfs is possible.

xfs/178 tries to decrease the agcount and re-mkfs, but if the
default AG size was chosen to be 1T, decreasing the AG count
results in too-large AGs and mkfs fails.  The intention here
AFAICT is to simply re-mkfs with non-overlapping AG headers,
so increasing the AG count should achieve the same purpose,
and cause mkfs to choose a smaller-than-default AG size which
should pass.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/tests/xfs/010 b/tests/xfs/010
index ee1595c8..2b30c867 100755
--- a/tests/xfs/010
+++ b/tests/xfs/010
@@ -96,7 +96,14 @@ _require_xfs_finobt
 
 rm -f $seqres.full
 
-_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
+# If 2 AGs would result in too-large AG size, restrict the size
+DSIZEOPT=""
+DEV_SZ=$(blockdev --getsize64 $SCRATCH_DEV)
+if [ "$DEV_SZ" -ge "$((2*(2**40)))" ]; then
+  DSIZEOPT="-d size=2047g"
+fi
+
+_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2 $DSIZEOPT" | _filter_mkfs 2>$seqres.full
 
 # sparsely populate the fs such that we create records with free inodes
 _scratch_mount
diff --git a/tests/xfs/013 b/tests/xfs/013
index 4d31d793..ee752de1 100755
--- a/tests/xfs/013
+++ b/tests/xfs/013
@@ -97,7 +97,14 @@ _require_command "$KILLALL_PROG" killall
 
 rm -f $seqres.full
 
-_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | \
+# If 2 AGs would result in too-large AG size, restrict the size
+DSIZEOPT=""
+DEV_SZ=$(blockdev --getsize64 $SCRATCH_DEV)
+if [ "$DEV_SZ" -ge "$((2*(2**40)))" ]; then
+  DSIZEOPT="-d size=2047g"
+fi
+
+_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2 $DSIZEOPT" | \
 	_filter_mkfs 2>> $seqres.full
 _scratch_mount
 
diff --git a/tests/xfs/178 b/tests/xfs/178
index 84151056..f7ea9139 100755
--- a/tests/xfs/178
+++ b/tests/xfs/178
@@ -51,8 +51,8 @@ _supported_os Linux
 # o Summary of testing:
 #    1. mkfs.xfs a default filesystem, note agcount value.
 #    2. dd zero first sector and repair and verify.
-#    3. mkfs.xfs overriding agcount to a smaller value
-#             (ie. each AG is bigger)
+#    3. mkfs.xfs overriding agcount to a larger value
+#             (ie. each AG is smaller)
 #    4. dd zero first sector, repair and verify.
 #          -> old mkfs.xfs will cause repair to incorrectly
 #             fix filesystem, new mkfs.xfs will be fine.
@@ -74,8 +74,8 @@ fi
 
 _dd_repair_check $SCRATCH_DEV $sectsz
 
-# smaller AGCOUNT
-let "agcount=$agcount-2"
+# larger AGCOUNT
+let "agcount=$agcount+2"
 _scratch_mkfs_xfs -dagcount=$agcount >/dev/null 2>&1 \
         || _fail "mkfs failed!"
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] misc large filesystem fixes
  2018-08-29 21:43 [PATCH] misc large filesystem fixes Eric Sandeen
@ 2018-08-30  3:19 ` Eryu Guan
  2018-08-30  3:30   ` Eric Sandeen
  2018-09-25 19:12 ` [PATCH V2] " Eric Sandeen
  1 sibling, 1 reply; 8+ messages in thread
From: Eryu Guan @ 2018-08-30  3:19 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: fstests

On Wed, Aug 29, 2018 at 04:43:38PM -0500, Eric Sandeen wrote:
> There are a few tests which fail on large filesytems because
> we run into mkfs limits.
> 
> xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
> than 2T this will fail.  Check the device size and restrict it
> to just under 2T so that a 2-AG mkfs is possible.
> 
> xfs/178 tries to decrease the agcount and re-mkfs, but if the
> default AG size was chosen to be 1T, decreasing the AG count
> results in too-large AGs and mkfs fails.  The intention here
> AFAICT is to simply re-mkfs with non-overlapping AG headers,
> so increasing the AG count should achieve the same purpose,
> and cause mkfs to choose a smaller-than-default AG size which
> should pass.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/tests/xfs/010 b/tests/xfs/010
> index ee1595c8..2b30c867 100755
> --- a/tests/xfs/010
> +++ b/tests/xfs/010
> @@ -96,7 +96,14 @@ _require_xfs_finobt
>  
>  rm -f $seqres.full
>  
> -_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
> +# If 2 AGs would result in too-large AG size, restrict the size
> +DSIZEOPT=""
> +DEV_SZ=$(blockdev --getsize64 $SCRATCH_DEV)
> +if [ "$DEV_SZ" -ge "$((2*(2**40)))" ]; then
> +  DSIZEOPT="-d size=2047g"
> +fi
> +
> +_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2 $DSIZEOPT" | _filter_mkfs 2>$seqres.full

I think this kind of problem will happen to all tests that specify a
custom "agcount" in mkfs but without an "agsize" nor "size". For
example, xfs/062 would fail if $SCRATCH_DEV is larger than 8T. I guess
we need a more general way to fix the problem.

I'm thinking to introduce a new helper to require a given agcount will
fit the device size, and if the device is bigger than ($agcount * 1T)
then _notrun the test, something like (may need a better helper name):

_require_xfs_support_agcount()
{
	local dev=$1
	local agcount=$2
	local max_sz=$((agcount*(2**40)))
	local dev_sz=$(blockdev --getsize64 $dev)

	if [ $dev_sz -gt $max_sz ]; then
		_notrun "agcount $agcount is too small to hold $dev_sz device"
	fi
}

Then add this _require rule to all tests that only specify a custom
agsize, e.g. for xfs/010 we could do

_require_xfs_support_agcount $SCRATCH_DEV 2

I roughly went through all xfs tests and found that the following ones
may need this new _require rule

xfs/010
xfs/013
xfs/062
xfs/178
xfs/179
xfs/310

Perhaps there're better ways to solve the problem, any suggestions are
welcomed! 

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] misc large filesystem fixes
  2018-08-30  3:19 ` Eryu Guan
@ 2018-08-30  3:30   ` Eric Sandeen
  2018-08-30  4:29     ` Eryu Guan
  2018-08-30  4:34     ` Dave Chinner
  0 siblings, 2 replies; 8+ messages in thread
From: Eric Sandeen @ 2018-08-30  3:30 UTC (permalink / raw)
  To: Eryu Guan, Eric Sandeen; +Cc: fstests

On 8/29/18 10:19 PM, Eryu Guan wrote:
> On Wed, Aug 29, 2018 at 04:43:38PM -0500, Eric Sandeen wrote:
>> There are a few tests which fail on large filesytems because
>> we run into mkfs limits.
>>
>> xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
>> than 2T this will fail.  Check the device size and restrict it
>> to just under 2T so that a 2-AG mkfs is possible.
>>
>> xfs/178 tries to decrease the agcount and re-mkfs, but if the
>> default AG size was chosen to be 1T, decreasing the AG count
>> results in too-large AGs and mkfs fails.  The intention here
>> AFAICT is to simply re-mkfs with non-overlapping AG headers,
>> so increasing the AG count should achieve the same purpose,
>> and cause mkfs to choose a smaller-than-default AG size which
>> should pass.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/tests/xfs/010 b/tests/xfs/010
>> index ee1595c8..2b30c867 100755
>> --- a/tests/xfs/010
>> +++ b/tests/xfs/010
>> @@ -96,7 +96,14 @@ _require_xfs_finobt
>>  
>>  rm -f $seqres.full
>>  
>> -_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
>> +# If 2 AGs would result in too-large AG size, restrict the size
>> +DSIZEOPT=""
>> +DEV_SZ=$(blockdev --getsize64 $SCRATCH_DEV)
>> +if [ "$DEV_SZ" -ge "$((2*(2**40)))" ]; then
>> +  DSIZEOPT="-d size=2047g"
>> +fi
>> +
>> +_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2 $DSIZEOPT" | _filter_mkfs 2>$seqres.full
> 
> I think this kind of problem will happen to all tests that specify a
> custom "agcount" in mkfs but without an "agsize" nor "size". For
> example, xfs/062 would fail if $SCRATCH_DEV is larger than 8T. I guess
> we need a more general way to fix the problem.

Whoops, yes, I missed xfs/062
 
> I'm thinking to introduce a new helper to require a given agcount will
> fit the device size, and if the device is bigger than ($agcount * 1T)
> then _notrun the test, something like (may need a better helper name):
> 
> _require_xfs_support_agcount()
> {
> 	local dev=$1
> 	local agcount=$2
> 	local max_sz=$((agcount*(2**40)))
> 	local dev_sz=$(blockdev --getsize64 $dev)
> 
> 	if [ $dev_sz -gt $max_sz ]; then
> 		_notrun "agcount $agcount is too small to hold $dev_sz device"
> 	fi
> }

I'm not sure we should _notrun, though - if what we really want to check
is a 2-AG filesystem on the scratch dev, there are times when we may simply
want to make a smaller filesystem and proceed.  For the cases I sent
in my patch this should be perfectly fine...

> Then add this _require rule to all tests that only specify a custom
> agsize, e.g. for xfs/010 we could do
> 
> _require_xfs_support_agcount $SCRATCH_DEV 2
> 
> I roughly went through all xfs tests and found that the following ones
> may need this new _require rule
> 
> xfs/010
> xfs/013
> xfs/062
> xfs/178
> xfs/179
> xfs/310
> 
> Perhaps there're better ways to solve the problem, any suggestions are
> welcomed! 

Rather than a _require and a _notrun, what about a mkfs_scratch_agcount()
that does something like

mkfs_scratch_agcount()
{
 agcount=$1
 opts=$2

 # If $agcount AGs would result in too-large AG size, restrict the size
 # to create $agcount roughly 1T AGs.
 dsizeopt=""
 dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)
 if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
   dsizeopt="-d size=$(($agcount*((2**40)-1)))"
 fi
 _scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
}

or something like that?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] misc large filesystem fixes
  2018-08-30  3:30   ` Eric Sandeen
@ 2018-08-30  4:29     ` Eryu Guan
  2018-08-30  4:34     ` Dave Chinner
  1 sibling, 0 replies; 8+ messages in thread
From: Eryu Guan @ 2018-08-30  4:29 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, fstests

On Wed, Aug 29, 2018 at 10:30:29PM -0500, Eric Sandeen wrote:
> On 8/29/18 10:19 PM, Eryu Guan wrote:
> > On Wed, Aug 29, 2018 at 04:43:38PM -0500, Eric Sandeen wrote:
> >> There are a few tests which fail on large filesytems because
> >> we run into mkfs limits.
> >>
> >> xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
> >> than 2T this will fail.  Check the device size and restrict it
> >> to just under 2T so that a 2-AG mkfs is possible.
> >>
> >> xfs/178 tries to decrease the agcount and re-mkfs, but if the
> >> default AG size was chosen to be 1T, decreasing the AG count
> >> results in too-large AGs and mkfs fails.  The intention here
> >> AFAICT is to simply re-mkfs with non-overlapping AG headers,
> >> so increasing the AG count should achieve the same purpose,
> >> and cause mkfs to choose a smaller-than-default AG size which
> >> should pass.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >>
> >> diff --git a/tests/xfs/010 b/tests/xfs/010
> >> index ee1595c8..2b30c867 100755
> >> --- a/tests/xfs/010
> >> +++ b/tests/xfs/010
> >> @@ -96,7 +96,14 @@ _require_xfs_finobt
> >>  
> >>  rm -f $seqres.full
> >>  
> >> -_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
> >> +# If 2 AGs would result in too-large AG size, restrict the size
> >> +DSIZEOPT=""
> >> +DEV_SZ=$(blockdev --getsize64 $SCRATCH_DEV)
> >> +if [ "$DEV_SZ" -ge "$((2*(2**40)))" ]; then
> >> +  DSIZEOPT="-d size=2047g"
> >> +fi
> >> +
> >> +_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2 $DSIZEOPT" | _filter_mkfs 2>$seqres.full
> > 
> > I think this kind of problem will happen to all tests that specify a
> > custom "agcount" in mkfs but without an "agsize" nor "size". For
> > example, xfs/062 would fail if $SCRATCH_DEV is larger than 8T. I guess
> > we need a more general way to fix the problem.
> 
> Whoops, yes, I missed xfs/062
>  
> > I'm thinking to introduce a new helper to require a given agcount will
> > fit the device size, and if the device is bigger than ($agcount * 1T)
> > then _notrun the test, something like (may need a better helper name):
> > 
> > _require_xfs_support_agcount()
> > {
> > 	local dev=$1
> > 	local agcount=$2
> > 	local max_sz=$((agcount*(2**40)))
> > 	local dev_sz=$(blockdev --getsize64 $dev)
> > 
> > 	if [ $dev_sz -gt $max_sz ]; then
> > 		_notrun "agcount $agcount is too small to hold $dev_sz device"
> > 	fi
> > }
> 
> I'm not sure we should _notrun, though - if what we really want to check
> is a 2-AG filesystem on the scratch dev, there are times when we may simply
> want to make a smaller filesystem and proceed.  For the cases I sent
> in my patch this should be perfectly fine...
> 
> > Then add this _require rule to all tests that only specify a custom
> > agsize, e.g. for xfs/010 we could do
> > 
> > _require_xfs_support_agcount $SCRATCH_DEV 2
> > 
> > I roughly went through all xfs tests and found that the following ones
> > may need this new _require rule
> > 
> > xfs/010
> > xfs/013
> > xfs/062
> > xfs/178
> > xfs/179
> > xfs/310
> > 
> > Perhaps there're better ways to solve the problem, any suggestions are
> > welcomed! 
> 
> Rather than a _require and a _notrun, what about a mkfs_scratch_agcount()
> that does something like

Yeah, I know there're better ways :)

> 
> mkfs_scratch_agcount()

Or _scratch_mkfs_agcount()? Following the naming as in _scratch_mkfs_sized().

> {
>  agcount=$1
>  opts=$2

And make sure local variables are declared as "local".

> 
>  # If $agcount AGs would result in too-large AG size, restrict the size
>  # to create $agcount roughly 1T AGs.
>  dsizeopt=""
>  dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)
>  if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
>    dsizeopt="-d size=$(($agcount*((2**40)-1)))"
>  fi
>  _scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
> }
> 
> or something like that?

Thanks!

Eryu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] misc large filesystem fixes
  2018-08-30  3:30   ` Eric Sandeen
  2018-08-30  4:29     ` Eryu Guan
@ 2018-08-30  4:34     ` Dave Chinner
  2018-08-31 17:44       ` Eric Sandeen
  1 sibling, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2018-08-30  4:34 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eryu Guan, Eric Sandeen, fstests

On Wed, Aug 29, 2018 at 10:30:29PM -0500, Eric Sandeen wrote:
> On 8/29/18 10:19 PM, Eryu Guan wrote:
> > On Wed, Aug 29, 2018 at 04:43:38PM -0500, Eric Sandeen wrote:
> >> There are a few tests which fail on large filesytems because
> >> we run into mkfs limits.
> >>
> >> xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
> >> than 2T this will fail.  Check the device size and restrict it
> >> to just under 2T so that a 2-AG mkfs is possible.

....
> > I'm thinking to introduce a new helper to require a given agcount will
> > fit the device size, and if the device is bigger than ($agcount * 1T)
> > then _notrun the test, something like (may need a better helper name):
> > 
> > _require_xfs_support_agcount()
> > {
> > 	local dev=$1
> > 	local agcount=$2
> > 	local max_sz=$((agcount*(2**40)))
> > 	local dev_sz=$(blockdev --getsize64 $dev)
> > 
> > 	if [ $dev_sz -gt $max_sz ]; then
> > 		_notrun "agcount $agcount is too small to hold $dev_sz device"
> > 	fi
> > }
> 
> I'm not sure we should _notrun, though - if what we really want to check
> is a 2-AG filesystem on the scratch dev, there are times when we may simply
> want to make a smaller filesystem and proceed.  For the cases I sent
> in my patch this should be perfectly fine...
> 
> > Then add this _require rule to all tests that only specify a custom
> > agsize, e.g. for xfs/010 we could do
> > 
> > _require_xfs_support_agcount $SCRATCH_DEV 2
> > 
> > I roughly went through all xfs tests and found that the following ones
> > may need this new _require rule
> > 
> > xfs/010
> > xfs/013
> > xfs/062
> > xfs/178
> > xfs/179
> > xfs/310
> > 
> > Perhaps there're better ways to solve the problem, any suggestions are
> > welcomed! 
> 
> Rather than a _require and a _notrun, what about a mkfs_scratch_agcount()
> that does something like
> 
> mkfs_scratch_agcount()
> {
>  agcount=$1
>  opts=$2
> 
>  # If $agcount AGs would result in too-large AG size, restrict the size
>  # to create $agcount roughly 1T AGs.
>  dsizeopt=""
>  dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)
>  if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
>    dsizeopt="-d size=$(($agcount*((2**40)-1)))"
>  fi
>  _scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
> }
> 
> or something like that?

Or, simpler options:

	- scratch_mkfs_sized with a size appropriate for the test
	  (works for every configuration)
	- _require_no_large_scratch_device (or whatever it's name
	  is) to skip the test on large devices
	- SCRATCH_MKFS_OPTIONS="-d agcount=500" on a 10GB scratch
	  device (i.e. 500x20MB AGs) will exercise most of the dusty
	  code corners cases that a 500TB filesystem with 1TB AGs
	  and 49.95TB preallocated by --largefs.

Remember, we don't have to test /everything/ with large filesystems
- most of the filesystem functionality behaves exactly the same on
small and large filesytsems. i.e. the largefs option is to be able
run smoke, stress and ENOSPC tests on unreasonably large
filesystems with a very small sparse backing store space
requirements, not run our entire suite of pin-point correctness and
regression tests that mostly only require a few megabytes of space
to run.... 

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] misc large filesystem fixes
  2018-08-30  4:34     ` Dave Chinner
@ 2018-08-31 17:44       ` Eric Sandeen
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2018-08-31 17:44 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Eryu Guan, Eric Sandeen, fstests



On 8/29/18 11:34 PM, Dave Chinner wrote:
> On Wed, Aug 29, 2018 at 10:30:29PM -0500, Eric Sandeen wrote:
>> On 8/29/18 10:19 PM, Eryu Guan wrote:
>>> On Wed, Aug 29, 2018 at 04:43:38PM -0500, Eric Sandeen wrote:
>>>> There are a few tests which fail on large filesytems because
>>>> we run into mkfs limits.
>>>>
>>>> xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
>>>> than 2T this will fail.  Check the device size and restrict it
>>>> to just under 2T so that a 2-AG mkfs is possible.
> 
> ....
>>> I'm thinking to introduce a new helper to require a given agcount will
>>> fit the device size, and if the device is bigger than ($agcount * 1T)
>>> then _notrun the test, something like (may need a better helper name):
>>>
>>> _require_xfs_support_agcount()
>>> {
>>> 	local dev=$1
>>> 	local agcount=$2
>>> 	local max_sz=$((agcount*(2**40)))
>>> 	local dev_sz=$(blockdev --getsize64 $dev)
>>>
>>> 	if [ $dev_sz -gt $max_sz ]; then
>>> 		_notrun "agcount $agcount is too small to hold $dev_sz device"
>>> 	fi
>>> }
>>
>> I'm not sure we should _notrun, though - if what we really want to check
>> is a 2-AG filesystem on the scratch dev, there are times when we may simply
>> want to make a smaller filesystem and proceed.  For the cases I sent
>> in my patch this should be perfectly fine...
>>
>>> Then add this _require rule to all tests that only specify a custom
>>> agsize, e.g. for xfs/010 we could do
>>>
>>> _require_xfs_support_agcount $SCRATCH_DEV 2
>>>
>>> I roughly went through all xfs tests and found that the following ones
>>> may need this new _require rule
>>>
>>> xfs/010
>>> xfs/013
>>> xfs/062
>>> xfs/178
>>> xfs/179
>>> xfs/310
>>>
>>> Perhaps there're better ways to solve the problem, any suggestions are
>>> welcomed! 
>>
>> Rather than a _require and a _notrun, what about a mkfs_scratch_agcount()
>> that does something like
>>
>> mkfs_scratch_agcount()
>> {
>>  agcount=$1
>>  opts=$2
>>
>>  # If $agcount AGs would result in too-large AG size, restrict the size
>>  # to create $agcount roughly 1T AGs.
>>  dsizeopt=""
>>  dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)
>>  if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
>>    dsizeopt="-d size=$(($agcount*((2**40)-1)))"
>>  fi
>>  _scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
>> }
>>
>> or something like that?
> 
> Or, simpler options:
> 
> 	- scratch_mkfs_sized with a size appropriate for the test
> 	  (works for every configuration)

AFAIK no way to specify agcount there at this time, is there?
Could teach it to accept more options I suppose.

> 	- _require_no_large_scratch_device (or whatever it's name
> 	  is) to skip the test on large devices

Works if people always specify --large-fs for >= 4T, but not if they
don't.

> 	- SCRATCH_MKFS_OPTIONS="-d agcount=500" on a 10GB scratch
> 	  device (i.e. 500x20MB AGs) will exercise most of the dusty
> 	  code corners cases that a 500TB filesystem with 1TB AGs
> 	  and 49.95TB preallocated by --largefs.

Sure, we could run different tests on smaller devices instead.

But that doesn't solve the problem of spurious errors on actual large
devices, which is what I was trying to solve here.

> Remember, we don't have to test /everything/ with large filesystems
> - most of the filesystem functionality behaves exactly the same on
> small and large filesytsems. i.e. the largefs option is to be able
> run smoke, stress and ENOSPC tests on unreasonably large
> filesystems with a very small sparse backing store space
> requirements, not run our entire suite of pin-point correctness and
> regression tests that mostly only require a few megabytes of space
> to run.... 

If we'd rather _notrun than fix, it might take a new _require* to exclude
sufficiently large devices even in cases where --large-fs was not specified.

Really not sure what's wrong with a function which can reliably create X AGs
on the scratch dev, though.  Multiple tests want to do that, and the request
can fail today if the device is either too large or too small, and it's not
gracefully handled.

-Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V2] misc large filesystem fixes
  2018-08-29 21:43 [PATCH] misc large filesystem fixes Eric Sandeen
  2018-08-30  3:19 ` Eryu Guan
@ 2018-09-25 19:12 ` Eric Sandeen
  2018-10-06 11:15   ` Eryu Guan
  1 sibling, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2018-09-25 19:12 UTC (permalink / raw)
  To: Eric Sandeen, fstests

There are a few tests which fail on large filesytems because
we run into mkfs limits.

xfs/010, xfs/013, and xfs/062 specify AG count, but if the device
is larger than agcount*1T this will fail.  Add a mkfs helper that
will adjust the data size to accommodate the ag count request.
xfs/178 tries to decrease the agcount and re-mkfs, but if the
default AG size was chosen to be 1T, decreasing the AG count
results in too-large AGs and mkfs fails.  The intention here
AFAICT is to simply re-mkfs with non-overlapping AG headers,
so increasing the AG count should achieve the same purpose,
and cause mkfs to choose a smaller-than-default AG size which
should pass.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/common/xfs b/common/xfs
index d971b4a8..34412882 100644
--- a/common/xfs
+++ b/common/xfs
@@ -110,6 +110,21 @@ _scratch_mkfs_xfs()
 	return $mkfs_status
 }
 
+_scratch_mkfs_xfs_agcount()
+{
+	local agcount=$1
+	local opts=$2
+
+	# If $agcount AGs would result in too-large AG size, restrict the size
+	# to create $agcount AGS roughly 1T in size.
+	local dsizeopt=""
+	dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)
+	if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
+		  dsizeopt="-d size=$(($agcount*((2**40)-1)))"
+	fi
+	_scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
+}
+
 # xfs_check script is planned to be deprecated. But, we want to
 # be able to invoke "xfs_check" behavior in xfstests in order to
 # maintain the current verification levels.
diff --git a/tests/xfs/010 b/tests/xfs/010
index ee1595c8..5da815a5 100755
--- a/tests/xfs/010
+++ b/tests/xfs/010
@@ -96,7 +96,7 @@ _require_xfs_finobt
 
 rm -f $seqres.full
 
-_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
+_scratch_mkfs_xfs_agcount 2 "-m crc=1,finobt=1"
 
 # sparsely populate the fs such that we create records with free inodes
 _scratch_mount
diff --git a/tests/xfs/013 b/tests/xfs/013
index 4d31d793..f45217ba 100755
--- a/tests/xfs/013
+++ b/tests/xfs/013
@@ -97,8 +97,7 @@ _require_command "$KILLALL_PROG" killall
 
 rm -f $seqres.full
 
-_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | \
-	_filter_mkfs 2>> $seqres.full
+_scratch_mkfs_xfs_agcount 2 "-m crc=1,finobt=1"
 _scratch_mount
 
 COUNT=20000	# number of files per directory
diff --git a/tests/xfs/062 b/tests/xfs/062
index 755c5243..c0f82a78 100755
--- a/tests/xfs/062
+++ b/tests/xfs/062
@@ -57,7 +57,7 @@ rm -f $seqres.full
 DIRCOUNT=8
 INOCOUNT=$((2048 / DIRCOUNT))
 
-_scratch_mkfs "-d agcount=$DIRCOUNT" >> $seqres.full 2>&1 || _fail "mkfs failed"
+_scratch_mkfs_xfs_agcount $DIRCOUNT >> $seqres.full 2>&1 || _fail "mkfs failed"
 _scratch_mount
 
 # create a set of directories and fill each with a fixed number of files
diff --git a/tests/xfs/178 b/tests/xfs/178
index 84151056..f7ea9139 100755
--- a/tests/xfs/178
+++ b/tests/xfs/178
@@ -51,8 +51,8 @@ _supported_os Linux
 # o Summary of testing:
 #    1. mkfs.xfs a default filesystem, note agcount value.
 #    2. dd zero first sector and repair and verify.
-#    3. mkfs.xfs overriding agcount to a smaller value
-#             (ie. each AG is bigger)
+#    3. mkfs.xfs overriding agcount to a larger value
+#             (ie. each AG is smaller)
 #    4. dd zero first sector, repair and verify.
 #          -> old mkfs.xfs will cause repair to incorrectly
 #             fix filesystem, new mkfs.xfs will be fine.
@@ -74,8 +74,8 @@ fi
 
 _dd_repair_check $SCRATCH_DEV $sectsz
 
-# smaller AGCOUNT
-let "agcount=$agcount-2"
+# larger AGCOUNT
+let "agcount=$agcount+2"
 _scratch_mkfs_xfs -dagcount=$agcount >/dev/null 2>&1 \
         || _fail "mkfs failed!"
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] misc large filesystem fixes
  2018-09-25 19:12 ` [PATCH V2] " Eric Sandeen
@ 2018-10-06 11:15   ` Eryu Guan
  0 siblings, 0 replies; 8+ messages in thread
From: Eryu Guan @ 2018-10-06 11:15 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, fstests

On Tue, Sep 25, 2018 at 02:12:40PM -0500, Eric Sandeen wrote:
> There are a few tests which fail on large filesytems because
> we run into mkfs limits.
> 
> xfs/010, xfs/013, and xfs/062 specify AG count, but if the device
> is larger than agcount*1T this will fail.  Add a mkfs helper that
> will adjust the data size to accommodate the ag count request.
> xfs/178 tries to decrease the agcount and re-mkfs, but if the
> default AG size was chosen to be 1T, decreasing the AG count
> results in too-large AGs and mkfs fails.  The intention here
> AFAICT is to simply re-mkfs with non-overlapping AG headers,
> so increasing the AG count should achieve the same purpose,
> and cause mkfs to choose a smaller-than-default AG size which
> should pass.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/common/xfs b/common/xfs
> index d971b4a8..34412882 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -110,6 +110,21 @@ _scratch_mkfs_xfs()
>  	return $mkfs_status
>  }
>  
> +_scratch_mkfs_xfs_agcount()
> +{
> +	local agcount=$1
> +	local opts=$2
> +
> +	# If $agcount AGs would result in too-large AG size, restrict the size
> +	# to create $agcount AGS roughly 1T in size.
> +	local dsizeopt=""
> +	dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)

local dev_sz ?

> +	if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
> +		  dsizeopt="-d size=$(($agcount*((2**40)-1)))"

Hmm.. this size doesn't seem right to me, I think the $agcount should
not be accounted in the size, otherwise the size can go beyond 1T again.

(After fixing above issue) mkfs reports failure as well:

"illegal data length 2199023255550, not a multiple of 512"

Perhaps $((((2**40)) - 4096)) would be a safe size.

Thanks,
Eryu

> +	fi
> +	_scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
> +}
> +
>  # xfs_check script is planned to be deprecated. But, we want to
>  # be able to invoke "xfs_check" behavior in xfstests in order to
>  # maintain the current verification levels.
> diff --git a/tests/xfs/010 b/tests/xfs/010
> index ee1595c8..5da815a5 100755
> --- a/tests/xfs/010
> +++ b/tests/xfs/010
> @@ -96,7 +96,7 @@ _require_xfs_finobt
>  
>  rm -f $seqres.full
>  
> -_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
> +_scratch_mkfs_xfs_agcount 2 "-m crc=1,finobt=1"
>  
>  # sparsely populate the fs such that we create records with free inodes
>  _scratch_mount
> diff --git a/tests/xfs/013 b/tests/xfs/013
> index 4d31d793..f45217ba 100755
> --- a/tests/xfs/013
> +++ b/tests/xfs/013
> @@ -97,8 +97,7 @@ _require_command "$KILLALL_PROG" killall
>  
>  rm -f $seqres.full
>  
> -_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | \
> -	_filter_mkfs 2>> $seqres.full
> +_scratch_mkfs_xfs_agcount 2 "-m crc=1,finobt=1"
>  _scratch_mount
>  
>  COUNT=20000	# number of files per directory
> diff --git a/tests/xfs/062 b/tests/xfs/062
> index 755c5243..c0f82a78 100755
> --- a/tests/xfs/062
> +++ b/tests/xfs/062
> @@ -57,7 +57,7 @@ rm -f $seqres.full
>  DIRCOUNT=8
>  INOCOUNT=$((2048 / DIRCOUNT))
>  
> -_scratch_mkfs "-d agcount=$DIRCOUNT" >> $seqres.full 2>&1 || _fail "mkfs failed"
> +_scratch_mkfs_xfs_agcount $DIRCOUNT >> $seqres.full 2>&1 || _fail "mkfs failed"
>  _scratch_mount
>  
>  # create a set of directories and fill each with a fixed number of files
> diff --git a/tests/xfs/178 b/tests/xfs/178
> index 84151056..f7ea9139 100755
> --- a/tests/xfs/178
> +++ b/tests/xfs/178
> @@ -51,8 +51,8 @@ _supported_os Linux
>  # o Summary of testing:
>  #    1. mkfs.xfs a default filesystem, note agcount value.
>  #    2. dd zero first sector and repair and verify.
> -#    3. mkfs.xfs overriding agcount to a smaller value
> -#             (ie. each AG is bigger)
> +#    3. mkfs.xfs overriding agcount to a larger value
> +#             (ie. each AG is smaller)
>  #    4. dd zero first sector, repair and verify.
>  #          -> old mkfs.xfs will cause repair to incorrectly
>  #             fix filesystem, new mkfs.xfs will be fine.
> @@ -74,8 +74,8 @@ fi
>  
>  _dd_repair_check $SCRATCH_DEV $sectsz
>  
> -# smaller AGCOUNT
> -let "agcount=$agcount-2"
> +# larger AGCOUNT
> +let "agcount=$agcount+2"
>  _scratch_mkfs_xfs -dagcount=$agcount >/dev/null 2>&1 \
>          || _fail "mkfs failed!"
>  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-06 18:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29 21:43 [PATCH] misc large filesystem fixes Eric Sandeen
2018-08-30  3:19 ` Eryu Guan
2018-08-30  3:30   ` Eric Sandeen
2018-08-30  4:29     ` Eryu Guan
2018-08-30  4:34     ` Dave Chinner
2018-08-31 17:44       ` Eric Sandeen
2018-09-25 19:12 ` [PATCH V2] " Eric Sandeen
2018-10-06 11:15   ` Eryu Guan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.