* [PATCH] fstests: generic/500 doesn't work for btrfs @ 2019-08-15 18:26 Josef Bacik 2019-08-16 2:16 ` Darrick J. Wong 2019-08-18 15:44 ` Eryu Guan 0 siblings, 2 replies; 4+ messages in thread From: Josef Bacik @ 2019-08-15 18:26 UTC (permalink / raw) To: fstests, linux-btrfs, kernel-team Btrfs does COW, so when we unlink the file we need to update metadata and write it to a new location, which we can't do because the thinp is full. This results in an EIO during a metadata write, which makes us flip read only, thus making it impossible to fstrim the fs. Just make it so we skip this test for btrfs. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- tests/generic/500 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/generic/500 b/tests/generic/500 index 201d8b9f..5cd7126f 100755 --- a/tests/generic/500 +++ b/tests/generic/500 @@ -49,6 +49,12 @@ _supported_os Linux _require_scratch_nocheck _require_dm_target thin-pool +# The unlink below will result in new metadata blocks for btrfs because of CoW, +# and since we've filled the thinp device it'll return EIO, which will make +# btrfs flip read only, making it fail this test when it just won't work right +# for us in the first place. +test $FSTYP == "btrfs" && _notrun "btrfs doesn't work that way lol" + # Require underlying device support discard _scratch_mkfs >>$seqres.full 2>&1 _scratch_mount -- 2.21.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fstests: generic/500 doesn't work for btrfs 2019-08-15 18:26 [PATCH] fstests: generic/500 doesn't work for btrfs Josef Bacik @ 2019-08-16 2:16 ` Darrick J. Wong 2019-08-18 15:44 ` Eryu Guan 1 sibling, 0 replies; 4+ messages in thread From: Darrick J. Wong @ 2019-08-16 2:16 UTC (permalink / raw) To: Josef Bacik; +Cc: fstests, linux-btrfs, kernel-team On Thu, Aug 15, 2019 at 02:26:59PM -0400, Josef Bacik wrote: > Btrfs does COW, so when we unlink the file we need to update metadata > and write it to a new location, which we can't do because the thinp is > full. This results in an EIO during a metadata write, which makes us > flip read only, thus making it impossible to fstrim the fs. Just make > it so we skip this test for btrfs. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > tests/generic/500 | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tests/generic/500 b/tests/generic/500 > index 201d8b9f..5cd7126f 100755 > --- a/tests/generic/500 > +++ b/tests/generic/500 > @@ -49,6 +49,12 @@ _supported_os Linux > _require_scratch_nocheck > _require_dm_target thin-pool > > +# The unlink below will result in new metadata blocks for btrfs because of CoW, > +# and since we've filled the thinp device it'll return EIO, which will make > +# btrfs flip read only, making it fail this test when it just won't work right > +# for us in the first place. > +test $FSTYP == "btrfs" && _notrun "btrfs doesn't work that way lol" I did it for the lulz, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > + > # Require underlying device support discard > _scratch_mkfs >>$seqres.full 2>&1 > _scratch_mount > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fstests: generic/500 doesn't work for btrfs 2019-08-15 18:26 [PATCH] fstests: generic/500 doesn't work for btrfs Josef Bacik 2019-08-16 2:16 ` Darrick J. Wong @ 2019-08-18 15:44 ` Eryu Guan 2019-08-19 16:25 ` Darrick J. Wong 1 sibling, 1 reply; 4+ messages in thread From: Eryu Guan @ 2019-08-18 15:44 UTC (permalink / raw) To: Josef Bacik; +Cc: fstests, linux-btrfs, kernel-team, Darrick J. Wong On Thu, Aug 15, 2019 at 02:26:59PM -0400, Josef Bacik wrote: > Btrfs does COW, so when we unlink the file we need to update metadata > and write it to a new location, which we can't do because the thinp is > full. This results in an EIO during a metadata write, which makes us > flip read only, thus making it impossible to fstrim the fs. Just make > it so we skip this test for btrfs. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > tests/generic/500 | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tests/generic/500 b/tests/generic/500 > index 201d8b9f..5cd7126f 100755 > --- a/tests/generic/500 > +++ b/tests/generic/500 > @@ -49,6 +49,12 @@ _supported_os Linux > _require_scratch_nocheck > _require_dm_target thin-pool > > +# The unlink below will result in new metadata blocks for btrfs because of CoW, > +# and since we've filled the thinp device it'll return EIO, which will make > +# btrfs flip read only, making it fail this test when it just won't work right > +# for us in the first place. > +test $FSTYP == "btrfs" && _notrun "btrfs doesn't work that way lol" I'm wondering if we could introduce a proper _require rule to cover this case? e.g. require the fs doesn't allocate new blocks on unlink? or something like that. But I'm not sure what's the proper fs feature to require here, any suggestions? Thanks, Eryu > + > # Require underlying device support discard > _scratch_mkfs >>$seqres.full 2>&1 > _scratch_mount > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fstests: generic/500 doesn't work for btrfs 2019-08-18 15:44 ` Eryu Guan @ 2019-08-19 16:25 ` Darrick J. Wong 0 siblings, 0 replies; 4+ messages in thread From: Darrick J. Wong @ 2019-08-19 16:25 UTC (permalink / raw) To: Eryu Guan; +Cc: Josef Bacik, fstests, linux-btrfs, kernel-team On Sun, Aug 18, 2019 at 11:44:28PM +0800, Eryu Guan wrote: > On Thu, Aug 15, 2019 at 02:26:59PM -0400, Josef Bacik wrote: > > Btrfs does COW, so when we unlink the file we need to update metadata > > and write it to a new location, which we can't do because the thinp is > > full. This results in an EIO during a metadata write, which makes us > > flip read only, thus making it impossible to fstrim the fs. Just make > > it so we skip this test for btrfs. > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > --- > > tests/generic/500 | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/tests/generic/500 b/tests/generic/500 > > index 201d8b9f..5cd7126f 100755 > > --- a/tests/generic/500 > > +++ b/tests/generic/500 > > @@ -49,6 +49,12 @@ _supported_os Linux > > _require_scratch_nocheck > > _require_dm_target thin-pool > > > > +# The unlink below will result in new metadata blocks for btrfs because of CoW, > > +# and since we've filled the thinp device it'll return EIO, which will make > > +# btrfs flip read only, making it fail this test when it just won't work right > > +# for us in the first place. > > +test $FSTYP == "btrfs" && _notrun "btrfs doesn't work that way lol" > > I'm wondering if we could introduce a proper _require rule to cover this > case? e.g. require the fs doesn't allocate new blocks on unlink? or > something like that. But I'm not sure what's the proper fs feature to > require here, any suggestions? I'd be careful with this -- xfs can allocate new metadata blocks on unlink too -- changes in the free space btrees, expansion of the free inode btree, etc. For the 20 inodes in play in g/500 this won't be the case, but if you had a test that created 20,000 inodes, then that could happen. --D > Thanks, > Eryu > > > + > > # Require underlying device support discard > > _scratch_mkfs >>$seqres.full 2>&1 > > _scratch_mount > > -- > > 2.21.0 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-19 16:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-15 18:26 [PATCH] fstests: generic/500 doesn't work for btrfs Josef Bacik 2019-08-16 2:16 ` Darrick J. Wong 2019-08-18 15:44 ` Eryu Guan 2019-08-19 16:25 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).