| Message ID | 20220520143249.2103631-1-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: annotate fix commits for upcoming 5.10.y backports | expand |
On Fri, May 20, 2022 at 05:32:49PM +0300, Amir Goldstein wrote: > In preparation for backporting xfs fixes to stable kernel 5.10.y, > annotate some of the tests that pass after applying the backports. > > Most of the annotated tests have the fix commit documented either > in comment or in commit message already. > > All tests have been verified to pass with fix commits apply, but > for a few tests, a failure was observed when running on kernel without > the documented fix commit. That is probably because failure happens > only on a specific setup. > > Generic tests have also been annotated with xfs fix commits. > That may produce wrong hints if the test fails on another fs, but > that is what hints are for - to give tester a hint, so if tester is > not testing xfs, it's easy to figure out that the hint is irrelevant. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > > Hi Zorro, > > Here are a bunch of fixed_by annotations for xfs bug fixes, > which I am in the process of testing for stable kernel v5.10.y [1]. I just confirmed that these commit IDs are right, and these cases are specific reproducer for them. So I'd like to ack this patch. Reviewed-by: Zorro Lang <zlang@redhat.com> > > There are a lot more tests with fix commits documented in comments > and/or commit message, but I did not annotate any test that I did not > verify myself to pass with the fix commit. Thanks for you did that such carefully. You remind me that I just release a new fstests version v2022.05.22, which has several known failures too. https://lore.kernel.org/fstests/20220522094622.25751C385AA@smtp.kernel.org/T/#u I'm thinking about another question, if a case covers more and more known issue, the known list will be very long, even might take 1/3 of the code lines. That will look not graceful ... We'd better to not make an existed case grow biger and biger. And don't record known issues in a case which just can reproduce it with small chance randomly. Likes you record e4826691cc7e ("xfs: restore shutdown check in mapped write fault path") into g/623, actually we found it by g/019 at first, then write g/623 to cover that. So g/019 isn't the recommended case to record this issue. Thanks, Zorro > > If tests were not documented in kernel commit message nor mentioned > in the mailing list correspondance and if kernel commit or subject were > not mentioned in fstests commit message or comments, then I may have > missed those secret tests. > > Thanks, > Amir. > > [1] https://github.com/amir73il/linux/commits/xfs-5.10.y > > tests/generic/623 | 3 +++ > tests/generic/631 | 2 ++ > tests/generic/646 | 3 +++ > tests/generic/649 | 3 +++ > tests/xfs/145 | 3 +++ > tests/xfs/177 | 2 ++ > tests/xfs/513 | 3 +++ > tests/xfs/539 | 7 +++++-- > tests/xfs/542 | 3 +++ > 9 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/tests/generic/623 b/tests/generic/623 > index 324251b7..ea016d91 100755 > --- a/tests/generic/623 > +++ b/tests/generic/623 > @@ -12,6 +12,9 @@ _begin_fstest auto quick shutdown > . ./common/filter > > _supported_fs generic > +_fixed_by_kernel_commit e4826691cc7e \ > + "xfs: restore shutdown check in mapped write fault path" > + > _require_scratch_nocheck > _require_scratch_shutdown > > diff --git a/tests/generic/631 b/tests/generic/631 > index 219f7a05..ff1bb113 100755 > --- a/tests/generic/631 > +++ b/tests/generic/631 > @@ -40,6 +40,8 @@ _require_scratch > _require_attrs trusted > _supported_fs ^overlay > _require_extra_fs overlay > +_fixed_by_kernel_commit 6da1b4b1ab36 \ > + "xfs: fix an ABBA deadlock in xfs_rename" > > _scratch_mkfs >> $seqres.full > _scratch_mount > diff --git a/tests/generic/646 b/tests/generic/646 > index 79d3f17c..027df557 100755 > --- a/tests/generic/646 > +++ b/tests/generic/646 > @@ -15,6 +15,9 @@ > _begin_fstest auto quick recoveryloop shutdown > > # real QA test starts here > +_supported_fs generic > +_fixed_by_kernel_commit 50d25484bebe \ > + "xfs: sync lazy sb accounting on quiesce of read-only mounts" > > _require_scratch > _require_scratch_shutdown > diff --git a/tests/generic/649 b/tests/generic/649 > index a6aabfaf..d6727765 100755 > --- a/tests/generic/649 > +++ b/tests/generic/649 > @@ -33,6 +33,9 @@ _cleanup() > > # Modify as appropriate. > _supported_fs generic > +_fixed_by_kernel_commit 72a048c1056a \ > + "xfs: only set IOMAP_F_SHARED when providing a srcmap to a write" > + > _require_cp_reflink > _require_test_reflink > _require_test_program "punch-alternating" > diff --git a/tests/xfs/145 b/tests/xfs/145 > index d32e726e..5fd8dcad 100755 > --- a/tests/xfs/145 > +++ b/tests/xfs/145 > @@ -18,6 +18,9 @@ _begin_fstest auto quick quota > > # real QA test starts here > _supported_fs xfs > +_fixed_by_kernel_commit 1aecf3734a95 \ > + "xfs: fix chown leaking delalloc quota blocks when fssetxattr fails" > + > _require_command "$FILEFRAG_PROG" filefrag > _require_test_program "chprojid_fail" > _require_quota > diff --git a/tests/xfs/177 b/tests/xfs/177 > index 10891550..1e59bd6c 100755 > --- a/tests/xfs/177 > +++ b/tests/xfs/177 > @@ -39,6 +39,8 @@ _cleanup() > > # Modify as appropriate. > _supported_fs xfs > +_fixed_by_kernel_commit f38a032b165d "xfs: fix I_DONTCACHE" > + > _require_xfs_io_command "bulkstat" > _require_scratch > > diff --git a/tests/xfs/513 b/tests/xfs/513 > index bfdfd4f6..85500af0 100755 > --- a/tests/xfs/513 > +++ b/tests/xfs/513 > @@ -31,6 +31,9 @@ _cleanup() > > # real QA test starts here > _supported_fs xfs > +_fixed_by_kernel_commit 237d7887ae72 \ > + "xfs: show the proper user quota options" > + > _require_test > _require_loop > _require_xfs_io_command "falloc" > diff --git a/tests/xfs/539 b/tests/xfs/539 > index 4bc52c1a..77b44c89 100755 > --- a/tests/xfs/539 > +++ b/tests/xfs/539 > @@ -9,15 +9,18 @@ > # the same value as during the mount > # > # Regression test for commit: > -# xfs: Skip repetitive warnings about mount options > +# 92cf7d36384b xfs: Skip repetitive warnings about mount options > > . ./common/preamble > _begin_fstest auto quick mount > > # Import common functions. > > -_require_check_dmesg > _supported_fs xfs > +_fixed_by_kernel_commit 92cf7d36384b \ > + "xfs: Skip repetitive warnings about mount options" > + > +_require_check_dmesg > _require_scratch > > log_tag() > diff --git a/tests/xfs/542 b/tests/xfs/542 > index 5c45eed7..1540541e 100755 > --- a/tests/xfs/542 > +++ b/tests/xfs/542 > @@ -26,6 +26,9 @@ _cleanup() > > # real QA test starts here > _supported_fs xfs > +_fixed_by_kernel_commit 5ca5916b6bc9 \ > + "xfs: punch out data fork delalloc blocks on COW writeback failure" > + > _require_scratch_reflink > _require_cp_reflink > _require_xfs_io_command "cowextsize" > -- > 2.25.1 >
On Sun, May 22, 2022 at 5:10 PM Zorro Lang <zlang@redhat.com> wrote: > > On Fri, May 20, 2022 at 05:32:49PM +0300, Amir Goldstein wrote: > > In preparation for backporting xfs fixes to stable kernel 5.10.y, > > annotate some of the tests that pass after applying the backports. > > > > Most of the annotated tests have the fix commit documented either > > in comment or in commit message already. > > > > All tests have been verified to pass with fix commits apply, but > > for a few tests, a failure was observed when running on kernel without > > the documented fix commit. That is probably because failure happens > > only on a specific setup. > > > > Generic tests have also been annotated with xfs fix commits. > > That may produce wrong hints if the test fails on another fs, but > > that is what hints are for - to give tester a hint, so if tester is > > not testing xfs, it's easy to figure out that the hint is irrelevant. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > > > Hi Zorro, > > > > Here are a bunch of fixed_by annotations for xfs bug fixes, > > which I am in the process of testing for stable kernel v5.10.y [1]. > > I just confirmed that these commit IDs are right, and these cases are > specific reproducer for them. So I'd like to ack this patch. > > Reviewed-by: Zorro Lang <zlang@redhat.com> > > > > > There are a lot more tests with fix commits documented in comments > > and/or commit message, but I did not annotate any test that I did not > > verify myself to pass with the fix commit. > > Thanks for you did that such carefully. You remind me that I just release > a new fstests version v2022.05.22, which has several known failures too. > https://lore.kernel.org/fstests/20220522094622.25751C385AA@smtp.kernel.org/T/#u > > I'm thinking about another question, if a case covers more and more known > issue, the known list will be very long, even might take 1/3 of the code > lines. That will look not graceful ... > > We'd better to not make an existed case grow biger and biger. And don't > record known issues in a case which just can reproduce it with small chance No of course not, that would not be productive. fixed_by_commit should be reserved to "proper" regression tests failing before fix, passing after fix There could still be several fixed_by commits for different fs and for different regressions that were detected on different kernel versions. > randomly. Likes you record e4826691cc7e ("xfs: restore shutdown check in mapped > write fault path") into g/623, actually we found it by g/019 at first, then > write g/623 to cover that. So g/019 isn't the recommended case to record this > issue. > Exactly. Good example. Thanks, Amir.
diff --git a/tests/generic/623 b/tests/generic/623 index 324251b7..ea016d91 100755 --- a/tests/generic/623 +++ b/tests/generic/623 @@ -12,6 +12,9 @@ _begin_fstest auto quick shutdown . ./common/filter _supported_fs generic +_fixed_by_kernel_commit e4826691cc7e \ + "xfs: restore shutdown check in mapped write fault path" + _require_scratch_nocheck _require_scratch_shutdown diff --git a/tests/generic/631 b/tests/generic/631 index 219f7a05..ff1bb113 100755 --- a/tests/generic/631 +++ b/tests/generic/631 @@ -40,6 +40,8 @@ _require_scratch _require_attrs trusted _supported_fs ^overlay _require_extra_fs overlay +_fixed_by_kernel_commit 6da1b4b1ab36 \ + "xfs: fix an ABBA deadlock in xfs_rename" _scratch_mkfs >> $seqres.full _scratch_mount diff --git a/tests/generic/646 b/tests/generic/646 index 79d3f17c..027df557 100755 --- a/tests/generic/646 +++ b/tests/generic/646 @@ -15,6 +15,9 @@ _begin_fstest auto quick recoveryloop shutdown # real QA test starts here +_supported_fs generic +_fixed_by_kernel_commit 50d25484bebe \ + "xfs: sync lazy sb accounting on quiesce of read-only mounts" _require_scratch _require_scratch_shutdown diff --git a/tests/generic/649 b/tests/generic/649 index a6aabfaf..d6727765 100755 --- a/tests/generic/649 +++ b/tests/generic/649 @@ -33,6 +33,9 @@ _cleanup() # Modify as appropriate. _supported_fs generic +_fixed_by_kernel_commit 72a048c1056a \ + "xfs: only set IOMAP_F_SHARED when providing a srcmap to a write" + _require_cp_reflink _require_test_reflink _require_test_program "punch-alternating" diff --git a/tests/xfs/145 b/tests/xfs/145 index d32e726e..5fd8dcad 100755 --- a/tests/xfs/145 +++ b/tests/xfs/145 @@ -18,6 +18,9 @@ _begin_fstest auto quick quota # real QA test starts here _supported_fs xfs +_fixed_by_kernel_commit 1aecf3734a95 \ + "xfs: fix chown leaking delalloc quota blocks when fssetxattr fails" + _require_command "$FILEFRAG_PROG" filefrag _require_test_program "chprojid_fail" _require_quota diff --git a/tests/xfs/177 b/tests/xfs/177 index 10891550..1e59bd6c 100755 --- a/tests/xfs/177 +++ b/tests/xfs/177 @@ -39,6 +39,8 @@ _cleanup() # Modify as appropriate. _supported_fs xfs +_fixed_by_kernel_commit f38a032b165d "xfs: fix I_DONTCACHE" + _require_xfs_io_command "bulkstat" _require_scratch diff --git a/tests/xfs/513 b/tests/xfs/513 index bfdfd4f6..85500af0 100755 --- a/tests/xfs/513 +++ b/tests/xfs/513 @@ -31,6 +31,9 @@ _cleanup() # real QA test starts here _supported_fs xfs +_fixed_by_kernel_commit 237d7887ae72 \ + "xfs: show the proper user quota options" + _require_test _require_loop _require_xfs_io_command "falloc" diff --git a/tests/xfs/539 b/tests/xfs/539 index 4bc52c1a..77b44c89 100755 --- a/tests/xfs/539 +++ b/tests/xfs/539 @@ -9,15 +9,18 @@ # the same value as during the mount # # Regression test for commit: -# xfs: Skip repetitive warnings about mount options +# 92cf7d36384b xfs: Skip repetitive warnings about mount options . ./common/preamble _begin_fstest auto quick mount # Import common functions. -_require_check_dmesg _supported_fs xfs +_fixed_by_kernel_commit 92cf7d36384b \ + "xfs: Skip repetitive warnings about mount options" + +_require_check_dmesg _require_scratch log_tag() diff --git a/tests/xfs/542 b/tests/xfs/542 index 5c45eed7..1540541e 100755 --- a/tests/xfs/542 +++ b/tests/xfs/542 @@ -26,6 +26,9 @@ _cleanup() # real QA test starts here _supported_fs xfs +_fixed_by_kernel_commit 5ca5916b6bc9 \ + "xfs: punch out data fork delalloc blocks on COW writeback failure" + _require_scratch_reflink _require_cp_reflink _require_xfs_io_command "cowextsize"
In preparation for backporting xfs fixes to stable kernel 5.10.y, annotate some of the tests that pass after applying the backports. Most of the annotated tests have the fix commit documented either in comment or in commit message already. All tests have been verified to pass with fix commits apply, but for a few tests, a failure was observed when running on kernel without the documented fix commit. That is probably because failure happens only on a specific setup. Generic tests have also been annotated with xfs fix commits. That may produce wrong hints if the test fails on another fs, but that is what hints are for - to give tester a hint, so if tester is not testing xfs, it's easy to figure out that the hint is irrelevant. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- Hi Zorro, Here are a bunch of fixed_by annotations for xfs bug fixes, which I am in the process of testing for stable kernel v5.10.y [1]. There are a lot more tests with fix commits documented in comments and/or commit message, but I did not annotate any test that I did not verify myself to pass with the fix commit. If tests were not documented in kernel commit message nor mentioned in the mailing list correspondance and if kernel commit or subject were not mentioned in fstests commit message or comments, then I may have missed those secret tests. Thanks, Amir. [1] https://github.com/amir73il/linux/commits/xfs-5.10.y tests/generic/623 | 3 +++ tests/generic/631 | 2 ++ tests/generic/646 | 3 +++ tests/generic/649 | 3 +++ tests/xfs/145 | 3 +++ tests/xfs/177 | 2 ++ tests/xfs/513 | 3 +++ tests/xfs/539 | 7 +++++-- tests/xfs/542 | 3 +++ 9 files changed, 27 insertions(+), 2 deletions(-)