Date: Tue, 18 Apr 2023 17:07:07 -0600 From: Warner Losh <imp@bsdimp.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@freebsd.org> Subject: Re: git: 238271f4a66b - main - stand: Add a snarky note about the upstream ZFS situation Message-ID: <CANCZdfr6%2BXsedE690%2BmR3zug6ZOGRErsLg%2BHhWsuB2nYcS-0zg@mail.gmail.com> In-Reply-To: <B84E9112-427A-4B93-940A-A7734D00A3AB@freebsd.org> References: <202304182131.33ILVSoG020217@gitrepo.freebsd.org> <C898F70B-7CA2-451F-A080-ADBFB472561D@freebsd.org> <CANCZdfry1T-xy-xQ19Cch42qmeNNAHFgQf_W61FCbSHWis_aNg@mail.gmail.com> <B84E9112-427A-4B93-940A-A7734D00A3AB@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000007d0ba505f9a45e9e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Apr 18, 2023 at 4:52=E2=80=AFPM Jessica Clarke <jrtc27@freebsd.org>= wrote: > On 18 Apr 2023, at 23:49, Warner Losh <imp@bsdimp.com> wrote: > > > > > > > > On Tue, Apr 18, 2023, 3:34 PM Jessica Clarke <jrtc27@freebsd.org> wrote= : > >> On 18 Apr 2023, at 22:31, Warner Losh <imp@FreeBSD.org> wrote: > >> > > >> > The branch main has been updated by imp: > >> > > >> > URL: > https://cgit.FreeBSD.org/src/commit/?id=3D238271f4a66bd06b8b9a232a82f3ee0= 882e4cbb9 > >> > > >> > commit 238271f4a66bd06b8b9a232a82f3ee0882e4cbb9 > >> > Author: Warner Losh <imp@FreeBSD.org> > >> > AuthorDate: 2023-04-18 21:29:45 +0000 > >> > Commit: Warner Losh <imp@FreeBSD.org> > >> > CommitDate: 2023-04-18 21:31:17 +0000 > >> > > >> > stand: Add a snarky note about the upstream ZFS situation > >> > > >> > The latest import of openzfs broke the hacks that we used to omit > the > >> > special registers being used on arm64. Add snarky note documentin= g > this > >> > situation since it's a mess now since the hack was only partially > >> > undone, leaving behind a mess. > >> > > >> > Sponsored by: Netflix > >> > --- > >> > stand/libsa/zfs/Makefile.inc | 4 ++++ > >> > 1 file changed, 4 insertions(+) > >> > > >> > diff --git a/stand/libsa/zfs/Makefile.inc > b/stand/libsa/zfs/Makefile.inc > >> > index f4cecdbc3085..7660f4ab7baf 100644 > >> > --- a/stand/libsa/zfs/Makefile.inc > >> > +++ b/stand/libsa/zfs/Makefile.inc > >> > @@ -19,6 +19,7 @@ ZSTD_SRC+=3D zstd_common.c > >> > ZSTD_SRC+=3D zstd_ddict.c zstd_decompress.c zstd_decompress_block= .c > >> > ZSTD_SRC+=3D zstd_double_fast.c zstd_fast.c zstd_lazy.c zstd_ldm.= c > >> > > >> > +# This is completely bogus: We should be able to omit this code > completely. > >> > .if ${MACHINE_ARCH} =3D=3D "aarch64" > >> > ZFS_SRC_AS =3D b3_aarch64_sse2.S b3_aarch64_sse41.S > >> > .endif > >> > @@ -90,10 +91,13 @@ CFLAGS.skein_block.c+=3D -DSKEIN_LOOP=3D111 > >> > > >> > # To find blake3_impl.c in OpenZFS tree for our somehat ugly > blake3_impl_hack.c > >> > # that's needed until the necessary tweaks can be upstreamed. > >> > +# XXX the last import gutted all this since upstream changes broke > this hack. > >> > CFLAGS.blake3_impl_hack.c+=3D -I${OZFS}/module/icp/algs/blake3 > -I${OZFS}/module/icp/include > >> > > >> > CWARNFLAGS.zfs.c+=3D ${NO_WDANGLING_POINTER} > >> > > >> > +# Needing to remove the -mgeneral-regs-only is a red flag that this > is not quite > >> > +# right. But it's needed at the moment due to the muddled upstream. > >> > >> This one isn=E2=80=99t bogus? The file is deliberately using NEON so n= eeds > >> access to floating-point registers, which LLVM (mostly) enforces for > >> the assembler, unlike GNU as. > > > > No. It's bogus because we should not be using it at all. The generic > implementation is fast enough for the boot loader and we have a blanket > policy against using extra register sets. The change wasn't discussed > before hand and what's really needed are upstream changes to be able to > omit it entirely. > > Oh I see, I misunderstood the point the comment was making. > It's also the third or fourth time that I've had to sort this, or similar, issues out in the boot loader (after spending a week or two initially to enable the OpenZFS import), so I'm a little frustrated by the situation. Warner > Thanks, > Jess > > > > b3_aarch64_sse2.o: b3_aarch64_sse2.S > > > ${CC} -c ${CFLAGS:N-mgeneral-regs-only} ${WERROR} ${.IMPSRC} \ > > > -o ${.TARGET} > > --0000000000007d0ba505f9a45e9e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Tue, Apr 18, 2023 at 4:52=E2=80=AF= PM Jessica Clarke <<a href=3D"mailto:jrtc27@freebsd.org">jrtc27@freebsd.= org</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"marg= in:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1e= x">On 18 Apr 2023, at 23:49, Warner Losh <<a href=3D"mailto:imp@bsdimp.c= om" target=3D"_blank">imp@bsdimp.com</a>> wrote:<br> > <br> > <br> > <br> > On Tue, Apr 18, 2023, 3:34 PM Jessica Clarke <<a href=3D"mailto:jrt= c27@freebsd.org" target=3D"_blank">jrtc27@freebsd.org</a>> wrote:<br> >> On 18 Apr 2023, at 22:31, Warner Losh <imp@FreeBSD.org> wrot= e:<br> >> > <br> >> > The branch main has been updated by imp:<br> >> > <br> >> > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D238= 271f4a66bd06b8b9a232a82f3ee0882e4cbb9" rel=3D"noreferrer" target=3D"_blank"= >https://cgit.FreeBSD.org/src/commit/?id=3D238271f4a66bd06b8b9a232a82f3ee08= 82e4cbb9</a><br> >> > <br> >> > commit 238271f4a66bd06b8b9a232a82f3ee0882e4cbb9<br> >> > Author:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org>= ;<br> >> > AuthorDate: 2023-04-18 21:29:45 +0000<br> >> > Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org>= ;<br> >> > CommitDate: 2023-04-18 21:31:17 +0000<br> >> > <br> >> >=C2=A0 =C2=A0 stand: Add a snarky note about the upstream ZFS = situation<br> >> > <br> >> >=C2=A0 =C2=A0 The latest import of openzfs broke the hacks tha= t we used to omit the<br> >> >=C2=A0 =C2=A0 special registers being used on arm64. Add snark= y note documenting this<br> >> >=C2=A0 =C2=A0 situation since it's a mess now since the ha= ck was only partially<br> >> >=C2=A0 =C2=A0 undone, leaving behind a mess.<br> >> > <br> >> >=C2=A0 =C2=A0 Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0Netflix<br> >> > ---<br> >> > stand/libsa/zfs/Makefile.inc | 4 ++++<br> >> > 1 file changed, 4 insertions(+)<br> >> > <br> >> > diff --git a/stand/libsa/zfs/Makefile.inc b/stand/libsa/zfs/M= akefile.inc<br> >> > index f4cecdbc3085..7660f4ab7baf 100644<br> >> > --- a/stand/libsa/zfs/Makefile.inc<br> >> > +++ b/stand/libsa/zfs/Makefile.inc<br> >> > @@ -19,6 +19,7 @@ ZSTD_SRC+=3D=C2=A0 zstd_common.c<br> >> > ZSTD_SRC+=3D=C2=A0 =C2=A0 zstd_ddict.c zstd_decompress.c zstd= _decompress_block.c<br> >> > ZSTD_SRC+=3D=C2=A0 =C2=A0 zstd_double_fast.c zstd_fast.c zstd= _lazy.c zstd_ldm.c<br> >> > <br> >> > +# This is completely bogus: We should be able to omit this c= ode completely.<br> >> > .if ${MACHINE_ARCH} =3D=3D "aarch64"<br> >> > ZFS_SRC_AS =3D=C2=A0 b3_aarch64_sse2.S b3_aarch64_sse41.S<br> >> > .endif<br> >> > @@ -90,10 +91,13 @@ CFLAGS.skein_block.c+=3D=C2=A0 =C2=A0 -DS= KEIN_LOOP=3D111<br> >> > <br> >> > # To find blake3_impl.c in OpenZFS tree for our somehat ugly = blake3_impl_hack.c<br> >> > # that's needed until the necessary tweaks can be upstrea= med.<br> >> > +# XXX the last import gutted all this since upstream changes= broke this hack.<br> >> > CFLAGS.blake3_impl_hack.c+=3D -I${OZFS}/module/icp/algs/blake= 3 -I${OZFS}/module/icp/include<br> >> > <br> >> > CWARNFLAGS.zfs.c+=3D ${NO_WDANGLING_POINTER}<br> >> > <br> >> > +# Needing to remove the -mgeneral-regs-only is a red flag th= at this is not quite<br> >> > +# right. But it's needed at the moment due to the muddle= d upstream.<br> >> <br> >> This one isn=E2=80=99t bogus? The file is deliberately using NEON = so needs<br> >> access to floating-point registers, which LLVM (mostly) enforces f= or<br> >> the assembler, unlike GNU as.<br> > <br> > No. It's bogus because we should not be using it at all. The gener= ic implementation is fast enough for the boot loader and we have a blanket = policy against using extra register sets. The change wasn't discussed b= efore hand and what's really needed are upstream changes to be able to = omit it entirely.<br> <br> Oh I see, I misunderstood the point the comment was making.<br></blockquote= ><div><br></div><div>It's also the third or fourth time that I've h= ad to sort this, or similar, issues out in the boot loader (after spending = a week or two initially to enable the OpenZFS import), so I'm a little = frustrated by the situation.</div><div><br></div><div>Warner</div><div>=C2= =A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8e= x;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Thanks,<br> Jess<br> <br> > > b3_aarch64_sse2.o: b3_aarch64_sse2.S<br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0${CC} -c ${CFLAGS:N-mgeneral-regs-only}= ${WERROR} ${.IMPSRC} \<br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0-o ${.TARGET}<br> <br> </blockquote></div></div> --0000000000007d0ba505f9a45e9e--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfr6%2BXsedE690%2BmR3zug6ZOGRErsLg%2BHhWsuB2nYcS-0zg>