Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

["Darrick J. Wong" <djwong@kernel.org>]

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
> > > > 
> > > 
> > 
>