* [PATCH] fstests: btrfs/153: Remove it from auto group
@ 2020-01-14 12:50 Qu Wenruo
2020-02-01 7:36 ` Eryu Guan
0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2020-01-14 12:50 UTC (permalink / raw)
To: fstests, linux-btrfs
This test case always fail after commit c6887cd11149 ("Btrfs: don't do
nocow check unless we have to").
As btrfs no longer checks nodatacow at buffered write time.
That commits brings in a big performance enhancement, as that check is
not cheap, but breaks qgroup, as write into preallocated space now needs
extra space.
There isn't yet a good solution (reverting that patch is not possible,
and only check nodatacow for quota enabled case is very bug prune due to
quite a lot code change).
We may solve it using the new ticketed space reservation facility, but
that won't come into fruit anytime soon.
So let's just remove that test case from 'auto' group, but still keep
the test case to inform we still have a lot of work to do.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
tests/btrfs/group | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 697b6a38ea00..3c554a194742 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -155,7 +155,7 @@
150 auto quick dangerous
151 auto quick volume
152 auto quick metadata qgroup send
-153 auto quick qgroup limit
+153 quick qgroup limit
154 auto quick volume
155 auto quick send
156 auto quick trim
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] fstests: btrfs/153: Remove it from auto group 2020-01-14 12:50 [PATCH] fstests: btrfs/153: Remove it from auto group Qu Wenruo @ 2020-02-01 7:36 ` Eryu Guan 2020-02-01 11:10 ` Amir Goldstein 0 siblings, 1 reply; 5+ messages in thread From: Eryu Guan @ 2020-02-01 7:36 UTC (permalink / raw) To: Qu Wenruo; +Cc: fstests, linux-btrfs On Tue, Jan 14, 2020 at 08:50:44PM +0800, Qu Wenruo wrote: > This test case always fail after commit c6887cd11149 ("Btrfs: don't do > nocow check unless we have to"). > As btrfs no longer checks nodatacow at buffered write time. > > That commits brings in a big performance enhancement, as that check is > not cheap, but breaks qgroup, as write into preallocated space now needs > extra space. > > There isn't yet a good solution (reverting that patch is not possible, > and only check nodatacow for quota enabled case is very bug prune due to > quite a lot code change). > > We may solve it using the new ticketed space reservation facility, but > that won't come into fruit anytime soon. > > So let's just remove that test case from 'auto' group, but still keep > the test case to inform we still have a lot of work to do. > > Signed-off-by: Qu Wenruo <wqu@suse.com> I'd like to see an ACK from btrfs folks. Thanks! Eryu > --- > tests/btrfs/group | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/btrfs/group b/tests/btrfs/group > index 697b6a38ea00..3c554a194742 100644 > --- a/tests/btrfs/group > +++ b/tests/btrfs/group > @@ -155,7 +155,7 @@ > 150 auto quick dangerous > 151 auto quick volume > 152 auto quick metadata qgroup send > -153 auto quick qgroup limit > +153 quick qgroup limit > 154 auto quick volume > 155 auto quick send > 156 auto quick trim > -- > 2.24.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fstests: btrfs/153: Remove it from auto group 2020-02-01 7:36 ` Eryu Guan @ 2020-02-01 11:10 ` Amir Goldstein 2020-02-01 12:38 ` Qu Wenruo 0 siblings, 1 reply; 5+ messages in thread From: Amir Goldstein @ 2020-02-01 11:10 UTC (permalink / raw) To: Eryu Guan; +Cc: Qu Wenruo, fstests, Linux Btrfs On Sat, Feb 1, 2020 at 9:41 AM Eryu Guan <guaneryu@gmail.com> wrote: > > On Tue, Jan 14, 2020 at 08:50:44PM +0800, Qu Wenruo wrote: > > This test case always fail after commit c6887cd11149 ("Btrfs: don't do > > nocow check unless we have to"). > > As btrfs no longer checks nodatacow at buffered write time. > > > > That commits brings in a big performance enhancement, as that check is > > not cheap, but breaks qgroup, as write into preallocated space now needs > > extra space. > > > > There isn't yet a good solution (reverting that patch is not possible, > > and only check nodatacow for quota enabled case is very bug prune due to > > quite a lot code change). > > > > We may solve it using the new ticketed space reservation facility, but > > that won't come into fruit anytime soon. > > > > So let's just remove that test case from 'auto' group, but still keep > > the test case to inform we still have a lot of work to do. > > > > Signed-off-by: Qu Wenruo <wqu@suse.com> > > I'd like to see an ACK from btrfs folks. Thanks! > > Eryu > > > --- > > tests/btrfs/group | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/btrfs/group b/tests/btrfs/group > > index 697b6a38ea00..3c554a194742 100644 > > --- a/tests/btrfs/group > > +++ b/tests/btrfs/group > > @@ -155,7 +155,7 @@ > > 150 auto quick dangerous > > 151 auto quick volume > > 152 auto quick metadata qgroup send > > -153 auto quick qgroup limit > > +153 quick qgroup limit Hmm, if removing from auto it might make sense to also remove it from quick, because people often use quick as a sanity regression group. The issue at hand is a recurring pattern. It is also been discussed recently about generic/484: https://lore.kernel.org/fstests/20200131164619.GA13005@infradead.org/ I also handled something like this with: fdb69864 overlay/061: remove from auto and quick groups I suggest adding a group 'broken' to mark known/wontfix issues then a default regression test could run -g auto -x broken or -g quick -x broken for a quick regression sanity. Thoughts? Amir. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fstests: btrfs/153: Remove it from auto group 2020-02-01 11:10 ` Amir Goldstein @ 2020-02-01 12:38 ` Qu Wenruo 2020-02-01 13:07 ` Amir Goldstein 0 siblings, 1 reply; 5+ messages in thread From: Qu Wenruo @ 2020-02-01 12:38 UTC (permalink / raw) To: Amir Goldstein, Eryu Guan; +Cc: fstests, Linux Btrfs On 2020/2/1 下午7:10, Amir Goldstein wrote: > On Sat, Feb 1, 2020 at 9:41 AM Eryu Guan <guaneryu@gmail.com> wrote: >> >> On Tue, Jan 14, 2020 at 08:50:44PM +0800, Qu Wenruo wrote: >>> This test case always fail after commit c6887cd11149 ("Btrfs: don't do >>> nocow check unless we have to"). >>> As btrfs no longer checks nodatacow at buffered write time. >>> >>> That commits brings in a big performance enhancement, as that check is >>> not cheap, but breaks qgroup, as write into preallocated space now needs >>> extra space. >>> >>> There isn't yet a good solution (reverting that patch is not possible, >>> and only check nodatacow for quota enabled case is very bug prune due to >>> quite a lot code change). >>> >>> We may solve it using the new ticketed space reservation facility, but >>> that won't come into fruit anytime soon. >>> >>> So let's just remove that test case from 'auto' group, but still keep >>> the test case to inform we still have a lot of work to do. >>> >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >> >> I'd like to see an ACK from btrfs folks. Thanks! >> >> Eryu >> >>> --- >>> tests/btrfs/group | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/btrfs/group b/tests/btrfs/group >>> index 697b6a38ea00..3c554a194742 100644 >>> --- a/tests/btrfs/group >>> +++ b/tests/btrfs/group >>> @@ -155,7 +155,7 @@ >>> 150 auto quick dangerous >>> 151 auto quick volume >>> 152 auto quick metadata qgroup send >>> -153 auto quick qgroup limit >>> +153 quick qgroup limit > > Hmm, if removing from auto it might make sense to also remove it > from quick, because people often use quick as a sanity regression group. That's also one of my concern. However recently I tend to run more same VMs on different ranges of fstests to speed up the testing progress other than using 'quick' group. Anyway this depends on the end users (QA and developers). > > The issue at hand is a recurring pattern. > It is also been discussed recently about generic/484: > https://lore.kernel.org/fstests/20200131164619.GA13005@infradead.org/ > > I also handled something like this with: > fdb69864 overlay/061: remove from auto and quick groups > > I suggest adding a group 'broken' to mark known/wontfix issues > then a default regression test could run -g auto -x broken > or -g quick -x broken for a quick regression sanity. That's much better. Just one question, if a test is in both quick and broken group, will -g quick -x broken still exclude it? Thanks, Qu > > Thoughts? > > Amir. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fstests: btrfs/153: Remove it from auto group 2020-02-01 12:38 ` Qu Wenruo @ 2020-02-01 13:07 ` Amir Goldstein 0 siblings, 0 replies; 5+ messages in thread From: Amir Goldstein @ 2020-02-01 13:07 UTC (permalink / raw) To: Qu Wenruo; +Cc: Eryu Guan, fstests, Linux Btrfs On Sat, Feb 1, 2020 at 2:39 PM Qu Wenruo <wqu@suse.com> wrote: > > > > On 2020/2/1 下午7:10, Amir Goldstein wrote: > > On Sat, Feb 1, 2020 at 9:41 AM Eryu Guan <guaneryu@gmail.com> wrote: > >> > >> On Tue, Jan 14, 2020 at 08:50:44PM +0800, Qu Wenruo wrote: > >>> This test case always fail after commit c6887cd11149 ("Btrfs: don't do > >>> nocow check unless we have to"). > >>> As btrfs no longer checks nodatacow at buffered write time. > >>> > >>> That commits brings in a big performance enhancement, as that check is > >>> not cheap, but breaks qgroup, as write into preallocated space now needs > >>> extra space. > >>> > >>> There isn't yet a good solution (reverting that patch is not possible, > >>> and only check nodatacow for quota enabled case is very bug prune due to > >>> quite a lot code change). > >>> > >>> We may solve it using the new ticketed space reservation facility, but > >>> that won't come into fruit anytime soon. > >>> > >>> So let's just remove that test case from 'auto' group, but still keep > >>> the test case to inform we still have a lot of work to do. > >>> > >>> Signed-off-by: Qu Wenruo <wqu@suse.com> > >> > >> I'd like to see an ACK from btrfs folks. Thanks! > >> > >> Eryu > >> > >>> --- > >>> tests/btrfs/group | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/tests/btrfs/group b/tests/btrfs/group > >>> index 697b6a38ea00..3c554a194742 100644 > >>> --- a/tests/btrfs/group > >>> +++ b/tests/btrfs/group > >>> @@ -155,7 +155,7 @@ > >>> 150 auto quick dangerous > >>> 151 auto quick volume > >>> 152 auto quick metadata qgroup send > >>> -153 auto quick qgroup limit > >>> +153 quick qgroup limit > > > > Hmm, if removing from auto it might make sense to also remove it > > from quick, because people often use quick as a sanity regression group. > > That's also one of my concern. > > However recently I tend to run more same VMs on different ranges of > fstests to speed up the testing progress other than using 'quick' group. > > Anyway this depends on the end users (QA and developers). > > > > > The issue at hand is a recurring pattern. > > It is also been discussed recently about generic/484: > > https://lore.kernel.org/fstests/20200131164619.GA13005@infradead.org/ > > > > I also handled something like this with: > > fdb69864 overlay/061: remove from auto and quick groups > > > > I suggest adding a group 'broken' to mark known/wontfix issues > > then a default regression test could run -g auto -x broken > > or -g quick -x broken for a quick regression sanity. > > That's much better. > > Just one question, if a test is in both quick and broken group, will -g > quick -x broken still exclude it? > Yes. That is the main use case for -x. Often used with -x dangerous. What we don't have, AFAIK, is the ability to request 'auto && quick'. Thanks, Amir. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-01 13:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-14 12:50 [PATCH] fstests: btrfs/153: Remove it from auto group Qu Wenruo 2020-02-01 7:36 ` Eryu Guan 2020-02-01 11:10 ` Amir Goldstein 2020-02-01 12:38 ` Qu Wenruo 2020-02-01 13:07 ` Amir Goldstein
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).