diff mbox series

[2/2] generic/530: only use xfs-specific mkfs options when testing on xfs

Message ID 20241215051242.3340572-3-tytso@mit.edu (mailing list archive)
State New
Headers show
Series Fix recent regressions from for_next | expand

Commit Message

Theodore Ts'o Dec. 15, 2024, 5:12 a.m. UTC
This fixes a regression introduced by commit 000813899afb ("fstests:
scale some tests for high CPU count sanity") where this test
unconditionally tried to use the mkfs option "-l size=256m" which
would break when testing any file sytem other than xfs.

Fix this the same way commit 000813899afb dealt with this for
generic/531; so this was just an oversight.

Fixes: 000813899afb ("fstests: scale some tests for high CPU count sanity")
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/generic/530 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Dec. 17, 2024, 8:11 a.m. UTC | #1
On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> +if [ $FSTYP = "xfs" ]; then
> +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> +else
> +    _scratch_mkfs >> $seqres.full 2>&1
> +fi

We really need to document why generic tests have file system specific
hacks.  And yes, that's a request for Dave who originally added it
without any explanation and not Ted.
Dave Chinner Dec. 18, 2024, 10:26 p.m. UTC | #2
On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > +if [ $FSTYP = "xfs" ]; then
> > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > +else
> > +    _scratch_mkfs >> $seqres.full 2>&1
> > +fi
> 
> We really need to document why generic tests have file system specific
> hacks.  And yes, that's a request for Dave who originally added it
> without any explanation and not Ted.

Creating and then unlinking 50000 files is journal space bound
when using default 64MB logs on small test filesystems. Increasing
the journal size to 256MB halved the runtime of this test.

-Dave.
Christoph Hellwig Dec. 19, 2024, 5:37 a.m. UTC | #3
On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > +if [ $FSTYP = "xfs" ]; then
> > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > +else
> > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > +fi
> > 
> > We really need to document why generic tests have file system specific
> > hacks.  And yes, that's a request for Dave who originally added it
> > without any explanation and not Ted.
> 
> Creating and then unlinking 50000 files is journal space bound
> when using default 64MB logs on small test filesystems. Increasing
> the journal size to 256MB halved the runtime of this test.

Please explain this in thet test.  And you probably also want to
ensure that you don't force the log smaller than 256 either, otherwise
people in 10 or 20 years will wonder why this test forces logs to
be so small.
Zorro Lang Dec. 19, 2024, 10:16 a.m. UTC | #4
On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > +if [ $FSTYP = "xfs" ]; then
> > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > +else
> > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > +fi
> > > 
> > > We really need to document why generic tests have file system specific
> > > hacks.  And yes, that's a request for Dave who originally added it
> > > without any explanation and not Ted.
> > 
> > Creating and then unlinking 50000 files is journal space bound
> > when using default 64MB logs on small test filesystems. Increasing
> > the journal size to 256MB halved the runtime of this test.
> 
> Please explain this in thet test.  And you probably also want to
> ensure that you don't force the log smaller than 256 either, otherwise
> people in 10 or 20 years will wonder why this test forces logs to
> be so small.

I'll help to add this comment when I merge this patch, if Dave hope to
keep "-l size=256m" for xfs.

Thanks,
Zorro

>
Darrick J. Wong Dec. 19, 2024, 4:32 p.m. UTC | #5
On Thu, Dec 19, 2024 at 06:16:26PM +0800, Zorro Lang wrote:
> On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> > On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > > +if [ $FSTYP = "xfs" ]; then
> > > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > > +else
> > > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > > +fi
> > > > 
> > > > We really need to document why generic tests have file system specific
> > > > hacks.  And yes, that's a request for Dave who originally added it
> > > > without any explanation and not Ted.
> > > 
> > > Creating and then unlinking 50000 files is journal space bound
> > > when using default 64MB logs on small test filesystems. Increasing
> > > the journal size to 256MB halved the runtime of this test.
> > 
> > Please explain this in thet test.  And you probably also want to
> > ensure that you don't force the log smaller than 256 either, otherwise
> > people in 10 or 20 years will wonder why this test forces logs to
> > be so small.
> 
> I'll help to add this comment when I merge this patch, if Dave hope to
> keep "-l size=256m" for xfs.

What happens if someone runs fstests with a 128M external log device?
Is this one of those cases where _scratch_mkfs notices the mkfs failure
and formats without MKFS_OPTIONS?  And if that's true, what about my
test configs that set MKFS_OPTIONS to test new non-default features?

--D

> Thanks,
> Zorro
> 
> > 
> 
>
Dave Chinner Dec. 19, 2024, 8:49 p.m. UTC | #6
On Thu, Dec 19, 2024 at 08:32:06AM -0800, Darrick J. Wong wrote:
> On Thu, Dec 19, 2024 at 06:16:26PM +0800, Zorro Lang wrote:
> > On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> > > On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > > > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > > > +if [ $FSTYP = "xfs" ]; then
> > > > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > > > +else
> > > > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > > > +fi
> > > > > 
> > > > > We really need to document why generic tests have file system specific
> > > > > hacks.  And yes, that's a request for Dave who originally added it
> > > > > without any explanation and not Ted.
> > > > 
> > > > Creating and then unlinking 50000 files is journal space bound
> > > > when using default 64MB logs on small test filesystems. Increasing
> > > > the journal size to 256MB halved the runtime of this test.
> > > 
> > > Please explain this in thet test.  And you probably also want to
> > > ensure that you don't force the log smaller than 256 either, otherwise
> > > people in 10 or 20 years will wonder why this test forces logs to
> > > be so small.
> > 
> > I'll help to add this comment when I merge this patch, if Dave hope to
> > keep "-l size=256m" for xfs.
> 
> What happens if someone runs fstests with a 128M external log device?

It fails, then drops MKFS_OPTIONS.  The simple solution for this is
to simply to use a larger external log device....

> Is this one of those cases where _scratch_mkfs notices the mkfs failure
> and formats without MKFS_OPTIONS? 

More than likely.

> And if that's true, what about my
> test configs that set MKFS_OPTIONS to test new non-default features?

Changing existing infrastructure behaviour to better suit *your*
test environments is *your* responsibility to address, not mine.
I don't care if MKFS_OPTIONS get dropped in occasional tests, it's
more important to me that the tests run fast so I can iterate
my modify-build-test development cycle faster.

It should be trivial for you to address, though. Add a function
like:

_scratch_mkfs_try_option "-l size=256M" 

which has the opposite fallback behaviour of dropping the test
supplied option and using MKFS_OPTIONS instead, and I'll use it for
all these "test go faster" modifications that we badly need to
address...

FWIW, this would also get rid of the need for the FSTYP checks in
the test, too, because passing "-l size=256M" will fail on btrfs,
ext4, etc and then they fall back to the specific test config...

So, provide me with the infrastructure that makes stuff like this
work properly in *your test environment*, and I'll use it
appropriately.

-Dave.
Darrick J. Wong Dec. 19, 2024, 10:53 p.m. UTC | #7
On Fri, Dec 20, 2024 at 07:49:28AM +1100, Dave Chinner wrote:
> On Thu, Dec 19, 2024 at 08:32:06AM -0800, Darrick J. Wong wrote:
> > On Thu, Dec 19, 2024 at 06:16:26PM +0800, Zorro Lang wrote:
> > > On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> > > > On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > > > > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > > > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > > > > +if [ $FSTYP = "xfs" ]; then
> > > > > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > > > > +else
> > > > > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > > > > +fi
> > > > > > 
> > > > > > We really need to document why generic tests have file system specific
> > > > > > hacks.  And yes, that's a request for Dave who originally added it
> > > > > > without any explanation and not Ted.
> > > > > 
> > > > > Creating and then unlinking 50000 files is journal space bound
> > > > > when using default 64MB logs on small test filesystems. Increasing
> > > > > the journal size to 256MB halved the runtime of this test.
> > > > 
> > > > Please explain this in thet test.  And you probably also want to
> > > > ensure that you don't force the log smaller than 256 either, otherwise
> > > > people in 10 or 20 years will wonder why this test forces logs to
> > > > be so small.
> > > 
> > > I'll help to add this comment when I merge this patch, if Dave hope to
> > > keep "-l size=256m" for xfs.
> > 
> > What happens if someone runs fstests with a 128M external log device?
> 
> It fails, then drops MKFS_OPTIONS.  The simple solution for this is
> to simply to use a larger external log device....
> 
> > Is this one of those cases where _scratch_mkfs notices the mkfs failure
> > and formats without MKFS_OPTIONS? 
> 
> More than likely.
> 
> > And if that's true, what about my
> > test configs that set MKFS_OPTIONS to test new non-default features?
> 
> Changing existing infrastructure behaviour to better suit *your*
> test environments is *your* responsibility to address, not mine.
> I don't care if MKFS_OPTIONS get dropped in occasional tests, it's
> more important to me that the tests run fast so I can iterate
> my modify-build-test development cycle faster.

I don't agree that your personal development velocity is more important
than silent loss of code coverage of everyone who happens to be testing
external log devices.  That at least covers myself, our QA department,
and everyone who uses the prepackaged configurations in kdevops and
xfstests-bld.

> It should be trivial for you to address, though. Add a function
> like:
> 
> _scratch_mkfs_try_option "-l size=256M" 

Or instead:

# Pushing 50000 unlinked inode inactivations through a small xfs log can
# result in bottlenecks on the log grant heads, so try to make the log
# larger to reduce runtime.
if [ $FSTYP = xfs ] && [ -z "$SCRATCH_LOGDEV ]; then
	_scratch_mkfs -l size=256M
else
	_scratch_mkfs
fi

> which has the opposite fallback behaviour of dropping the test
> supplied option and using MKFS_OPTIONS instead, and I'll use it for
> all these "test go faster" modifications that we badly need to
> address...
> 
> FWIW, this would also get rid of the need for the FSTYP checks in
> the test, too, because passing "-l size=256M" will fail on btrfs,
> ext4, etc and then they fall back to the specific test config...

Huh?  And what happens when btrfs want their own try option?

> So, provide me with the infrastructure that makes stuff like this
> work properly in *your test environment*, and I'll use it
> appropriately.

Dave.

You're the one who made the changes to generic/53*.  It worked before.

Like any other code change, it's /your/ responsibility to try not to
cause regressions for everyone else using the same code.  If other
people have problems with a change, it's longstanding community
expectation that the author should help resolve the problem.

It is /not/ standard practice to lecture the affected people about how
they should embark on a treewide change to fix a problem that didn't
exist previously.  The maintainer reverts the offending commit and the
author redrafts the patch.  You're well aware of this, because you've
said as much about other patches (e.g. iomap zeroing cleanups).

It is not everyone else's (Ted, Christoph, Zorro) role to clean up after
you.

I do not appreciate the condescending behavior you have shown towards me
this year and last.  Next year I will take more steps to insulate my
team from you and other community members as needed because I don't want
them to continue to deal with such conduct[1][2].

--D

[1] https://lore.kernel.org/fstests/Z0pBKWBlXLBxwK3D@dread.disaster.area/
[2] https://lore.kernel.org/linux-xfs/Z1FQdYEXLR5BoOE-@redhat.com/

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
Eric Sandeen Dec. 20, 2024, 3:11 p.m. UTC | #8
<snip the latest grousing back and forth>

Gentlemen, It would go a really long way if everyone could keep a
few things in mind as we work together in this public space.

1) Act in good faith
2) Assume good intent
3) Don't escalate conflict
4) Own your mistakes

That's basic stuff. Could we maybe give that a shot for a while,
and see if we can reduce this misery a bit?

(If you took any of the above personally, please don't. I'm just
reflecting on what I think are some longstanding patterns, here.)

Everybody's working hard, everybody's stressed, everybody makes
mistakes. I have no desire to go point by point through some
of the many recent blow-ups, but in nearly every one of them I
think things would have gone better if at any time during the back
and forth, people had stopped to reflect on the points above
before firing off a reply.

Above isn't perfect and perhaps others have thoughts on basic
guidelines for communicating without conflict, but maybe it's a
start?

-Eric
Zorro Lang Dec. 22, 2024, 12:22 p.m. UTC | #9
On Thu, Dec 19, 2024 at 02:53:31PM -0800, Darrick J. Wong wrote:
> On Fri, Dec 20, 2024 at 07:49:28AM +1100, Dave Chinner wrote:
> > On Thu, Dec 19, 2024 at 08:32:06AM -0800, Darrick J. Wong wrote:
> > > On Thu, Dec 19, 2024 at 06:16:26PM +0800, Zorro Lang wrote:
> > > > On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> > > > > On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > > > > > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > > > > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > > > > > +if [ $FSTYP = "xfs" ]; then
> > > > > > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > > > > > +else
> > > > > > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > > > > > +fi
> > > > > > > 
> > > > > > > We really need to document why generic tests have file system specific
> > > > > > > hacks.  And yes, that's a request for Dave who originally added it
> > > > > > > without any explanation and not Ted.
> > > > > > 
> > > > > > Creating and then unlinking 50000 files is journal space bound
> > > > > > when using default 64MB logs on small test filesystems. Increasing
> > > > > > the journal size to 256MB halved the runtime of this test.
> > > > > 
> > > > > Please explain this in thet test.  And you probably also want to
> > > > > ensure that you don't force the log smaller than 256 either, otherwise
> > > > > people in 10 or 20 years will wonder why this test forces logs to
> > > > > be so small.
> > > > 
> > > > I'll help to add this comment when I merge this patch, if Dave hope to
> > > > keep "-l size=256m" for xfs.
> > > 
> > > What happens if someone runs fstests with a 128M external log device?
> > 
> > It fails, then drops MKFS_OPTIONS.  The simple solution for this is
> > to simply to use a larger external log device....
> > 
> > > Is this one of those cases where _scratch_mkfs notices the mkfs failure
> > > and formats without MKFS_OPTIONS? 
> > 
> > More than likely.
> > 
> > > And if that's true, what about my
> > > test configs that set MKFS_OPTIONS to test new non-default features?
> > 
> > Changing existing infrastructure behaviour to better suit *your*
> > test environments is *your* responsibility to address, not mine.
> > I don't care if MKFS_OPTIONS get dropped in occasional tests, it's
> > more important to me that the tests run fast so I can iterate
> > my modify-build-test development cycle faster.
> 
> I don't agree that your personal development velocity is more important
> than silent loss of code coverage of everyone who happens to be testing
> external log devices.  That at least covers myself, our QA department,
> and everyone who uses the prepackaged configurations in kdevops and
> xfstests-bld.
> 
> > It should be trivial for you to address, though. Add a function
> > like:
> > 
> > _scratch_mkfs_try_option "-l size=256M" 
> 
> Or instead:
> 
> # Pushing 50000 unlinked inode inactivations through a small xfs log can
> # result in bottlenecks on the log grant heads, so try to make the log
> # larger to reduce runtime.
> if [ $FSTYP = xfs ] && [ -z "$SCRATCH_LOGDEV ]; then
> 	_scratch_mkfs -l size=256M

Maybe better to use "_has_logdev", due to there's "$USE_EXTERNAL".
I can change this patch as:

  # Pushing 50000 unlinked inode inactivations through a small xfs log can
  # result in bottlenecks on the log grant heads, so try to make the log
  # larger to reduce runtime.
  if [ "$FSTYP" = "xfs" ] && _has_logdev; then
  	_scratch_mkfs -l size=256M
  else
  	_scratch_mkfs
  fi

when I merge it, if you all agree :)

Thanks,
Zorro

> else
> 	_scratch_mkfs
> fi
> 
> > which has the opposite fallback behaviour of dropping the test
> > supplied option and using MKFS_OPTIONS instead, and I'll use it for
> > all these "test go faster" modifications that we badly need to
> > address...
> > 
> > FWIW, this would also get rid of the need for the FSTYP checks in
> > the test, too, because passing "-l size=256M" will fail on btrfs,
> > ext4, etc and then they fall back to the specific test config...
> 
> Huh?  And what happens when btrfs want their own try option?
> 
> > So, provide me with the infrastructure that makes stuff like this
> > work properly in *your test environment*, and I'll use it
> > appropriately.
> 
> Dave.
> 
> You're the one who made the changes to generic/53*.  It worked before.
> 
> Like any other code change, it's /your/ responsibility to try not to
> cause regressions for everyone else using the same code.  If other
> people have problems with a change, it's longstanding community
> expectation that the author should help resolve the problem.
> 
> It is /not/ standard practice to lecture the affected people about how
> they should embark on a treewide change to fix a problem that didn't
> exist previously.  The maintainer reverts the offending commit and the
> author redrafts the patch.  You're well aware of this, because you've
> said as much about other patches (e.g. iomap zeroing cleanups).
> 
> It is not everyone else's (Ted, Christoph, Zorro) role to clean up after
> you.
> 
> I do not appreciate the condescending behavior you have shown towards me
> this year and last.  Next year I will take more steps to insulate my
> team from you and other community members as needed because I don't want
> them to continue to deal with such conduct[1][2].
> 
> --D
> 
> [1] https://lore.kernel.org/fstests/Z0pBKWBlXLBxwK3D@dread.disaster.area/
> [2] https://lore.kernel.org/linux-xfs/Z1FQdYEXLR5BoOE-@redhat.com/
> 
> > -Dave.
> > -- 
> > Dave Chinner
> > david@fromorbit.com
>
Zorro Lang Dec. 22, 2024, 2:22 p.m. UTC | #10
On Sun, Dec 22, 2024 at 08:22:46PM +0800, Zorro Lang wrote:
> On Thu, Dec 19, 2024 at 02:53:31PM -0800, Darrick J. Wong wrote:
> > On Fri, Dec 20, 2024 at 07:49:28AM +1100, Dave Chinner wrote:
> > > On Thu, Dec 19, 2024 at 08:32:06AM -0800, Darrick J. Wong wrote:
> > > > On Thu, Dec 19, 2024 at 06:16:26PM +0800, Zorro Lang wrote:
> > > > > On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> > > > > > On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > > > > > > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > > > > > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > > > > > > +if [ $FSTYP = "xfs" ]; then
> > > > > > > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > > > > > > +else
> > > > > > > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > > > > > > +fi
> > > > > > > > 
> > > > > > > > We really need to document why generic tests have file system specific
> > > > > > > > hacks.  And yes, that's a request for Dave who originally added it
> > > > > > > > without any explanation and not Ted.
> > > > > > > 
> > > > > > > Creating and then unlinking 50000 files is journal space bound
> > > > > > > when using default 64MB logs on small test filesystems. Increasing
> > > > > > > the journal size to 256MB halved the runtime of this test.
> > > > > > 
> > > > > > Please explain this in thet test.  And you probably also want to
> > > > > > ensure that you don't force the log smaller than 256 either, otherwise
> > > > > > people in 10 or 20 years will wonder why this test forces logs to
> > > > > > be so small.
> > > > > 
> > > > > I'll help to add this comment when I merge this patch, if Dave hope to
> > > > > keep "-l size=256m" for xfs.
> > > > 
> > > > What happens if someone runs fstests with a 128M external log device?
> > > 
> > > It fails, then drops MKFS_OPTIONS.  The simple solution for this is
> > > to simply to use a larger external log device....
> > > 
> > > > Is this one of those cases where _scratch_mkfs notices the mkfs failure
> > > > and formats without MKFS_OPTIONS? 
> > > 
> > > More than likely.
> > > 
> > > > And if that's true, what about my
> > > > test configs that set MKFS_OPTIONS to test new non-default features?
> > > 
> > > Changing existing infrastructure behaviour to better suit *your*
> > > test environments is *your* responsibility to address, not mine.
> > > I don't care if MKFS_OPTIONS get dropped in occasional tests, it's
> > > more important to me that the tests run fast so I can iterate
> > > my modify-build-test development cycle faster.
> > 
> > I don't agree that your personal development velocity is more important
> > than silent loss of code coverage of everyone who happens to be testing
> > external log devices.  That at least covers myself, our QA department,
> > and everyone who uses the prepackaged configurations in kdevops and
> > xfstests-bld.
> > 
> > > It should be trivial for you to address, though. Add a function
> > > like:
> > > 
> > > _scratch_mkfs_try_option "-l size=256M" 
> > 
> > Or instead:
> > 
> > # Pushing 50000 unlinked inode inactivations through a small xfs log can
> > # result in bottlenecks on the log grant heads, so try to make the log
> > # larger to reduce runtime.
> > if [ $FSTYP = xfs ] && [ -z "$SCRATCH_LOGDEV ]; then
> > 	_scratch_mkfs -l size=256M
> 
> Maybe better to use "_has_logdev", due to there's "$USE_EXTERNAL".
> I can change this patch as:
> 
>   # Pushing 50000 unlinked inode inactivations through a small xfs log can
>   # result in bottlenecks on the log grant heads, so try to make the log
>   # larger to reduce runtime.
>   if [ "$FSTYP" = "xfs" ] && _has_logdev; then

Sorry, should be "! _has_logdev".

>   	_scratch_mkfs -l size=256M
>   else
>   	_scratch_mkfs
>   fi
> 
> when I merge it, if you all agree :)
> 
> Thanks,
> Zorro
> 
> > else
> > 	_scratch_mkfs
> > fi
> > 
> > > which has the opposite fallback behaviour of dropping the test
> > > supplied option and using MKFS_OPTIONS instead, and I'll use it for
> > > all these "test go faster" modifications that we badly need to
> > > address...
> > > 
> > > FWIW, this would also get rid of the need for the FSTYP checks in
> > > the test, too, because passing "-l size=256M" will fail on btrfs,
> > > ext4, etc and then they fall back to the specific test config...
> > 
> > Huh?  And what happens when btrfs want their own try option?
> > 
> > > So, provide me with the infrastructure that makes stuff like this
> > > work properly in *your test environment*, and I'll use it
> > > appropriately.
> > 
> > Dave.
> > 
> > You're the one who made the changes to generic/53*.  It worked before.
> > 
> > Like any other code change, it's /your/ responsibility to try not to
> > cause regressions for everyone else using the same code.  If other
> > people have problems with a change, it's longstanding community
> > expectation that the author should help resolve the problem.
> > 
> > It is /not/ standard practice to lecture the affected people about how
> > they should embark on a treewide change to fix a problem that didn't
> > exist previously.  The maintainer reverts the offending commit and the
> > author redrafts the patch.  You're well aware of this, because you've
> > said as much about other patches (e.g. iomap zeroing cleanups).
> > 
> > It is not everyone else's (Ted, Christoph, Zorro) role to clean up after
> > you.
> > 
> > I do not appreciate the condescending behavior you have shown towards me
> > this year and last.  Next year I will take more steps to insulate my
> > team from you and other community members as needed because I don't want
> > them to continue to deal with such conduct[1][2].
> > 
> > --D
> > 
> > [1] https://lore.kernel.org/fstests/Z0pBKWBlXLBxwK3D@dread.disaster.area/
> > [2] https://lore.kernel.org/linux-xfs/Z1FQdYEXLR5BoOE-@redhat.com/
> > 
> > > -Dave.
> > > -- 
> > > Dave Chinner
> > > david@fromorbit.com
> >
diff mbox series

Patch

diff --git a/tests/generic/530 b/tests/generic/530
index 18256b870..06d7532f0 100755
--- a/tests/generic/530
+++ b/tests/generic/530
@@ -22,7 +22,11 @@  _require_scratch_shutdown
 _require_metadata_journaling
 _require_test_program "t_open_tmpfiles"
 
-_scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
+if [ $FSTYP = "xfs" ]; then
+    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
+else
+    _scratch_mkfs >> $seqres.full 2>&1
+fi
 _scratch_mount
 
 # Set ULIMIT_NOFILE to min(file-max / 2, 50000 files per LOAD_FACTOR)