| Message ID | 170899915304.896550.17104868811908659798.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/8] generic/604: try to make race occur reliably | expand |
On Mon, Feb 26, 2024 at 06:02:05PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Update this test to check that the ondisk unions for rt bitmap word and > rt summary counts are always the correct size. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- Reviewed-by: Zorro Lang <zlang@redhat.com> > tests/xfs/122 | 2 +- > tests/xfs/122.out | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > > diff --git a/tests/xfs/122 b/tests/xfs/122 > index ba927c77c4..4e5ba1dfee 100755 > --- a/tests/xfs/122 > +++ b/tests/xfs/122 > @@ -195,7 +195,7 @@ echo 'int main(int argc, char *argv[]) {' >>$cprog > # > cat /usr/include/xfs/xfs*.h | indent |\ > _attribute_filter |\ > -grep -E '(} *xfs_.*_t|^struct xfs_[a-z0-9_]*$)' |\ > +grep -E '(} *xfs_.*_t|^(union|struct) xfs_[a-z0-9_]*$)' |\ > grep -E -v -f $tmp.ignore |\ > sed -e 's/^.*}[[:space:]]*//g' -e 's/;.*$//g' -e 's/_t, /_t\n/g' |\ > sort | uniq |\ > diff --git a/tests/xfs/122.out b/tests/xfs/122.out > index 067a0ec76b..a2b57cfb9b 100644 > --- a/tests/xfs/122.out > +++ b/tests/xfs/122.out > @@ -124,6 +124,8 @@ sizeof(struct xfs_swap_extent) = 64 > sizeof(struct xfs_sxd_log_format) = 16 > sizeof(struct xfs_sxi_log_format) = 80 > sizeof(struct xfs_unmount_log_format) = 8 > +sizeof(union xfs_rtword_raw) = 4 > +sizeof(union xfs_suminfo_raw) = 4 > sizeof(xfs_agf_t) = 224 > sizeof(xfs_agfl_t) = 36 > sizeof(xfs_agi_t) = 344 >
Can we please just kill the goddamn test? Just waiting for the xfsprogs 6.8 resync to submit the static_asserts for libxfs that will handle this much better.
On Tue, Feb 27, 2024 at 06:54:41AM -0800, Christoph Hellwig wrote: > Can we please just kill the goddamn test? Just waiting for the > xfsprogs 6.8 resync to submit the static_asserts for libxfs that > will handle this much better. I'll be very happen when we scuttle xfs/122 finally. However, in theory it's still be useful for QA departments to make sure that xfsprogs backports (HA!) don't accidentally break things. IOWs, I advocate for _notrunning this test if xfsprogs >= 6.8 is detected, not removing it completely. Unless someone wants to chime in and say that actually, nobody backports stuff to old xfsprogs? (We don't really...) --D
On Tue, Feb 27, 2024 at 05:27:04PM -0800, Darrick J. Wong wrote: > On Tue, Feb 27, 2024 at 06:54:41AM -0800, Christoph Hellwig wrote: > > Can we please just kill the goddamn test? Just waiting for the > > xfsprogs 6.8 resync to submit the static_asserts for libxfs that > > will handle this much better. > > I'll be very happen when we scuttle xfs/122 finally. > > However, in theory it's still be useful for QA departments to make sure > that xfsprogs backports (HA!) don't accidentally break things. > > IOWs, I advocate for _notrunning this test if xfsprogs >= 6.8 is > detected, not removing it completely. > > Unless someone wants to chime in and say that actually, nobody backports > stuff to old xfsprogs? (We don't really...) Well, who is going to backport changes to the on-disk format in a way that is complex enough to change strutures, and not also backport the patch to actually check the sizes? Sounds like a weird use case to optimize for.
On Wed, Feb 28, 2024 at 07:39:29AM -0800, Christoph Hellwig wrote: > On Tue, Feb 27, 2024 at 05:27:04PM -0800, Darrick J. Wong wrote: > > On Tue, Feb 27, 2024 at 06:54:41AM -0800, Christoph Hellwig wrote: > > > Can we please just kill the goddamn test? Just waiting for the > > > xfsprogs 6.8 resync to submit the static_asserts for libxfs that > > > will handle this much better. > > > > I'll be very happen when we scuttle xfs/122 finally. > > > > However, in theory it's still be useful for QA departments to make sure > > that xfsprogs backports (HA!) don't accidentally break things. > > > > IOWs, I advocate for _notrunning this test if xfsprogs >= 6.8 is > > detected, not removing it completely. > > > > Unless someone wants to chime in and say that actually, nobody backports > > stuff to old xfsprogs? (We don't really...) > > Well, who is going to backport changes to the on-disk format in a way > that is complex enough to change strutures, and not also backport the > patch to actually check the sizes? Sounds like a weird use case to > optimize for. It turns out that xfs/122 also captures ioctl structure sizes, and those are /not/ captured by xfs_ondisk.h. I think we should add those before we kill xfs/122. --D
On Thu, Feb 29, 2024 at 09:48:31AM -0800, Darrick J. Wong wrote: > It turns out that xfs/122 also captures ioctl structure sizes, and those > are /not/ captured by xfs_ondisk.h. I think we should add those before > we kill xfs/122. Sure, I can look into that.
On Thu, Feb 29, 2024 at 11:42:07AM -0800, Christoph Hellwig wrote: > On Thu, Feb 29, 2024 at 09:48:31AM -0800, Darrick J. Wong wrote: > > It turns out that xfs/122 also captures ioctl structure sizes, and those > > are /not/ captured by xfs_ondisk.h. I think we should add those before > > we kill xfs/122. > > Sure, I can look into that. Hi Darrick, Do you still want to have this patch? Half of this patchset got RVB. As it's a random fix patchset, we can choose merging those reviewed patches at first. Or you'd like to have them together in next next release? Thanks, Zorro > >
On Fri, Mar 01, 2024 at 09:18:48PM +0800, Zorro Lang wrote: > On Thu, Feb 29, 2024 at 11:42:07AM -0800, Christoph Hellwig wrote: > > On Thu, Feb 29, 2024 at 09:48:31AM -0800, Darrick J. Wong wrote: > > > It turns out that xfs/122 also captures ioctl structure sizes, and those > > > are /not/ captured by xfs_ondisk.h. I think we should add those before > > > we kill xfs/122. > > > > Sure, I can look into that. > > Hi Darrick, > > Do you still want to have this patch? > > Half of this patchset got RVB. As it's a random fix patchset, we can choose > merging those reviewed patches at first. Or you'd like to have them together > in next next release? I was about to resend the second to last patch. If you decide to remove xfs/122 then I'll drop this one. --D > Thanks, > Zorro > > > > > > >
On Fri, Mar 01, 2024 at 09:50:20AM -0800, Darrick J. Wong wrote: > On Fri, Mar 01, 2024 at 09:18:48PM +0800, Zorro Lang wrote: > > On Thu, Feb 29, 2024 at 11:42:07AM -0800, Christoph Hellwig wrote: > > > On Thu, Feb 29, 2024 at 09:48:31AM -0800, Darrick J. Wong wrote: > > > > It turns out that xfs/122 also captures ioctl structure sizes, and those > > > > are /not/ captured by xfs_ondisk.h. I think we should add those before > > > > we kill xfs/122. > > > > > > Sure, I can look into that. > > > > Hi Darrick, > > > > Do you still want to have this patch? > > > > Half of this patchset got RVB. As it's a random fix patchset, we can choose > > merging those reviewed patches at first. Or you'd like to have them together > > in next next release? > > I was about to resend the second to last patch. If you decide to remove > xfs/122 then I'll drop this one. xfs/122 is a xfs specific test case, it's more important for xfs list than me. As it doesn't break the fstests testing, I respect the decision from xfs folks, about keeping or removing it :) Thanks, Zorro > > --D > > > Thanks, > > Zorro > > > > > > > > > > > > >
On Sat, Mar 02, 2024 at 12:55:52PM +0800, Zorro Lang wrote: > On Fri, Mar 01, 2024 at 09:50:20AM -0800, Darrick J. Wong wrote: > > On Fri, Mar 01, 2024 at 09:18:48PM +0800, Zorro Lang wrote: > > > On Thu, Feb 29, 2024 at 11:42:07AM -0800, Christoph Hellwig wrote: > > > > On Thu, Feb 29, 2024 at 09:48:31AM -0800, Darrick J. Wong wrote: > > > > > It turns out that xfs/122 also captures ioctl structure sizes, and those > > > > > are /not/ captured by xfs_ondisk.h. I think we should add those before > > > > > we kill xfs/122. > > > > > > > > Sure, I can look into that. > > > > > > Hi Darrick, > > > > > > Do you still want to have this patch? > > > > > > Half of this patchset got RVB. As it's a random fix patchset, we can choose > > > merging those reviewed patches at first. Or you'd like to have them together > > > in next next release? > > > > I was about to resend the second to last patch. If you decide to remove > > xfs/122 then I'll drop this one. > > xfs/122 is a xfs specific test case, it's more important for xfs list than me. > As it doesn't break the fstests testing, I respect the decision from xfs folks, > about keeping or removing it :) I think we shouldn't consider dropping this until there's an xfsprogs release with xfs_ondisk.h in the build process. Even then, my preference would be to leave a mark in xfs_db somewhere so that we keep running this test for old userspace (i.e. the mark isn't found). --D > Thanks, > Zorro > > > > > --D > > > > > Thanks, > > > Zorro > > > > > > > > > > > > > > > > > > > > >
diff --git a/tests/xfs/122 b/tests/xfs/122 index ba927c77c4..4e5ba1dfee 100755 --- a/tests/xfs/122 +++ b/tests/xfs/122 @@ -195,7 +195,7 @@ echo 'int main(int argc, char *argv[]) {' >>$cprog # cat /usr/include/xfs/xfs*.h | indent |\ _attribute_filter |\ -grep -E '(} *xfs_.*_t|^struct xfs_[a-z0-9_]*$)' |\ +grep -E '(} *xfs_.*_t|^(union|struct) xfs_[a-z0-9_]*$)' |\ grep -E -v -f $tmp.ignore |\ sed -e 's/^.*}[[:space:]]*//g' -e 's/;.*$//g' -e 's/_t, /_t\n/g' |\ sort | uniq |\ diff --git a/tests/xfs/122.out b/tests/xfs/122.out index 067a0ec76b..a2b57cfb9b 100644 --- a/tests/xfs/122.out +++ b/tests/xfs/122.out @@ -124,6 +124,8 @@ sizeof(struct xfs_swap_extent) = 64 sizeof(struct xfs_sxd_log_format) = 16 sizeof(struct xfs_sxi_log_format) = 80 sizeof(struct xfs_unmount_log_format) = 8 +sizeof(union xfs_rtword_raw) = 4 +sizeof(union xfs_suminfo_raw) = 4 sizeof(xfs_agf_t) = 224 sizeof(xfs_agfl_t) = 36 sizeof(xfs_agi_t) = 344