Message ID | 167096074260.1750519.311637326793150150.stgit@magnolia (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | fstests: fix tests for kernel 6.1 | expand |
On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Adjust this test since made EFI/EFD log item format structs proper flex > arrays instead of array[1]. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- So we let this case fail on all older system/kernel? Is the kernel patch recommended to be merged on downstream kernel? Thanks, Zorro > tests/xfs/122.out | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out > index a56cbee84f..95e53c5081 100644 > --- a/tests/xfs/122.out > +++ b/tests/xfs/122.out > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104 > sizeof(xfs_dq_logformat_t) = 24 > sizeof(xfs_dqblk_t) = 136 > sizeof(xfs_dsb_t) = 264 > -sizeof(xfs_efd_log_format_32_t) = 28 > -sizeof(xfs_efd_log_format_64_t) = 32 > -sizeof(xfs_efi_log_format_32_t) = 28 > -sizeof(xfs_efi_log_format_64_t) = 32 > +sizeof(xfs_efd_log_format_32_t) = 16 > +sizeof(xfs_efd_log_format_64_t) = 16 > +sizeof(xfs_efi_log_format_32_t) = 16 > +sizeof(xfs_efi_log_format_64_t) = 16 > sizeof(xfs_error_injection_t) = 8 > sizeof(xfs_exntfmt_t) = 4 > sizeof(xfs_exntst_t) = 4 >
On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote: > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Adjust this test since made EFI/EFD log item format structs proper flex > > arrays instead of array[1]. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > So we let this case fail on all older system/kernel? Is the kernel patch > recommended to be merged on downstream kernel? Yes, since there are certain buggy compilers that mishandle the array size computation. Prior to the introduction of xfs_ondisk.h, they were silently writing out filesystem structures that would not be recognized by more mainstream systems (e.g. x86). OFC nearly all those reports about buggy compilers are for tiny architectures that XFS doesn't work well on anyways, so in practice it hasn't created any user problems (AFAIK). --D > Thanks, > Zorro > > > tests/xfs/122.out | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out > > index a56cbee84f..95e53c5081 100644 > > --- a/tests/xfs/122.out > > +++ b/tests/xfs/122.out > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104 > > sizeof(xfs_dq_logformat_t) = 24 > > sizeof(xfs_dqblk_t) = 136 > > sizeof(xfs_dsb_t) = 264 > > -sizeof(xfs_efd_log_format_32_t) = 28 > > -sizeof(xfs_efd_log_format_64_t) = 32 > > -sizeof(xfs_efi_log_format_32_t) = 28 > > -sizeof(xfs_efi_log_format_64_t) = 32 > > +sizeof(xfs_efd_log_format_32_t) = 16 > > +sizeof(xfs_efd_log_format_64_t) = 16 > > +sizeof(xfs_efi_log_format_32_t) = 16 > > +sizeof(xfs_efi_log_format_64_t) = 16 > > sizeof(xfs_error_injection_t) = 8 > > sizeof(xfs_exntfmt_t) = 4 > > sizeof(xfs_exntst_t) = 4 > > >
On Sat, Dec 17, 2022 at 12:14:10AM -0800, Darrick J. Wong wrote: > On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote: > > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > Adjust this test since made EFI/EFD log item format structs proper flex > > > arrays instead of array[1]. > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > > So we let this case fail on all older system/kernel? Is the kernel patch > > recommended to be merged on downstream kernel? > > Yes, since there are certain buggy compilers that mishandle the array > size computation. Prior to the introduction of xfs_ondisk.h, they were > silently writing out filesystem structures that would not be recognized > by more mainstream systems (e.g. x86). > > OFC nearly all those reports about buggy compilers are for tiny > architectures that XFS doesn't work well on anyways, so in practice it > hasn't created any user problems (AFAIK). Thanks, may you provide this detailed explanation in commit log, and better to point out the kernel commits which is related with this testing change. Due to this case isn't a case for a known issue, I think it might be no suitable to add _fixed_by_kernel_commit in this case, but how about giving more details in commit log. Thanks, Zorro > > --D > > > Thanks, > > Zorro > > > > > tests/xfs/122.out | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out > > > index a56cbee84f..95e53c5081 100644 > > > --- a/tests/xfs/122.out > > > +++ b/tests/xfs/122.out > > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104 > > > sizeof(xfs_dq_logformat_t) = 24 > > > sizeof(xfs_dqblk_t) = 136 > > > sizeof(xfs_dsb_t) = 264 > > > -sizeof(xfs_efd_log_format_32_t) = 28 > > > -sizeof(xfs_efd_log_format_64_t) = 32 > > > -sizeof(xfs_efi_log_format_32_t) = 28 > > > -sizeof(xfs_efi_log_format_64_t) = 32 > > > +sizeof(xfs_efd_log_format_32_t) = 16 > > > +sizeof(xfs_efd_log_format_64_t) = 16 > > > +sizeof(xfs_efi_log_format_32_t) = 16 > > > +sizeof(xfs_efi_log_format_64_t) = 16 > > > sizeof(xfs_error_injection_t) = 8 > > > sizeof(xfs_exntfmt_t) = 4 > > > sizeof(xfs_exntst_t) = 4 > > > > > >
On Sat, Dec 17, 2022 at 06:00:29PM +0800, Zorro Lang wrote: > On Sat, Dec 17, 2022 at 12:14:10AM -0800, Darrick J. Wong wrote: > > On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote: > > > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote: > > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > > > Adjust this test since made EFI/EFD log item format structs proper flex > > > > arrays instead of array[1]. > > > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > > --- > > > > > > So we let this case fail on all older system/kernel? Is the kernel patch > > > recommended to be merged on downstream kernel? > > > > Yes, since there are certain buggy compilers that mishandle the array > > size computation. Prior to the introduction of xfs_ondisk.h, they were > > silently writing out filesystem structures that would not be recognized > > by more mainstream systems (e.g. x86). > > > > OFC nearly all those reports about buggy compilers are for tiny > > architectures that XFS doesn't work well on anyways, so in practice it > > hasn't created any user problems (AFAIK). > > Thanks, may you provide this detailed explanation in commit log, and better > to point out the kernel commits which is related with this testing change. Will do. > Due to this case isn't a case for a known issue, I think it might be no > suitable to add _fixed_by_kernel_commit in this case, but how about giving > more details in commit log. Er.... xfs/122 isn't a regression test, so it's not testing previously broken and now fixed code. While I sense that a few peoples' understanding of _fixed_by_kernel_commit might be constrained to "if this test fails, check that your kernel/*fsprogs have commit XXXXX", I myself have started `grep _fixed_by_kernel_commit' to find bug fixes and their related regression tests to suggest backports. ...although I wonder if perhaps we should have a second set of _by_commit helpers that encode "not a regression test, but you might check such-and-such commit"? --D > Thanks, > Zorro > > > > > --D > > > > > Thanks, > > > Zorro > > > > > > > tests/xfs/122.out | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out > > > > index a56cbee84f..95e53c5081 100644 > > > > --- a/tests/xfs/122.out > > > > +++ b/tests/xfs/122.out > > > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104 > > > > sizeof(xfs_dq_logformat_t) = 24 > > > > sizeof(xfs_dqblk_t) = 136 > > > > sizeof(xfs_dsb_t) = 264 > > > > -sizeof(xfs_efd_log_format_32_t) = 28 > > > > -sizeof(xfs_efd_log_format_64_t) = 32 > > > > -sizeof(xfs_efi_log_format_32_t) = 28 > > > > -sizeof(xfs_efi_log_format_64_t) = 32 > > > > +sizeof(xfs_efd_log_format_32_t) = 16 > > > > +sizeof(xfs_efd_log_format_64_t) = 16 > > > > +sizeof(xfs_efi_log_format_32_t) = 16 > > > > +sizeof(xfs_efi_log_format_64_t) = 16 > > > > sizeof(xfs_error_injection_t) = 8 > > > > sizeof(xfs_exntfmt_t) = 4 > > > > sizeof(xfs_exntst_t) = 4 > > > > > > > > > >
On Mon, Dec 19, 2022 at 09:14:50AM -0800, Darrick J. Wong wrote: > On Sat, Dec 17, 2022 at 06:00:29PM +0800, Zorro Lang wrote: > > On Sat, Dec 17, 2022 at 12:14:10AM -0800, Darrick J. Wong wrote: > > > On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote: > > > > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote: > > > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > > > > > Adjust this test since made EFI/EFD log item format structs proper flex > > > > > arrays instead of array[1]. > > > > > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > > > --- > > > > > > > > So we let this case fail on all older system/kernel? Is the kernel patch > > > > recommended to be merged on downstream kernel? > > > > > > Yes, since there are certain buggy compilers that mishandle the array > > > size computation. Prior to the introduction of xfs_ondisk.h, they were > > > silently writing out filesystem structures that would not be recognized > > > by more mainstream systems (e.g. x86). > > > > > > OFC nearly all those reports about buggy compilers are for tiny > > > architectures that XFS doesn't work well on anyways, so in practice it > > > hasn't created any user problems (AFAIK). > > > > Thanks, may you provide this detailed explanation in commit log, and better > > to point out the kernel commits which is related with this testing change. > > Will do. > > > Due to this case isn't a case for a known issue, I think it might be no > > suitable to add _fixed_by_kernel_commit in this case, but how about giving > > more details in commit log. > > Er.... xfs/122 isn't a regression test, so it's not testing previously > broken and now fixed code. While I sense that a few peoples' > understanding of _fixed_by_kernel_commit might be constrained to "if > this test fails, check that your kernel/*fsprogs have commit XXXXX", I > myself have started `grep _fixed_by_kernel_commit' to find bug fixes and > their related regression tests to suggest backports. I generally check _fixed_by_*, secondly check the comments in the code and the commit log related with the case. > > ...although I wonder if perhaps we should have a second set of > _by_commit helpers that encode "not a regression test, but you might > check such-and-such commit"? Yeah, the "fixed_by" sounds not precise for xfs/122. Maybe "_cover_commit" or some better names you have :) Thanks, Zorro > > --D > > > Thanks, > > Zorro > > > > > > > > --D > > > > > > > Thanks, > > > > Zorro > > > > > > > > > tests/xfs/122.out | 8 ++++---- > > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out > > > > > index a56cbee84f..95e53c5081 100644 > > > > > --- a/tests/xfs/122.out > > > > > +++ b/tests/xfs/122.out > > > > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104 > > > > > sizeof(xfs_dq_logformat_t) = 24 > > > > > sizeof(xfs_dqblk_t) = 136 > > > > > sizeof(xfs_dsb_t) = 264 > > > > > -sizeof(xfs_efd_log_format_32_t) = 28 > > > > > -sizeof(xfs_efd_log_format_64_t) = 32 > > > > > -sizeof(xfs_efi_log_format_32_t) = 28 > > > > > -sizeof(xfs_efi_log_format_64_t) = 32 > > > > > +sizeof(xfs_efd_log_format_32_t) = 16 > > > > > +sizeof(xfs_efd_log_format_64_t) = 16 > > > > > +sizeof(xfs_efi_log_format_32_t) = 16 > > > > > +sizeof(xfs_efi_log_format_64_t) = 16 > > > > > sizeof(xfs_error_injection_t) = 8 > > > > > sizeof(xfs_exntfmt_t) = 4 > > > > > sizeof(xfs_exntst_t) = 4 > > > > > > > > > > > > > > >
diff --git a/tests/xfs/122.out b/tests/xfs/122.out index a56cbee84f..95e53c5081 100644 --- a/tests/xfs/122.out +++ b/tests/xfs/122.out @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104 sizeof(xfs_dq_logformat_t) = 24 sizeof(xfs_dqblk_t) = 136 sizeof(xfs_dsb_t) = 264 -sizeof(xfs_efd_log_format_32_t) = 28 -sizeof(xfs_efd_log_format_64_t) = 32 -sizeof(xfs_efi_log_format_32_t) = 28 -sizeof(xfs_efi_log_format_64_t) = 32 +sizeof(xfs_efd_log_format_32_t) = 16 +sizeof(xfs_efd_log_format_64_t) = 16 +sizeof(xfs_efi_log_format_32_t) = 16 +sizeof(xfs_efi_log_format_64_t) = 16 sizeof(xfs_error_injection_t) = 8 sizeof(xfs_exntfmt_t) = 4 sizeof(xfs_exntst_t) = 4