From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f196.google.com ([209.85.214.196]:37906 "EHLO mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725904AbeH3I3l (ORCPT ); Thu, 30 Aug 2018 04:29:41 -0400 Received: by mail-pl1-f196.google.com with SMTP id u11-v6so3280374plq.5 for ; Wed, 29 Aug 2018 21:29:27 -0700 (PDT) Date: Thu, 30 Aug 2018 12:29:21 +0800 From: Eryu Guan Subject: Re: [PATCH] misc large filesystem fixes Message-ID: <20180830042921.GD3651@desktop> References: <20180830031902.GC3651@desktop> <7869cd0a-ddb6-4972-1a36-446549d59a62@sandeen.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7869cd0a-ddb6-4972-1a36-446549d59a62@sandeen.net> Sender: fstests-owner@vger.kernel.org To: Eric Sandeen Cc: Eric Sandeen , fstests List-ID: 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 > >> --- > >> > >> 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