| Message ID | 20200114125044.21594-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: btrfs/153: Remove it from auto group | expand |
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 >
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.
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. >
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.
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
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(-)