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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux